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

Handle inspector partial tree update on changed properties #44982

Conversation

pouleyKetchoupp
Copy link
Contributor

@pouleyKetchoupp pouleyKetchoupp commented Jan 6, 2021

Adds a cache to the inspector in order to update only modified properties when the property list changes and keep the previous controls whenever possible.

This allows calls to property_list_changed_notify to trigger inspector updates without resetting all properties.

Fixes #44854 (regression from #44326)
Fixes #43238 (re-applies #44326, reverted because of regression #44854)

Also fixes change signal emission for multiline string properties (same as #44326 but for multiline strings).

Note: The case with multiline string property when editing the string in the big popup window will require further changes. It currently closes and re-opens the popup when the inspector is updated (before this PR it was just closing it).

@pouleyKetchoupp pouleyKetchoupp added bug topic:editor cherrypick:3.x Considered for cherry-picking into a future 3.x release labels Jan 6, 2021
@akien-mga akien-mga requested review from reduz and groud January 7, 2021 09:28
@akien-mga
Copy link
Member

akien-mga commented Jan 7, 2021

Also fixes change signal emission for multiline string properties (same as #44326 but for multiline strings).

I was thinking it might be cleaner to revert #44326, and have its changes done again in this PR together with multiline strings. So this would no longer be a regression fix PR but a bugfix and enhancement of the inspector.

Then there's also only this PR to cherry-pick for 3.2, with no temporary regression.

akien-mga added a commit that referenced this pull request Jan 7, 2021
This reverts commit ed1f208.

This caused a regression: #44854.

Another PR will re-apply these changes while handling the regression: #44982.

Fixes #44854.
@akien-mga
Copy link
Member

I was thinking it might be cleaner to revert #44326, and have its changes done again in this PR together with multiline strings. So this would no longer be a regression fix PR but a bugfix and enhancement of the inspector.

Done with 6583ac3.

You can amend this PR to include the changes from #44326 now that you've made them safe, and this PR would now fix #43238 again.

@Chaosus Chaosus added this to the 4.0 milestone Jan 7, 2021
Adds a cache to the inspector in order to update only modified
properties when the property list changes and keep the previous controls
whenever possible.

This allows calls to property_list_changed_notify to trigger inspector
updates without resetting all properties.

Fixes godotengine#43238 (re-applies godotengine#44326, reverted because of regression godotengine#44854)

Also fixes change signal emission for multiline string properties (same as godotengine#44326 but for multiline strings).
@pouleyKetchoupp pouleyKetchoupp force-pushed the inspector-partial-update branch from 12864e4 to 67eec95 Compare January 7, 2021 16:17
@pouleyKetchoupp
Copy link
Contributor Author

You can amend this PR to include the changes from #44326 now that you've made them safe, and this PR would now fix #43238 again.

Done!

@reduz
Copy link
Member

reduz commented Mar 20, 2021

Given I refactored how inspector refreshes, including checking a polling approach, does this still make sense any more?

@pouleyKetchoupp
Copy link
Contributor Author

@reduz You're right, it probably doesn't make sense for master anymore. Once issues are confirmed to be fixed on master I'll make a new PR specifically for 3.4 since it can still be useful for 3.x users.

@pouleyKetchoupp
Copy link
Contributor Author

Closing this as it doesn't apply to master anymore, and might cause undesired side effects on 3.x.

@pouleyKetchoupp pouleyKetchoupp deleted the inspector-partial-update branch August 30, 2021 17:13
@YuriSizov YuriSizov removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants