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

Issues with RollbackSynchronizer taking input from multiple Nodes with different authorities #236

Open
Bryce-Dixon opened this issue Aug 19, 2024 · 3 comments
Assignees

Comments

@Bryce-Dixon
Copy link

If a RollbackSynchronizer has input paths from multiple Nodes with different authorities, a "Invalid assignment on read-only value" error in _submit_input (rollback-synchronizer.gd:248) will be produced as the Dictionaries contained within _inputs are made read-only. Using Dictionary.merged may solve this?

There's also an issue where _latest_state gets way ahead of the client if they don't have authority over all of their RollbackSynchronizer input Nodes. For example, if I have an "Input" Node with the client as the authority and an "Events" Node with the server as the authority which both are used by the "Player" Node for _rollback_tick, the "Events" Node's inputs coming from the server cause the Player's state to be assumed as fully server-determined even though their local inputs should still be considered.

@elementbound
Copy link
Contributor

Hi @BtheDestroyer,

Thanks for raising this! I see that merging per property instead of replacing the whole input reference does bite us in _submit_input.

As for having multiple input nodes with different authorities, could you please expand on your use case? Rollback code was built with the assumption that all input nodes will be owned by the player, since they provide their own input.

@Bryce-Dixon
Copy link
Author

Bryce-Dixon commented Aug 19, 2024

I'm looking to send rollback-aware events from the server which could influence players' resignation. This includes assigning a random spawn point, spawning random items, and creating random environmental hazards. Since these are all random events, they need to be driven by the server and while I could pre generate a handful of events or share a seed with each client so they'll all end up generating the same random numbers, having the server report "this event is happening now!" and all clients respond to it is honestly both theoretically easier to implement and prevents cheating through predicting future.

I originally tried just giving a Singleton a RollbackSynchronizer to trigger resimulations if an event arrived late, but the problem there was that (as I understand):

  1. The player would be given their state for the last tick
  2. The "Events" Singleton would do its _rollback_tick, modifying the player
  3. The player does doesn't do its _rollback_tick since it's RollbackSynchronizer doesn't know it's changed
  4. The player's state wouldn't be recorded since it's RollbackSynchronizer doesn't know it's changed

Maybe if there was a way to signify to a RollbackSynchronizer "hey, another rewinder has changed your state, so you need to rewind as well" that would be sufficient?

@Bryce-Dixon
Copy link
Author

I think I've got a solution for my "server-side events" problem:

  1. Make the event variables part of RollbackSynchronizer's State
  2. Make the "Events" script use _rollback_tick instead of _gather
  3. "Events" only updates its variables if fresh: in _rollback_tick

Still unsure if this is the best option and I still think it makes sense for a RollbackSynchronizer with multiple Nodes with different authorities providing input to not completely break, but I wanted to share an option for other people who run into this issue.

@elementbound elementbound self-assigned this Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants