godot
godot copied to clipboard
godotengine
`SceneTree` Improve logic for skipping nodes removed during group calls
~Kinda reverts #67831 as now the checks within the group calls are done again by pointers~ Edit: #67831 was actually reverted in #69252 so now this PR just improves the current by pointer checks for the nodes removed during a group calls (alternative would be to change node groups to store nodes as ObjectIDs instead of pointers but haven't gone for that as it could lead to costlier iteration over them etc.). Now these by pointer checks are supposed to be always correct for each separate group call as each group call has its own call_skip which is updated properly by checking node IDs (which now are also stored for the to-skip nodes) only when a node is added/removed to/from the scene tree. There are more detailed explanations in the code comments.
Should fix #44765 (haven't tested/built this PR for 3.x). Fixes #67819 (tested 4.0 version of the MRP).
~Should fix #69159 (as well as duplicated #68769, #68774). However, I couldn't reproduce any of the reported crashes in the first place (but of course I do get why it could have been failing, and this PR is supposed to address what can fail in theory), hence I can't really test it. So anyone who experienced the crashes, please test this PR.~ Edit: After #69252 these crashes shouldn't happen anymore, and this PR shouldn't reintroduce them.
If it'll be approved/merged, I think it could be cherry-picked / backported to 3.x as well.
@akien-mga If you'd want to wait e.g. for reduz's approval on this but have the crashes caused by #67831 quick-fixed I think it would be fine to revert #67831 in the meantime.
Yeah I think doing a revert first and taking the time to review this change would make sense 👍
I guess you should update the OP to clarify the new state (re-fixes the reopened issues).
I guess you should update the OP to clarify the new state (re-fixes the reopened issues).
Yeah, I was already on that. :slightly_smiling_face: