call RBs' _physics_rollback_tick(...) with tick rather than NetworkTi…#633
call RBs' _physics_rollback_tick(...) with tick rather than NetworkTi…#633jburdecki wants to merge 3 commits into
Conversation
…me.tick, so as to sim/resim with historically-accurate tick info. Separately, only apply state to valid (unfreed) RBs during rollback.
| set_body_states(rid, rid_states[rid]) | ||
| # Skip bodies freed (eg: despawned) since the snapshot was taken | ||
| if valid_rids.has(rid): | ||
| set_body_states(rid, rid_states[rid]) |
There was a problem hiding this comment.
Instead of the looping checks maybe we can clear out the object as soon as it's going to queue free.
Instead of node_removed being called from get_tree().node_removed.connect(node_removed) signal on line 16 we directly hook into the https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-signal-tree-exiting signal from the node itself.
We do this in node_added ( second last function in this class ) as the node is being registered.
There was a problem hiding this comment.
I like the idea of moving away from the loop. I changed it to a dictionary to improve lookup times.
Unfortunately though, to your point, tree_exiting() is called on any tree removal (including remove_child(), which could be a valid gameplay operation for moving RBs around a scene's tree/heirarchy). https://docs.godotengine.org/en/stable/classes/class_node.html#class-node-private-method-exit-tree And alternatively checking is_queued_for_deletion() would miss direct free() calls on RBs. So from what I see we're stuck without a proper (engine/solver-driven) event-based approach on this.
There was a problem hiding this comment.
Interesting. Are those really valid scenarios?
Calling free will definitely break netfox as will re-parenting.
@elementbound I think you've dealt with these life cycles a lot, what are your thoughts?
There was a problem hiding this comment.
Maybe they're not valid, if so, let's try the tree_exiting() proposal 👍 .
Curious on elementbound's thoughts here for (1.)
- I was thinking re-parenting could be a valid (or desired) gameplay operation (haven't tested/needed to in my current project though).
- I've never tried to manually
free()a body, have no need to, and personally don't think it's essential to support.
There was a problem hiding this comment.
So from what I see we're stuck without a proper (engine/solver-driven) event-based approach on this.
Exactly my experience. Due to lack of a better alternative, netfox considers exiting the tree as being destroyed. Unfortunately I had to make a choice, because after the rewrite, netfox needs to know when things are getting removed. Well either that, or every major system iterating its lists on every tick loop to filter out invalid nodes, which did not seem realistic nor performant to do.
So my suggestion is to keep that pattern, and treat the node exiting the scene tree as the node being destroyed. We can also consider re-adding it to whatever systems we need on _enter_tree().
- I was thinking re-parenting could be a valid (or desired) gameplay operation (haven't tested/needed to in my current project though).
I don't disagree, reparenting could be a legitimate use. Though, do objects need to exit the scene tree for that? Is it possible to travel directly from one parent to another?
- I've never tried to manually
free()a body, have no need to, and personally don't think it's essential to support.
Agreed. If someone really needs to immediately free() something, they can also call the servers directly to deregister. There's also no way to catch that from the outside.
There was a problem hiding this comment.
Due to lack of a better alternative, netfox considers exiting the tree as being destroyed.
Makes sense, thanks for clarifying. I will update to the tree_exiting() approach on next pass 👍
Though, do objects need to exit the scene tree for that? Is it possible to travel directly from one parent to another?
No (unless I can be shown how!).
Node::remove_child(...) always calls p_child->propogate_after_exit_tree(); which always calls emit_signal(SceneStringName(tree_exited));.
and Node::reparent(...) always calls Node::remove_child(...).
|
|
||
| # RIDs of tracked bodies currently in the tree. Snapshot entries whose | ||
| # body has been freed are skipped for state updates in rollback | ||
| var live_rids: Dictionary = {} |
There was a problem hiding this comment.
Sorry, only noticing this on second read. You can use netfox's _Set type for tracking a set of keys without (meaningful) values.
3 participants
Closes #632
Fixes
rollback_space(tick)trying to apply state to freed bodiestickavailable during rollback onNetworkRigidbodys. Now pass thetickbeing resimulated to RBs, rather than passing the currentNetworkTime.tick