Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[macOS] Always execute editor instances using NSWorkspace to ensure app window is registered and activated correctly. #54474

Merged
merged 1 commit into from
Nov 1, 2021

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Nov 1, 2021

Fixes #54306
Fixes #54458

Precompiled .app bundle to test (ad-hoc signed so xattr -dr com.apple.quarantine /path/to/GodotTest.app before running): https://mega.nz/file/IhAXVKBR#NDtx9atZNdSQBmbHDFfi9_aemlLmJ5-qBC2sSw82iSQ

Edit: changed the link to a new one, without unrelated custom modules.

@bruvzg
Copy link
Member Author

bruvzg commented Nov 1, 2021

For master we probably want to do it in the less hacky way, like change get_executable_path() to return app bundle path, and create_process to check if provided path is bundle and use NSWorkspace in this case.

…pp window is registered and activated correctly.
@NoFr1ends
Copy link
Contributor

Tested your changes and it seems to have solved the issue. The editor is now correctly focused and doesn't require the workaround anymore.

@bruvzg bruvzg marked this pull request as ready for review November 1, 2021 12:53
@bruvzg bruvzg requested a review from a team as a code owner November 1, 2021 12:53
@bruvzg
Copy link
Member Author

bruvzg commented Nov 1, 2021

So far I have not found any issues with this, should be good to go. It should not affect any other platforms and running anything else except new instances of the editor.

@akien-mga akien-mga modified the milestones: 3.5, 3.4 Nov 1, 2021
@akien-mga akien-mga merged commit 18c185b into godotengine:3.x Nov 1, 2021
@akien-mga
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants