bevy_replicon icon indicating copy to clipboard operation
bevy_replicon copied to clipboard

Use single event system for client events

Open notmd opened this issue 1 year ago • 3 comments

Fix #245

notmd avatar May 14 '24 16:05 notmd

I think we probably should make the API nicer by using two functions. First function will read the resource from the registry by ID (like you tried to do before, like Bevy does for updates), iterates over all events and passes each event into a second function with the correct type. And let user to override only the second function. The same for both sending and receiving.

I suspect that ClientEventFns will be the same as the upcoming ServerEventFns. So I would move it upper to the network_event module and rename into NetworkEventFns.

Also instead of passing the world, we probably should pass a context struct like we do for components. This context will contain the current tick and TypeRegistry.

Shatur avatar May 15 '24 21:05 Shatur

I think we probably should make the API nicer by using two functions. First function will read the resource from the registry by ID (like you tried to do before, like Bevy does for updates), iterates over all events and passes each event into a second function with the correct type. And let user to override only the second function. The same for both sending and receiving.

I'm not sure I fully understand, but doing that we need to store the ComponentId (maybe a few times), which seems unnecessary for the internal API. I suppose app can have maximum few hundred events, does this matter?

I suspect that ClientEventFns will be the same as the upcoming ServerEventFns. So I would move it upper to the network_event module and rename into NetworkEventFns.

The ServerEventFns have an additional pop_from_queue fn, so this is not applicable.

Also instead of passing the world, we probably should pass a context struct like we do for components. This context will contain the current tick and TypeRegistry.

I'm not get this, can you elaborate more?

notmd avatar May 16 '24 08:05 notmd

I'm not sure I fully understand, but doing that we need to store the ComponentId

I think that you can store it in the struct with functions. The idea behind it is to provide a nice API for users. Functions customization needed only to provide serialization/deserialization for events that doesn't have Serialize / Deserialize. For example, events with Box<dyn Reflect> inside.

The ServerEventFns have an additional pop_from_queue fn

Maybe we can unite pop_from_queue and receive? I think it will be possible with the mentioned two-functions approach.

I'm not get this, can you elaborate more?

Sure! Right now we pass &mut World into a single function. But let's consider separation into two functions for sending from server:

  1. Accepts &mut World, EventContext, current tick and channel ID. Obtains Events<T> from it, iterates over all, passes each T into second serialize function and sends the returned bytes over a channel ID to RepliconServer.
  2. Accepts T, EventContext, serializes, returns bytes. Similar for sending and client functions. Here EventContext is a struct with necessary context for serialization. I would store TypeRegistry and the current tick there. Take a look at functions for components API, it's similar. So the idea is to give user access only to necessary logic for serialization/deserialization instead of direct &mut World (maybe for sending we could use even only &World!). And instead of passing TypeRegistry and current tick directly, we also create a "context" struct with parameters. For consistency with components API and to avoid making breaking changes if we will need a new to pass more "context" data into functions.

Shatur avatar May 16 '24 08:05 Shatur

Superseded by #262.

Shatur avatar May 22 '24 00:05 Shatur