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

Adding nodes with a RollbackSynchronizer causes resimulation from tick -1 #175

Closed
Noobsaure opened this issue Dec 21, 2023 · 3 comments · Fixed by #204
Closed

Adding nodes with a RollbackSynchronizer causes resimulation from tick -1 #175

Noobsaure opened this issue Dec 21, 2023 · 3 comments · Fixed by #204
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@Noobsaure
Copy link

Noobsaure commented Dec 21, 2023

I encountered this issue while implementing periodic spawns of rollback-aware NPCs : each new RollbackSynchronizer still has a _latest_state set to -1 by the time _before_loop is first called, causing the _resim_from variable to be -1 for the corresponding _rollback call and thus resimulating everything from the start of the game (or history size, I didn't check, but it was tanking my FPS hard).

I worked around this by adding a _record_initial_tick function and calling it from process_settings as follows:

func _record_initial_tick(tick: int):
	# Broadcast state we own
	if not _auth_state_props.is_empty():
		var initial_state = {}

		for property in _auth_state_props:
			initial_state[property.to_string()] = property.get_value()
	
		if initial_state.size() > 0:
			_latest_state = max(_latest_state, tick)
			_states[tick] = PropertySnapshot.merge(_states.get(tick, {}), initial_state)
	
	# Record state for specified tick ( current + 1 )
	if not _record_state_props.is_empty() and tick > _latest_state:
		_states[tick] = PropertySnapshot.extract(_record_state_props)

It's more of a hack at this point but it works. I would be interested if there were a preferred solution to this.

@elementbound elementbound added bug Something isn't working good first issue Good for newcomers labels Dec 21, 2023
@elementbound
Copy link
Contributor

Good catch, ideally new nodes should start their history from their spawn tick. Thanks for providing a workaround as well! It's pretty sensible at first glance, but I'll look into it further.

@elementbound
Copy link
Contributor

Hey @Noobsaure, just pushed #204. Looks good from my testing, would you mind validating as well? Just swap RollbackSynchronizer with the new version.

@elementbound
Copy link
Contributor

Merged PR, but feel free to chime in with updates!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants