The library currently has a number of typenames prefixed with "Backroll," which seems to add a lot of name bloat. The examples seem to deal with this name bloat with use backroll::*;. Rather than typing something like

use backroll::*;
// ...
    let local_handle: BackrollPlayerHandle = session_builder.add_player(BackrollPlayer::Local);

would it be more idiomatic to leave the namespace intact and use

    let local_handle: backroll::PlayerHandle = session_builderr.add_player(backroll::Player::Local);

to avoid conflicts?

I'm not super familiar with Rust naming conventions, so consider this a suggestion, but it seems like this is the convention among other Rust libraries, with names like Context, Sprite, etc going un-prefixed.

This PR renames the following:

• BackrollConfig to Config
• BackrollPlayer to Player
• BackrollPlayerHandle to PlayerHandle
• BackrollEvent to Event
• BackrollPeer to Peer
• BackrollPeerConfig to PeerConfig
• BackrollSync to Sync
• backroll::sync::Config to PlayerConfig
• bevy_backroll::BackrollStageCallbacks to StageCallbacks

To avoid naming conflicts in protocol/mod.rs, transport::Peer is namespaced as TransportPeer and protocol::Event is namespaced to ProtocolEvent.

I also took the liberty of addressing default clippy warnings.

Let me know if any of these changes aren't acceptable 🙂

There's a comment saying "to avoid potentially introducing non-deterministic simulation results", but is there a specific unfixable issue? It seems to me that this should be at least configurable, since it should be possible for all the systems to be completely deterministic.

I apologize if this is documented and I just missed it, but I feel that this should be configurable or have a more descriptive comment saying why it has to be single threaded.

Currently in Backroll, gamestates that get saved and loaded HAVE to be able to be hashed. This leads to a lot of issues, since important gamestate stuff like floats and vectors aren't able to be hashed. This should be replaced with something more universal. Note: as far as I know I think this issue is with bevy_backroll but it could extend to the original backroll library as well

Commit cf0b896c7e56c7916aceb8595fd4cdb04df44ea8 fixed #12 but replaced the SaveState ring buffer with a frame-keyed hashmap instead. This adds the overhead of clearing out the HashMap of old states each time a frame is confirmed by a remote, and potentially allocates more on the heap each time a new state is saved. This indirection is likely not particularly cache friendly.

However, given this HashMap should normally never be more than 8-20 entries large at a given time, this may be a moot point in terms of performance.

Leaving this as an issue to revisit this when time permits.

Whenever the prediction barrier gets reached due to an influx of inputs, there will be a bunch of jitters, which leads to the players' positions in this sample project getting desynced. The current theory is that the inputs during that frame don't get read.

This implements the proposal from #6.

• Moves SavedFrame behind a Arc<Mutex<_>> wrapper called SavedCell for safely passing back to the API consumer in the form of Commands. This should allow a safer targeted control
• SaveState and LoadState are made to be single-use write-through structs that allow loading and saving game state in a controlled manner.
• SaveState implements Drop in debug mode only, which will panic if the provided client implementation did not write out to its SavedCell before dropping it.
• Replaced all &mut impl SessionCallbacks<T> with &mut Vec<Command<T>> instead.

This has the added benefit of removing another generic parameter from all of the associated functions, so compile times should be faster.

The public facing API is simpler, is more idiomatic than the callbacks being used from before, and removes the nasty lifetime interactions it had with reference based callbacks from before.

Currently missing:

• [x] a newtyped Commands and CommandsIterator types that strictly forbid all operations other than non-consuming iteration and sequential construction to ensure correctness on both the client and Backroll's ends.

Still a draft. There are some notable parts that require alteration to help prevent users of the API from shooting themselves in the foot.

P2PSession is not polling the Player reciever, which means that the queues of network events are not being forwarded to the top level. This usually ends in a prediction limit hit.

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.

Bumps actions/cache from 2 to 3.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Updates the requirements on serial_test to permit the latest version.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

Updates the requirements on serial_test to permit the latest version.

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

As of 014fd15da35a74a3fed620f44d0f5bd68a506f48, bevy_backroll is now saving the provided state without any hashing/checksum. This could lead to silent desyncs as no checksum checks will fail in these cases. We should look for a generic way of handling this without reintroducing the Hash bound on Config::State.

Thank you for this, looks awesome!

Could we get a minimalistic bevy app example that uses bevy_backroll? I think that would help getting into the whole networking and backroll topic.

See:

https://github.com/HouraiTeahouse/backroll-rs/blob/3c6bdc08a17d2f4e87524595a935d7f28de8e7a4/backroll/src/protocol/mod.rs#L38

which is used for estimating the remote player's frame for timesyncing purposes:

https://github.com/HouraiTeahouse/backroll-rs/blob/3c6bdc08a17d2f4e87524595a935d7f28de8e7a4/backroll/src/protocol/mod.rs#L740-L753

On an unrelated note, it that calculation correct? The comment seems to suggest round_trip_time.as_secs() / 2.

There are a notable amount of the application level protocol that belongs in a lower level implementation. The heartbeat, the initial synchronization, the magic number management, sequence number, and disconnect timeout are all to establish a unreliable but ordered connection, something Laminar and other reliable UDP implementations already support. This should exist at the backroll-transport layer. A good amount of this is already established in amethyst/laminar#290, and this change be considered a step towards implementing backroll-transport's connection model in Laminar.

• [ ] Create a wrapper around Peer for ordered unreliable sends. OrderedPeer?
• [ ] Add a way to notify the main game of incoming connections.
• [ ] Alter Peer to notify either end of a network interruption.
• [ ] Move "Synchronizing, Running, Interrupted, Disconnected" state machine to the transport layer.
• [ ] Move heartbeat task into OrderedPeer