feat: refactor tick interpolator to server pattern#602
Conversation
| ## Handles interpolation for multiple TickInterpolator nodes, storing snapshots | ||
| ## and applying interpolation based on the network tick factor. | ||
|
|
||
| class InterpolationGroup: |
There was a problem hiding this comment.
I'd prefer to flatten the groups into a few properties instead - a _PropertyPool that tracks what properties to interpolate, and probably two _Snapshots to track the two endpoints of the interpolation. This keeps it consistent with the other servers.
The rest of the methods would operate on subjects ( Nodes ), instead of groups.
| static var _logger := NetfoxLogger._for_netfox("InterpolationServer") | ||
|
|
||
| ## Register an interpolation group for a TickInterpolator node | ||
| func register_interpolation_group(group_id: int, root: Node, properties: Array[String], enabled: bool, enable_recording: bool) -> void: |
There was a problem hiding this comment.
We could limit the registration method to registering a property on a subject ( Node ), and manage the rest with setter methods ( e.g. set_recording(subject: Node, enabled: bool) ). Internally, the bool flags could be handled as _Sets, e.g. if a node is in the _recording_enabled set, then recording is enabled, otherwise not.
( The reason I keep referring to subjects instead of nodes is that there's a chance netfox will support recording, syncing, interpolating, etc. properties on any object, not just nodes )
| _groups[group_id] = group | ||
|
|
||
| ## Deregister an interpolation group | ||
| func deregister_interpolation_group(group_id: int) -> void: |
There was a problem hiding this comment.
We could keep this as deregister(subject: Node)
| group.is_teleporting = true | ||
|
|
||
| ## Interpolate properties for a group | ||
| func interpolate(group_id: int, factor: float) -> void: |
There was a problem hiding this comment.
Let's keep this as a private method, at least for the foreseeable future. We could also have a version that iterates over all the nodes and interpolates them. We could call that from NetworkTime.
My hope with shoving all these server calls into NetworkTime is fixing the order of operations, thus avoiding stuff breaking when you accidentally don't put TickInterpolator or RollbackSynchronizer at the bottom of your scene.
| if Engine.is_editor_hint(): | ||
| return | ||
|
|
||
| NetworkTime.before_tick_loop.connect(_before_tick_loop) |
There was a problem hiding this comment.
We could separate the pre- and post tick loop activities into discrete methods ( e.g. _clear_teleports(), _apply_target_state(), _record_next_state() ), and call them from NetworkTime.
Same rationale as with interpolate()
| if group.is_teleporting: | ||
| return | ||
|
|
||
| group.state_from = _PropertySnapshot.extract(group.property_entries) |
There was a problem hiding this comment.
I mentioned we could flatten groups into separate properties. I'm thinking we could keep the teleported subjects in a _Set, and then check that set during interpolation.
Or even better, we could keep a set of interpolated subjects and a set of teleported ones. When calling teleport(), we move the subject from the interpolated set to the teleported set. Once the tick loop is done, we reset both sets, either by adding the teleported subjects back to the interpolated set and clearing the teleported set; or clearing the teleported set, and repopulating the interpolated set from the property pool.
My thinking is that this might be somewhat faster than iterating every subject every time, at the cost of ( I think ) not that much extra code / complexity.
|
Supersedes #539 |
2 participants
Switch tick interpolation to use server pattern while keeping all original contracts.
Added unit and perf tests.