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

Remove duplicate displayed node update call #32908

Merged
merged 1 commit into from
Jul 21, 2020

Conversation

Nehluxhes
Copy link
Contributor

@Nehluxhes Nehluxhes commented Oct 18, 2019

Each time a new node is selected in the scene tree dock, both _node_selected() and _selection_changed() are called.
They are then both calling editor->push_item(), resulting in potentially lots of duplicate calls downstream (eg a Gridmap node is loaded twice)
This fixes this.

Ideally I think that _node_selected() and _selection_changed() should be merged, they are redundant.

Copy link
Member

@KoBeWi KoBeWi left a comment

Choose a reason for hiding this comment

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

I did some tests with the code and it seems to work fine. I can also confirm that some things were needlessly called twice before this change. Though can't tell if it doesn't have some unexpected effects somewhere.

@akien-mga
Copy link
Member

As it's not critical, let's merge it after 3.2 and cherry-pick if no regressions happen.

@akien-mga akien-mga modified the milestones: 3.2, 4.0 Nov 8, 2019
@akien-mga akien-mga added the cherrypick:3.x Considered for cherry-picking into a future 3.x release label Nov 8, 2019
@aaronfranke
Copy link
Member

@Nehluxhes Is this still desired? If so, it needs to be rebased on the latest master branch.

@Nehluxhes
Copy link
Contributor Author

@aaronfranke @akien-mga It is still desired and I just rebased on latest master.

@akien-mga akien-mga merged commit 60fab23 into godotengine:master Jul 21, 2020
@akien-mga
Copy link
Member

Thanks!

@akien-mga
Copy link
Member

Cherry-picked for 3.2.4.

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.

4 participants