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 TickInterpolator error when above interpolated property in scene tree #438

Merged
merged 4 commits into from
Mar 31, 2025

Conversation

kumpmati
Copy link
Contributor

@kumpmati kumpmati commented Mar 26, 2025

Hi! First of all I'm liking the addon, thank you for making it :-)

I noticed that TickInterpolator processes its settings inside _enter_tree() instead of _ready(), which results in an error when you add properties from nodes that are below TickInterpolator in the scene tree.

Example:
In this case I'm interpolating %Head.global_transform, which works but gives the below error at the beginning.
image
image

Changing _enter_tree to _ready seemed to work (correct me if there are any side effects to this), and it would have the benefit of making node order irrelevant.

p.s. the same pattern is also used in RollbackSynchronizer, which could also have the same issue.

Change `_enter_tree` to `_ready`
@DustieDog
Copy link
Collaborator

Looks like this was moved from _ready() to _enter_tree() in 3dc4e5c to fix an issue with signals. The proposed PR unintentionally reverts this fix.

Additionally, _ready() is only ever called once, whereas _enter_tree() is called every time a node enters the tree, so there's technically a difference there. Though it's not a big enough difference to matter in most cases, its current implementation may be preferred.

Does changing L121 to process_settings.call_deferred() inside _enter_tree() fix this issue for you?

@kumpmati
Copy link
Contributor Author

Looks like this was moved from _ready() to _enter_tree() in 3dc4e5c to fix an issue with signals. The proposed PR unintentionally reverts this fix.

Additionally, _ready() is only ever called once, whereas _enter_tree() is called every time a node enters the tree, so there's technically a difference there. Though it's not a big enough difference to matter in most cases, its current implementation may be preferred.

Does changing L121 to process_settings.call_deferred() inside _enter_tree() fix this issue for you?

Thanks for the quick reply. Your proposed solution seems to work, thanks! I modified my PR to do that instead.

@elementbound
Copy link
Contributor

Hey @kumpmati, thanks for the report and the PR! Could you please attach a minimum reproducible project for me to check this issue?
I wanted to see if this affects the other nodes too, but I've failed to reproduce this, even with original TickInterpolator code.

@kumpmati
Copy link
Contributor Author

Hey @kumpmati, thanks for the report and the PR! Could you please attach a minimum reproducible project for me to check this issue? I wanted to see if this affects the other nodes too, but I've failed to reproduce this, even with original TickInterpolator code.

Hi, here's a minimum reproducible demo of the issue: https://github.com/kumpmati/netfox-interpolator-bug-demo

@elementbound elementbound added the good first issue Good for newcomers label Mar 31, 2025
@elementbound
Copy link
Contributor

Thanks for the MPR @kumpmati. It seems like only TickInterpolator is affected, the synchronizer nodes either use call_deferred(), or don't even use the configs until the first tick loop.

Thanks for the effort, and congrats on your first contribution to netfox!

@elementbound elementbound merged commit 4c170a8 into foxssake:main Mar 31, 2025
2 checks passed
@kumpmati kumpmati deleted the fix/interpolator-init branch March 31, 2025 21:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants