HouraiTeahouse / backroll-rs

A (almost) 100% pure safe Rust implementation of GGPO-style rollback netcode.
ISC License
351 stars 20 forks source link

Proposal: 'Command Buffer' pattern to replace 'Callbacks' structure #6

Closed 0----0 closed 3 years ago

0----0 commented 3 years ago

Managing a SessionCallbacks-implementing structure that is expected to be able to directly access the current game state can be somewhat non-ergonomic. However, passing control back and forth between Backroll and client code is not necessarily, strictly speaking, necessary. Backroll needs to tell client code "load this state", "advance with these inputs", "save this state to this buffer", etc, but this does not need to be done during Backroll's own processing.

Therefore, rather than immediately passing control back to client code, Backroll can instead create a list of 'commands' to the client, which the client can execute afterwards all at once. A rough mock-up of this use of the Command pattern might look like this:

// backroll definition
enum Command {
    Save(rollback_buffer_idx: usize),
    Advance(inputs, rollback: bool),
    Load(rollback_buffer_idx: usize)
};

// client code
let command_buffer: Vec<Command> = session.advance_frame();

for command in command_buffer {
    match command {
        Command::Advance(inputs, rollback) => {
            self.state.advance(inputs, rollback);
        }
        Command::Save(rollback_buffer_idx) { 
            session.save(self.save_data(), rollback_buffer_idx);
        }
        Command::Load(rollback_buffer_idx) {
            self.load_data(session.load(rollback_buffer_idx));
        }
    }
}

This match command { statement effectively replaces the role of the SessionCallbacks struct in describing the client's performance of Save/Load/etc operations, and it could be easier to understand and implement than a separate struct.

james7132 commented 3 years ago

Thinking on this a bit more, we should definitely not give an arbitrary save/load at a given index asa public interface, IMO it's too much of a footgun to have in the public API. I wonder if it's possible to wrap a SaveContext/LoadContext for writing a state directly to the state buffer in a safe way.

We'd also likely need to wrap Vec<Command> with a struct that implements Drop, or have Command implement Drop itself, to ensure that the save state was written to properly so that the invariants for the simulation are not broken by a bad client implementation.

There is a sizable amount of interdependent state that we should ensure does not get botched by this model. We'll may end up eagerly altering that state and breaking some invariant that is currently only enforced by debug_asserts.