Store connected clients and their data as entities #423

Shatur · 2025-02-22T14:06:16Z

This PR replaces ConnectedEntities, ReplicatedEntities, ClientEntityMap, and Messages resources with entities and data stored as components.

Connected clients are now represented by entities with a ConnectedEntity component. Connected entities become replicated after the insertion of ReplicatedClient (which replaces the StartReplication trigger). It's just a marker now, I've split the original ReplicatedClient into multiple components. Visibility and statistics are now accessible via the ClientVisibility and ClientStats components on replicated entities. The ClientEntityMap is now a component on replicated entities that stores mappings for them. ClientTicks is no longer accessible to users, but it isn't needed for the public API - even prediction crates don't use it.

UpdateMessage and MutateMessages are now crate-private components, just to speed up iteration. Using zip is slower, but if everything is on an entity, the performance is similar to what we had before this PR. It's still a bit slower, but I believe the improved ergonomics are worth it.

Backends are now responsible for spawning and despawning connection entities. If we merge this, it'll close #176 because it won't be possible to implement.

Since connected clients now have a unique identifier (Entity), we no longer need ClientId. Users were often confused by the conversion between Replicon's identifiers and their backend's identifiers. Now, backends can simply insert their identifier as a component on connected entities. Backends that already use entities as identifiers (such as aeronet) can just insert Replicon's components on their entities. This also gives us both fast iteration and fast lookup (closes #326).

~~The server ID is now Entity::PLACEHOLDER. Maybe we should provide an alias to make it more explicit, such as Entity::SERVER?~~ Added SERVER constant.

I also removed the ClientConnected and ClientDisconnected events. Users can observe for Trigger<OnAdd, ConnectedClient> or Trigger<OnRemove, ConnectedClient>. DisconnectReason is also gone. We don't care about it in Replicon and can't provide a nice representation for it anyway. Users can access proper information from the used backend. There is no need to duplicate it for us.

Originally I tried to provide backend-independend API. But it doesn't make much sense because users don't user multiple backends. And some API limit backend implementation in some way, such as our ClientId or DisconnectReason. Changes to entities fixes it.

Any suggestions (or even objections!) about the API or naming are welcome.

~~Requires #421 to be merged first for tic_tac_toe example.~~ Rebased.

codecov · 2025-02-22T14:21:40Z

Codecov Report

Attention: Patch coverage is 86.43216% with 27 lines in your changes missing coverage. Please review.

Project coverage is 85.84%. Comparing base (2aa25d5) to head (1e21749).
Report is 2 commits behind head on master.

Files with missing lines	Patch %	Lines
src/server.rs	81.53%	12 Missing ⚠️
src/core/replication/client_ticks.rs	89.18%	4 Missing ⚠️
src/core/event/server_event.rs	90.00%	3 Missing ⚠️
src/core/replicon_client.rs	62.50%	3 Missing ⚠️
src/core/event/client_trigger.rs	66.66%	2 Missing ⚠️
src/core/event/client_event.rs	83.33%	1 Missing ⚠️
src/server/replication_messages/mutate_message.rs	75.00%	1 Missing ⚠️
src/server/replication_messages/serialized_data.rs	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #423      +/-   ##
==========================================
+ Coverage   84.56%   85.84%   +1.28%     
==========================================
  Files          53       51       -2     
  Lines        2941     2756     -185     
==========================================
- Hits         2487     2366     -121     
+ Misses        454      390      -64

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Shatur

Left a few notes to simplify the review process.

Shatur · 2025-02-22T16:00:54Z

bevy_replicon_example_backend/examples/tic_tac_toe.rs

+///
+/// It's not replicated and present only on server or singleplayer.
+#[derive(Clone, Copy, Component, Deref)]
+struct PlayerClient(Entity);


I reworked examples to avoid replicating components identifiers.
Client don't need know them and it's extra traffic. So we use a separate component that associates spawned entities with connected clients.

Shatur · 2025-02-22T16:35:50Z

src/core/replicon_client.rs

-    packet_loss: f64,
-    sent_bps: f64,
-    received_bps: f64,
+    stats: NetworkStats,


I replaced with a struct since we use it as a component on connected entities.

Shatur · 2025-02-22T16:38:12Z

