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

Defer ActionMapEditor::_action_edited signal to prevent tree updates when tree is blocked. #89346

Merged
merged 1 commit into from
May 29, 2024

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Mar 10, 2024

Should fix #86612, by preventing tree update inside propagate_mouse_event, but I'm not sure if this is the best fix (tree internal login probably should be changed to be more robust).

@bruvzg bruvzg added this to the 4.3 milestone Mar 10, 2024
@bruvzg bruvzg requested a review from a team as a code owner March 10, 2024 08:39
@@ -429,6 +429,7 @@ void ActionMapEditor::update_action_list(const Vector<ActionInfo> &p_action_info
// Update Tree...

TreeItem *action_item = action_tree->create_item(root);
ERR_FAIL_NULL(action_item);
Copy link
Member Author

Choose a reason for hiding this comment

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

This should never trigger, added just in case.

Copy link
Member

Choose a reason for hiding this comment

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

I know this comes from the issue, but there are more potentially crashing spots like that. Kinda odd to handle only this one specifically.

scene/gui/tree.cpp Outdated Show resolved Hide resolved
@bruvzg bruvzg changed the title [Tree] Defer item_edited signal when tree updates are blocked. Defer ActionMapEditor::_action_edited signal to prevent tree updates when tree is blocked. May 29, 2024
@bruvzg
Copy link
Member Author

bruvzg commented May 29, 2024

Updated to defer ActionMapEditor::_action_edited instead of changing tree.

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.

For some reason I can't reproduce the crash on master 🤔
The fix looks fine, assuming it's still relevant.

@akien-mga akien-mga merged commit 46629e1 into godotengine:master May 29, 2024
16 checks passed
@akien-mga
Copy link
Member

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.

Input System - Changing Value and pressing outside the field crashing Godot Engine
3 participants