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 the error when clicking AnimationTree in the editor #79588

Merged
merged 1 commit into from
Aug 2, 2023

Conversation

magian1127
Copy link
Contributor

Fix #78052

@magian1127 magian1127 changed the title Fix AnimationTree(AnimationNodeStateMachine) error reported in the ed… Fix AnimationNodeStateMachine error reported in the editor Jul 17, 2023
@magian1127 magian1127 changed the title Fix AnimationNodeStateMachine error reported in the editor Fix the error when clicking AnimationTree in the editor Jul 18, 2023
@YuriSizov YuriSizov added this to the 4.2 milestone Jul 18, 2023
@YuriSizov
Copy link
Contributor

YuriSizov commented Aug 1, 2023

Thanks for opening a PR, but I don't think this is a correct solution. These are exactly the same checks, so if this fixes the issue that can only be possible by accident. If we need to be aware of running this code from the main thread only, it needs to be done explicitly.

I, unfortunately, can't tell without investigating what exactly depends on the main thread. So I can't point you to anything more specific.

@YuriSizov YuriSizov requested a review from a team August 1, 2023 16:29
@magian1127
Copy link
Contributor Author

When singleton->tree is empty and AnimationTreeEditor.singleton is hidden, then AnimationTreeEditor is outside the main thread.
When singleton->tree is instantiated, AnimationTreeEditor is inside the main thread.
So changing the order of judgment will solve the problem.

@YuriSizov
Copy link
Contributor

Right, but my point is that the reason for this order is not obvious from the code as it is. It needs to be made explicit why the checks are there and in this order. At least something like this would help:

	if (!singleton->tree || !singleton->is_visible()) {
		return Vector<String>(); // Not in the main thread, returning.
	}

	AnimationTree *tree = singleton->tree;
	if (!tree->has_node(tree->get_animation_player())) {
		return Vector<String>();
	}

@magian1127
Copy link
Contributor Author

I followed your format and made some modifications, the deeper reasons, can only rely on others to solve it, my abilities are limited.

@YuriSizov YuriSizov merged commit 1886dee into godotengine:master Aug 2, 2023
@YuriSizov
Copy link
Contributor

Thanks!

@magian1127 magian1127 deleted the 4.0Fix78052 branch August 3, 2023 10:57
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.

AnimationTree(AnimationNodeStateMachine) error reported in the editor
2 participants