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 editor inspector refresh not working #41697

Merged

Conversation

EricEzaM
Copy link
Contributor

@EricEzaM EricEzaM commented Sep 2, 2020

Closes #41675

Inspector will cache refresh interval value so it does not need to EDITOR_GET after every interval.

@EricEzaM EricEzaM force-pushed the bug/editor-inspector-refresh-fix branch from 1daa2e9 to 5612770 Compare September 2, 2020 04:03
@akien-mga akien-mga merged commit 6e89d37 into godotengine:master Sep 2, 2020
@akien-mga
Copy link
Member

Thanks!

@akien-mga akien-mga added bug cherrypick:3.x Considered for cherry-picking into a future 3.x release topic:editor labels Sep 2, 2020
@akien-mga akien-mga added this to the 4.0 milestone Sep 2, 2020
@EricEzaM EricEzaM deleted the bug/editor-inspector-refresh-fix branch September 6, 2020 08:04
@reduz
Copy link
Member

reduz commented Sep 25, 2020

This needs to be reverted, as it misunderstood what refresh is for. Its causing the editor to redraw all the time and this is extremely inefficient.

@reduz
Copy link
Member

reduz commented Sep 25, 2020

The correct solution is a smart refresh, checking if properties changed and only then update them, but this is a much bigger undertaking

@reduz
Copy link
Member

reduz commented Sep 25, 2020

I think you need different logic here. There is a refresh() call which sets the refresh countdown. As you will be refreshing all the time, you can change that to a bool force_update_property_editors; to true instead of setting the refresh countdown.

Then in the refresh logic, you want something like this:

for (Map<StringName, List<EditorProperty *>>::Element *F = editor_property_map.front(); F; F = F->next()) {
	for (List<EditorProperty *>::Element *E = F->get().front(); E; E = E->next()) {
		if (E->get()->has_changed() || force_update_property_editors) {
			E->get()->update_property();
			E->get()->update_reload_status();
		}
	}
}

force_update_property_editors = false;

For the has_changed() method, I think EditorProperty need to be able to figure out when the value inside the object differs from what was edited. One way could be caching the value in EditorProperty::emit_changed, another is doing this manually in every editor which may be too much.

@akien-mga
Copy link
Member

Removing cherry-pick label as this was reverted.

@akien-mga akien-mga removed the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Sep 29, 2020
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.

Editor Inspector does not auto-refresh
3 participants