Skip to content

Conversation

@supertick
Copy link

I believe the function was broken 5 years ago to "satisfy a Mac problem?" - it does not spawn a new asynchronous thread or obey the waitFor parameter as was originally intended.

I was not sure what to do, I don't have a Mac, so I put in a conditional - probably someone who has mac hardware could fix this the right case for the single use case.

@tlf30
Copy link
Contributor

tlf30 commented Apr 6, 2021

Please note, this exact change was already discussed and closed in PR #1048.

@stephengold
Copy link
Member

Why was #1048 closed?

@tlf30
Copy link
Contributor

tlf30 commented Apr 6, 2021

It can be re-opened if people are interested. It was open for 6 months with no more feedback or interest in moving it forward.
I personally think it is ready to be merged if the community is interested.

Would you like me to re-open it?

@Ali-RS
Copy link
Member

Ali-RS commented Apr 6, 2021

Note from a quick glance PR #1048 is slightly different from this one.

This PR checks the OS type and does not start a new thread if it is on Mac.

@pspeed42
Copy link
Contributor

pspeed42 commented Apr 6, 2021

But it's not really a Mac-specific issue if you follow the Glfw docs. It's a Glfw issue that magically/accidentally/may-break-someday happens to work on other platforms today.

I'm also wary about adding "if this value contains one of these magic strings then guess that this might be MacOS today"... it's a maintenance nightmare as we add new strings for every new Mac release that might mess with that string.

@tlf30
Copy link
Contributor

tlf30 commented Apr 6, 2021

I personally think the check for if it is MacOS or not needs to be done at the application level and not within JME.
If an application is expecting the thread to be forked and it is not because it is on MacOS, then it potentially will not work.
Whereas if the application checks for MacOS, then calls the correct start function, it can properly handle the thread forking.

EDIT: And as @pspeed42 stated, really jme should not fork the thread at all as per the glfw docs.

@supertick
Copy link
Author

Note from a quick glance PR #1048 is slightly different from this one.

This PR checks the OS type and does not start a new thread if it is on Mac.

Yes, I thought the code which broke the functionality was unnecessarily Draconian, affecting previous behavior on all OSs.
I looked back in the original code and just tested
if "Mac" use drastic fix .. otherwise, do the thing that was done before.

The real solution would probably be test on Mac hardware, and figure out how to re-implement the original behavior.
Alas, no macs here....

@supertick
Copy link
Author

supertick commented Apr 6, 2021

I worked around the issue pretty easily at the app level, and for me this issue could be closed.
For my own selfish purposes I'm happy. If there is bizarre operating system specifics that need to be dealt with, I'd agree with @pspeed42 .. but perhaps the functions should be marked as deprecated, if they do not behave as originally intended.
Thanks for the input.

@Ali-RS Ali-RS closed this Apr 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants