Don't mark cached previously pinned AnimationMixers #102813
Conversation
|
Weird the fix has nothing to do with AnimationMixers. |
AnimationPlayerEditor will hold on to pointers to no-longer existing Nodes that were previously pinned. Make sure to not mark them as dirty if they are not already inside the cache. This fixes godotengine#102108
| } | ||
| // If the current pinned node changed update both the old and new node. | ||
| if (node_cache.current_pinned_node != pinned_node) { | ||
| node_cache.mark_dirty(pinned_node); | ||
| node_cache.mark_dirty(node_cache.current_pinned_node); | ||
| // get_editing_node() will return deleted nodes. If the nodes are not in cache don't try to mark them. |
There was a problem hiding this comment.
Why not also fix the upstream issue and null original_node when player is cleared? I think the has checks are unnecessary with that + your additional fixes with nulling current_pinned_node
There was a problem hiding this comment.
Apologies I missed this comment before merging, otherwise I'd have waited for @hpvb to reply first.
There was a problem hiding this comment.
No worries, ultimately the current code only uses get_editing_node in two places and both would now be safe handling that pointer but it seems like it would be better to prevent that from being returned in the first place.
There was a problem hiding this comment.
Because this was a release blocker I decided to do a local fix. The current way the editor pinning works is kind of broken, editors can be pinned but the status of whether or not it is pinned is stored in the button toggle state, which gets reset when switching scenes.
The behavior is kind of silly as it is now, and was silly before any of my changes. I'm planning to do a ui fix pr separately to fix the pinning behavior in general.
|
Thanks! |
AnimationPlayerEditor will hold on to pointers to no-longer existing Nodes that were previously pinned. Make sure to not mark them as dirty if they are not already inside the cache.
This fixes #102108