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] Fix updating editor tab titles in the dock menu. #86290

Merged
merged 1 commit into from
Dec 19, 2023

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Dec 18, 2023

Fixes errors cause by update_tab_titles trying to update non-existing global menu items.

@YuriSizov
Copy link
Contributor

YuriSizov commented Dec 18, 2023

I'm not entirely sure what's going on here. Where do these non-existent items come from? I think we should fix that instead.

Also, if I understand the hack correctly, adding and removing 1 would be sufficient, it doesn't have to be 100? We just need to differentiate between actual index 0 and nothing being cast to 0? If we keep the hack, worth adding a code comment to explain it.

cc @kitbdev.

@bruvzg
Copy link
Member Author

bruvzg commented Dec 18, 2023

Where do these non-existent items come from? I think we should fix that instead.

update_tab_titles can be called without full update (which is adding menu items, now both function can add items).

Also, if I understand the hack correctly, adding and removing 1 would be sufficient, it doesn't have to be 100?

Yes, it's only to detect empty meta (which is cast to zero).

@YuriSizov
Copy link
Contributor

update_tab_titles can be called without full update (which is adding menu items, now both function can add items).

How do we get into this situation? The point of separating update methods was to avoid calling some of the routines if the number of tabs didn't change. If we call update_tab_titles when we actually need to do a full update, that seems to be a bug.

@kitbdev
Copy link
Contributor

kitbdev commented Dec 18, 2023

_update_tab_titles() is only called when the number of tabs is equal to the number of scenes. The number of global menu items should always be the same as the number of tabs, since it is updated in the same place, in update_scene_tabs().

The only scenario I can think of that would cause the counts to not be the same is if DisplayServer::get_singleton()->has_feature(DisplayServer::FEATURE_GLOBAL_MENU) can return different values between calls. I don't think this is possible though.
Or is it possible that global_menu_add_item fails?
I don't have a mac to test, so I might be missing something.

@bruvzg
Copy link
Member Author

bruvzg commented Dec 19, 2023

Actually the fix is much simpler, the core reason: first tab was added before connecting signals and most of the editor initialized, so first update is always called when it's already 1 == 1.

Copy link
Contributor

@YuriSizov YuriSizov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that makes sense now. Thanks for looking into this!

@YuriSizov YuriSizov merged commit 1f5d4a6 into godotengine:master Dec 19, 2023
15 checks passed
@YuriSizov
Copy link
Contributor

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