HouraiTeahouse / backroll-rs

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

Naming Conventions: "Backroll" prefix redundant? #7

Closed 0----0 closed 3 years ago

0----0 commented 3 years ago

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.

james7132 commented 3 years ago

This is a bit of a holdover from the direct GGPO port where C doesn't exactly have namespaces/modules. Should be a pretty simple global %s/Backroll//g if anyone wants to tackle this.

seanchen1991 commented 3 years ago

I'd like to claim this 🙂

seanchen1991 commented 3 years ago

So it looks like there's a BackrollConfig in backroll/lib.rs, as well as a Config in backroll/sync.rs. I'm thinking that BackrollConfig should be renamed to Config and then changing the name of Config in sync.rs to something like PlayerConfig. Does that sound reasonable?

james7132 commented 3 years ago

For disambiguation, yes, that definitely should be renamed.