src/core/replicon_client.rs

    }

    /// Returns `true` if the client is connecting.
    ///
    /// See also [`Self::status`].
    #[inline]
    pub fn is_connecting(&self) -> bool {
-        matches!(self.status, RepliconClientStatus::Connecting)
+        self.status == RepliconClientStatus::Connecting


I added PartialEq since we no longer store ClientId in Connected.

Shatur · 2025-02-22T16:46:20Z

src/server/client_visibility.rs

@@ -7,64 +7,45 @@ use bevy::{
 use super::VisibilityPolicy;

 /// Entity visibility settings for a client.
+///


I moved this module under server since it's no longer needed in core.

Shatur · 2025-02-22T16:48:05Z

src/test_app.rs

+/// Inserted in [`ServerTestAppExt::connect_client`] and removed by [`ServerTestAppExt::disconnect_client`].
+/// Used to track which client corresponds to which connection.
+#[derive(Resource, Deref, Clone, Copy, Debug)]
+pub struct TestClientEntity(Entity);


Since clients no longer store ClientId in connected state, I need some mechanism to identify which client app belongs to which connected entity. This is why I introduced this resource.

Shatur · 2025-02-22T16:48:46Z

tests/connection.rs

-
-    let replicated_clients = server_app.world().resource::<ReplicatedClients>();
-    assert_eq!(replicated_clients.len(), 1);
+    server_app


Since it's no longer a trigger, I just check if ReplicatedClient is not inserted if replicate_after_connect is inserted.

Shatur · 2025-02-22T16:50:43Z

tests/insertion.rs

@@ -110,12 +110,10 @@ fn mapped_existing_entity() {

    server_app.connect_client(&mut client_app);

-    // Make client and server have different entity IDs.


Client and server already have different IDs due to observers and now connected clients.
So I just added an assertion that they are different below.

Shatur · 2025-02-22T16:50:53Z

tests/insertion.rs

-    // Make client and server have different entity IDs.
-    server_app.world_mut().spawn_empty();
-


Not needed for this test.

Shatur · 2025-02-22T16:51:34Z

tests/mutations.rs

@@ -997,6 +977,53 @@ fn confirm_history() {
    assert_eq!(event.tick, tick);
 }

+#[test]
+fn after_disconnect() {


I added a test to ensure that after disconnect mutation acks are dropped before processing.

Shatur · 2025-02-22T16:58:31Z

tests/server_event.rs

@@ -307,7 +306,7 @@ fn event_queue_and_mapping() {
                ..Default::default()
            }),
        ))
-        .add_server_event::<EntityEvent>(ChannelKind::Ordered)
+        .add_mapped_server_event::<EntityEvent>(ChannelKind::Ordered)


This should have been a mapped event. Test passed previously because we had the same entity identifiers.
Added an assertion below to prevent this from happening in the future.

Shatur · 2025-03-02T00:12:18Z

Opened projectharmonia/bevy_replicon_renet#24 to test how the new API works with bevy_replicon_renet.

UkoeHB · 2025-03-03T03:12:57Z

Unfortunately I don't have the mental bandwidth to do a proper in-depth review atm. My only comment is eliminating client ids seems like a bad move. Entity ids are essentially unintelligible keys into a big hashmap, not persistent network identifiers. If a client connects, disconnects, and reconnects, then they will end up having two different entity ids throughout the game's lifetime. Client id, on the other hand, clearly identifies the same client at all times.

For example, I have no idea how I would make bevy_replicon_attributes compatible with this PR, since it relies on client ids.

Shatur · 2025-03-03T08:51:32Z

Good point! What if we make client entities persistent? Instead of despawning them, we could just remove Replicon's components using remove_with_requires. Then, if the backend recognizes the same network identifier, it can re-use the same entity.

That's what Bevy does for gamepads. This will just require updating the docs in Replicon and adjusting the example backend implementation.

UkoeHB · 2025-03-03T19:59:25Z

What if we make client entities persistent?

This would be a memory leak for long-running game servers. It also doesn't solve how to make bevy_replicon_attributes work.

Shatur · 2025-03-03T22:10:03Z

This would be a memory leak for long-running game servers.

Maybe provide a setting to control the behavior (despawn or remove)? 🤔

It also doesn't solve how to make bevy_replicon_attributes work.

Right now, backends deterministically convert their client ID into our ClientId.
With this change, instead of ClientId, we just use Entity. If we reuse entities on reconnect, it should be functionally the same - just Entity instead of our ClientId. Or am I missing something?

It's possible to keep our ClientId by making it a component on connected entities and keep using it for things like events. However, we would need an additional resource for ClientId <-> Entity conversions. This would reduce ergonomics and require an extra lookup.

Another option is to use client entities everywhere and provide ClientId just as a way to identify the same client on reconnect. But I think just keeping the entities as I suggested above is simpler.

This is why I decided to reduce the number of ID types to just two: the backend's ID and Entity.

Utilizing the ECS to store connected clients opens up a lot of possibilities:

You can nicely replicate the list of connected clients from the server by just inserting Replicated on connected entities.
Users can attach their own game-related components.
Inspectable via existing tools and future editors.
Fast iteration and O(1) lookups via queries.
Works nicely with ECS features, such as triggers and possibly relations in the future.

This is why I'm trying to migrate to clients as entities in the first place.

Shatur · 2025-03-03T23:33:24Z

You can nicely replicate the list of connected clients from the server by just inserting Replicated on connected entities.

Users can attach their own game-related components.

Reworked the Tic-Tac-Toe example to showcase it 🙂

UkoeHB · 2025-03-03T23:44:19Z

You can nicely replicate the list of connected clients from the server by just inserting Replicated on connected entities.

This is only somewhat advantageous for a small subset of games. Most games you will know the client list before game start, and can include it in the game start pack.

Users can attach their own game-related components.

Game servers can spawn entities for clients as needed, providing bevy_replicon entities isn't a big advantage.

Inspectable via existing tools and future editors.

Resources are also inspectable.

Fast iteration and O(1) lookups via queries.

This appears to be an advantage, but lookups are only O(1) if you know the entity id. An entity lookup is equivalent in perf to a hashmap lookup, so we would get the same perf by storing a [ client id : index ] hashmap with the current vector-based design.

Works nicely with ECS features, such as triggers and possibly relations in the future.

Again, it's only nice if you know the entity id. Pretty much every single user will end up implementing a [client id : entity id] map resource, which kind of defeats the purpose.

Shatur · 2025-03-04T00:54:52Z

Most games you will know the client list before game start

For games with a matchmaking system - sure. But there are plenty of games where users can host, and the list of clients isn't known (mine included).

Resources are also inspectable.

You're right - we probably could have reflected our resources and added #[reflect(ignore)] for fields with implementation details.
It also requires Default for such fields, not sure if it's an issue. But I think it's cleaner with components.

so we would get the same perf by storing a [ client id : index ] hashmap

Sure, there are other ways to achieve this, ECS just provides it out of the box.

Pretty much every single user will end up implementing a [client id : entity id]

You mean for things like mapping player entities to controlled entities?
Yes, but if the client ID is Entity, you get relations for automatic cleanup and more optimized lookups with EntityHashMap.

Shatur requested a review from UkoeHB February 22, 2025 14:06

Shatur commented Feb 22, 2025

View reviewed changes

Shatur added 4 commits February 22, 2025 22:44

Store connected clients and their data as entities

2d903d4

Remove extra .iter() call

a4b6a42

Rename

bbdf346

Adjust docs

1241f5a

Shatur force-pushed the connected-entities branch from feb8e64 to 1241f5a Compare February 22, 2025 20:44

Shatur added 4 commits March 2, 2025 01:42

Add NetworkStats to prelude

8bfd791

Add dedicated constant for server

e301c0e

Fix log output for simple_box example

ef039fd

Do not use debug print for entities

d364aa5

Shatur added 2 commits March 3, 2025 00:41

Fix example doc link

c63ede3

Adjust naming

d54ba50

Shatur added 3 commits March 4, 2025 01:21

Document events

fb83066

Refactor Tic Tac Toe example

fd4a394

Adjust docs

066734c

Add name to ConnectedClient

1e21749

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Store connected clients and their data as entities #423

Store connected clients and their data as entities #423

Shatur commented Feb 22, 2025 •

edited

Loading

codecov bot commented Feb 22, 2025 •

edited

Loading

Shatur left a comment

Shatur Feb 22, 2025

Shatur Feb 22, 2025

Shatur Feb 22, 2025

Shatur Feb 22, 2025

Shatur Feb 22, 2025

Shatur Feb 22, 2025

Shatur Feb 22, 2025

Shatur Feb 22, 2025

Shatur Feb 22, 2025

Shatur Feb 22, 2025

Shatur commented Mar 2, 2025 •

edited

Loading

UkoeHB commented Mar 3, 2025 •

edited

Loading

Shatur commented Mar 3, 2025 •

edited

Loading

UkoeHB commented Mar 3, 2025

Shatur commented Mar 3, 2025 •

edited

Loading

Shatur commented Mar 3, 2025 •

edited

Loading

UkoeHB commented Mar 3, 2025

Shatur commented Mar 4, 2025

		@@ -110,12 +110,10 @@ fn mapped_existing_entity() {

		server_app.connect_client(&mut client_app);

		// Make client and server have different entity IDs.

		// Make client and server have different entity IDs.
		server_app.world_mut().spawn_empty();

Store connected clients and their data as entities #423

Are you sure you want to change the base?

Store connected clients and their data as entities #423

Conversation

Shatur commented Feb 22, 2025 • edited Loading

codecov bot commented Feb 22, 2025 • edited Loading

Codecov Report

Shatur left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Shatur commented Mar 2, 2025 • edited Loading

UkoeHB commented Mar 3, 2025 • edited Loading

Shatur commented Mar 3, 2025 • edited Loading

UkoeHB commented Mar 3, 2025

Shatur commented Mar 3, 2025 • edited Loading

Shatur commented Mar 3, 2025 • edited Loading

UkoeHB commented Mar 3, 2025

Shatur commented Mar 4, 2025

Shatur commented Feb 22, 2025 •

edited

Loading

codecov bot commented Feb 22, 2025 •

edited

Loading

Shatur commented Mar 2, 2025 •

edited

Loading

UkoeHB commented Mar 3, 2025 •

edited

Loading

Shatur commented Mar 3, 2025 •

edited

Loading

Shatur commented Mar 3, 2025 •

edited

Loading

Shatur commented Mar 3, 2025 •

edited

Loading