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

Fix closing Theme Editor not actually closing it #97543

Merged
merged 1 commit into from
Oct 4, 2024

Conversation

KoBeWi
Copy link
Member

@KoBeWi KoBeWi commented Sep 27, 2024

Fixes #97278

The issue has uncovered a bug in editor closing logic. Folding a resource will call the editor to stop editing it, but the actual unediting code assumed that the EditorPropertyResource no longer exists (which can happen when inspector updates, e.g. when editing another object). Which means the resource wasn't actually unedited.

I wasn't sure how to solve it properly, so I just added _should_stop_editing internal method. It's meant for editor contexts to inform whether they are still editing the object or not. EditorPropertyResource will return true when it's folded, so folding a property now properly stops editing it.

Then there is ThemeEditorPlugin, which used the self-owning mechanism added in #81523, but the method for determining whether the editor should be hidden was overly complex and assumed that the Theme is opened in the inspector. I changed it to simply stay open while it edits a valid theme. Which means that Theme Editor will keep Theme reference until you explicitly press Close button; it no longer automatically disappears. Not sure if that's expected behavior, but it's at least convenient 🤔

The remaining problem is that when you close the scene, a built-in Theme can still remain referenced, resulting in potential data loss. Not sure about a solution for this one (maybe some WeakRef?), but it's probably fine for now. Solved it with theme_closed signal, which will automatically close editor if it edits built-in Theme.

@KoBeWi KoBeWi added this to the 4.4 milestone Sep 27, 2024
@KoBeWi KoBeWi requested a review from a team as a code owner September 27, 2024 14:46
@KoBeWi KoBeWi force-pushed the to_edit_or_not_to_edit branch from 69f44a4 to 66d2b0f Compare September 27, 2024 15:08
@akien-mga akien-mga merged commit def5a04 into godotengine:master Oct 4, 2024
19 checks passed
@akien-mga
Copy link
Member

Thanks!

@KoBeWi KoBeWi deleted the to_edit_or_not_to_edit branch October 4, 2024 20:51
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.

The theme editor is opened only once
2 participants