godot-extended-libraries / godot-next

Godot Node Extensions - Basic Node Extensions for Godot Engine
MIT License
957 stars 61 forks source link

Update Trail2D to check for target node before setting auto Z-index #105

Closed David1Socha closed 1 year ago

David1Socha commented 1 year ago

Fixes a bug that can occur with Trail2D when AutoZIndex is enabled and the Trail2D is set to target a parent node. Under those conditions, ZIndex will always be set to 0 (and since SetAsToplevel is used in Trail2D it's effectively a global level 0 z-index) instead of the expected Z index of _target.ZIndex - 1.

Because the Trail2D has its EnterTree() function called before the parent has been added to the tree, the initial attempt to set _target finds nothing and _target is null. There is a later check in _Process that attempts to set _target, but that does not happen until after the auto z-index is set. This PR adds a search for _target to the NotificationParented handler.

This issue does not happen with trail_2d.gd because in that file the setter for target_path is called when NotificationParented is received, and the target_path setter updates target. So I did not change anything with that file.

aaronfranke commented 1 year ago

I think we can just remove _target = GetNodeOrNull<Node2D>(targetPath); from _EnterTree, since now all places that need to read _target have it set before.

David1Socha commented 1 year ago

I think we can just remove _target = GetNodeOrNull<Node2D>(targetPath); from _EnterTree, since now all places that need to read _target have it set before.

Sounds good, I added a commit removing that line.

aaronfranke commented 1 year ago

Thanks!