netfox icon indicating copy to clipboard operation
netfox copied to clipboard

Issues with RollbackSynchronizer taking input from multiple Nodes with different authorities

Open Bryce-Dixon opened this issue 1 year ago • 3 comments

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.

Bryce-Dixon avatar Aug 19 '24 12:08 Bryce-Dixon

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.

elementbound avatar Aug 19 '24 18:08 elementbound

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 avatar Aug 19 '24 19:08 Bryce-Dixon

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.

Bryce-Dixon avatar Aug 20 '24 20:08 Bryce-Dixon