johanhelsing / matchbox

Painless peer-to-peer WebRTC networking for rust wasm (and native!)
Apache License 2.0
889 stars 73 forks source link

Should peers be allowed to exchange metadata through the signalling server? #25

Closed johanhelsing closed 1 year ago

johanhelsing commented 2 years ago

@Jobieskii asked this in the matchbox intro blog comments: https://github.com/johanhelsing/johanhelsing.github.io/issues/11

The motivation behind this, is that often the peers want to exchange some extra information that is not necessarily game input. For instance seeds for rng, and chat before and after the game starts.

In the demo, and the extreme_bevy tutorial, this is currently hard to do, as the socket is simply handed to ggrs, and there is no way to use it to send other non-ggrs messages.

We should figure out if this is within scope for matchbox or not.

Technically, it could be done at many levels.

  1. Add some sort of broadcast event to the signalling server. This would be really simple to implement, but would increase traffic through the signalling server -> more expensive to run.
  2. Make it possible to open different sets of data channels for different parts of the app. One could unreliable and handed to ggrs, the others could perhaps be reliable (and be independent of the "ggrs" channel).
  3. Leave to some other service.
  4. Maybe it could be added to ggrs instead? Add optional support for rpc-like messages that happen sporadically with variable length?

@gschup What do you think?

gschup commented 2 years ago

Sending and receiving additional information before and after the match (or even during) might be a very handy feature, e.g. Character / map choices or chat messages during the session. This kind of information is rarely time-critical, so I would recommend sending them in a reliable way.

I would heavily lean against letting this stuff go over the signalling server. Also I don't think this is something that should be included in ggrs, which is supposed to be a rollback netcode library, nothing more.

Multiplexing one socket for multiple services can and has been done, but I think having a separate reliable channel would be much nicer. So I kind of like the idea of being able to create multiple separate channel(s) that are either reliable or unreliable. Does webRTC have support for this already?

johanhelsing commented 2 years ago

Yeah, webrtc supports multiple data channels. It should be the same API we're already using to create the unreliable channel, just with different config.

When I've tried to make a real, or I guess I should say more polished game, I keep coming back to this issue. I went down the road of creating a 3rd service, but for lots of things, it feels unnecessarily brittle and complex, when all we want to do is exchange some simple data. Having some sort of direct RPC-like communication certainly would be useful as well (things like chat for instance).

So in other words, I also feel like making it possible to open and access additional webrtc data channels would be the cleanest thing to do here.

I'm not sure about what the API, should look like, though... Currently we kind of assume a single unreliable connection to each peer. It's basically the project definition (to be udp for web), and I also want to avoid too much scope creep in this crate :/ At least I want to keep the basic API usage as simple as possible.

The current API is:

let (mut socket, loop_fut) = WebRtcSocket::new("ws://localhost:3536/example_room");
// wait
let new_peers = socket.accept_new_connections();

socket.send(packet, peer);

Currently peers are "accepted" once the data channel is established... I guess we could wait for a set of data channels instead... Perhaps something like:

let config = WebRtcSocketConfig {
    room_url: "ws://localhost:3536/example_room",
    data_channels: vec![
        // unreliable channel here
        // reliable channel here
    ],
    ..Default::default()
};
let (mut socket, loop_fut) = WebRtcSocket::new_with_config(config);
// wait
let new_peers = socket.accept_new_connections(); // spits out peers when all their data_channels are ready

socket.send(packet, peer); // sends on channel 0
socket.send_on_channel(0, packet, peer); // same as above
socket.send_on_channel(1, packet, peer); // sends on reliable channel

// ditto for receiving

The WebRtcSocketConfig::data_channel field would then default to have a single unreliable channel, so existing code would continue to work.

schicks commented 2 years ago

As long as it doesn't cost very much to set up an additional channel, it might be cleaner to always have a reliable channel and just add a send_reliable method. Matching up channel indices feels like a footgun.

zicklag commented 1 year ago

In my game ( not using matchbox ) I handled this by implementing a network client that had the following methods:

I use the reliable channel for things like selecting the player and level when starting up the game, etc. For one-shot messages that I just want to go through without getting dropped.

It works pretty well, but if you want any sort of channels you have to implement them manually.


I also had tried out a setup where all of the methods took a generic type argument for the type of message to send/recv. The type had to implement serialize and deserialize, and the client would create a channel for each type you wanted a channel for, so different parts of the game could send and receive only the kind of messages that they were interested in.

That was more complicated to setup, though, and you had to pre-register any types you wanted to send over the network.

I didn't end up needing the functionality so I took it out for now and just sent enums over the channels.

johanhelsing commented 1 year ago

I had some time to experiment with this, and with the new API from #64 for making extra channels we can indeed do more or less the same thing.

#[derive(Debug, Resource, Clone)]
pub struct MatchboxSocket(pub Arc<RwLock<WebRtcSocket>>);

impl ggrs::NonBlockingSocket<String> for MatchboxSocket {
    fn send_to(&mut self, msg: &ggrs::Message, addr: &String) {
        self.0
            .write()
            .unwrap()
            .send_to(msg, addr);
    }

    fn receive_all_messages(&mut self) -> Vec<(String, ggrs::Message)> {
        self.0
            .write()
            .unwrap()
            .receive_all_messages()
    }
}

impl MatchboxSocket {
    const RELIABLE_CHANNEL: usize = 1;

    pub fn send_reliable<T: Into<String>>(&mut self, packet: Box<[u8]>, id: T) {
        self.0
            .write()
            .unwrap()
            .send_on_channel(packet, id, Self::RELIABLE_CHANNEL);
    }

    pub fn new<T: Into<String>>(room_url: T) -> MatchboxSocket {
        let room_url = room_url.into();
        let config = WebRtcSocketConfig {
            room_url,
            channels: vec![ChannelConfig::unreliable(), ChannelConfig::reliable()],
            ..default()
        };
        let (socket, message_loop) = WebRtcSocket::new_with_config(config);

        let task_pool = bevy::tasks::IoTaskPool::get();
        task_pool.spawn_local(message_loop).detach();

        MatchboxSocket(Arc::new(RwLock::new(socket)))
    }
}

It's not the prettiest solution, but it works :)

In any case, the TL;DR anwer is: no, we will not allow metadata passing through the signalling server, as this can be handled just as well by the sockets themselves.