Track sent components for each entity #604

Shatur · 2025-11-02T17:02:00Z

Objective

We'll need this for per-component visibility, but this change is useful on its own because it fixes replication for rules with multiple components that were inserted on different ticks.

For example, you have a replication rule for (A, B). You spawn an entity with A, and on the next tick you insert B. Without this change, B is replicated, but A is wrongly considered as sent. I added a test for it.

Implementation details

Inside ClientTicks, we now store a HashSet<ComponentId> for each entity in addition to ticks. To make it look nicer, I wrapped them in an EntityTicks struct. I also renamed mutations to entities because it fits better (it's not only for mutation ticks - it's also updated on insertions and removals). To avoid double hashing for mutations, I switched to using the entry API and made the field mutable. Because of this change, I also had to remove the is_new_for_client helper (in one place I need entry API and in another it's a simple contains check). I adjusted the doc comment and with the new naming it should still be clear.

MutateInfo now stores a Vec<ComponentId> for each entity, and I update the acknowledged components inside EntityTicks.

During removals, we now need to remove components from the acknowledged lists, so I adjusted the logic similarly to how we collect changes.

codecov · 2025-11-02T18:56:28Z

Codecov Report

❌ Patch coverage is 93.69748% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.81%. Comparing base (df63540) to head (c9815a0).
⚠️ Report is 1 commits behind head on master.

Files with missing lines	Patch %	Lines
src/server/client_pools.rs	75.00%	6 Missing ⚠️
src/server.rs	94.11%	4 Missing ⚠️
src/shared/replication/registry/component_mask.rs	88.88%	2 Missing ⚠️
src/server/removal_buffer.rs	90.00%	1 Missing ⚠️
src/server/replication_messages/mutations.rs	94.44%	1 Missing ⚠️
src/server/replication_messages/updates.rs	97.56%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #604      +/-   ##
==========================================
- Coverage   91.90%   91.81%   -0.09%     
==========================================
  Files          58       61       +3     
  Lines        3311     3397      +86     
==========================================
+ Hits         3043     3119      +76     
- Misses        268      278      +10

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

We'll need this for per-component visibility, but this change is useful on its own because it fixes replication for rules with multiple components that were inserted on different ticks. For example, you have a replication rule for `(A, B)`. You spawn an entity with `A`, and on the next tick you insert `B`. Without this change, `B` is replicated, but `A` is wrongly considered as sent. I added a test for it.

UkoeHB · 2025-11-04T22:14:12Z

src/server.rs

+                        for (_, components) in entities.drain(..) {
+                            pools.components.push(components);
+                        }


Change to method on pools that takes entities.drain(...) iterator as input.

Makes sense, already addressed in the hashmap replacement commit.

UkoeHB · 2025-11-04T22:14:51Z

src/server.rs

+                for (_, components) in mutate_info.entities.drain(..) {
+                    pools.components.push(components);
+                }


Change to method on pools that takes mutate_info.entities.drain(...) iterator as input.

UkoeHB · 2025-11-04T22:15:31Z

src/server.rs

+        for entity_ticks in ticks.entities.values_mut() {
+            entity_ticks.system_tick.check_tick(*check);
+        }


This logic should be inside ClientTicks.

It's a tiny loop and since we already expose the access to tick.entities, I think it reads better this way.

It's not only about readability. These functions are suffering from growing line bloat.

This function is only 9 lines.

It's collect_* functions that are large.

UkoeHB · 2025-11-04T22:27:33Z

src/server.rs

+                let Some(entity_ticks) = ticks.entities.get_mut(&entity) else {
+                    continue;
+                };
+                if !entity_ticks.components.remove(&component_id) {
+                    continue;
+                }


Should also filter on visibility.hidden(entity) to avoid redundantly sending component removals when visibility is lost (which will also send a despawn).

This executed after processing despawns, where the entity removed from ticks.entities if it was despawned or lost visibility.

Added a comment about it.

UkoeHB · 2025-11-04T22:41:46Z

src/server.rs

-                        client_ticks.mutation_tick(entity.id())
-                        && !ticks.is_added(change_tick.last_run(), change_tick.this_run())
+                    if let Some(entity_ticks) = client_ticks.entities.get(&entity.id())
+                        && entity_ticks.components.contains(&component_id)


Suggested change

&& entity_ticks.components.contains(&component_id)

&& entity_ticks.components.contains(&component_id)

Hide this behind a method that clearly reveals its meaning here: was the component added this tick or not?

We actually check whether client received the component. But the struct exposes full access to components, it's documented and I think entity_ticks.components.contains(&component_id) is quite clear. So I'd prefer to leave this as is.

it's documented and I think entity_ticks.components.contains(&component_id) is quite clear.

The meaning was not clear to me, and would be even less clear to anyone other than us two reading it.

Let me add a comment about it.
I think having entity_ticks.is_new(component_index) won't clarify things enough.

UkoeHB · 2025-11-04T22:50:37Z

src/server.rs

                    }
-                    ticks.set_mutation_tick(entity.id(), change_tick.this_run(), server_tick);
+
+                    match entity_entry {


Move all this to a method on ClientTicksEntity.

UkoeHB · 2025-11-04T22:53:09Z

src/server.rs

    }

-    signature.is_added() || ticks.is_new_for_client(entity)
+    signature.is_added() || !ticks.entities.contains_key(&entity)


Don't like removing clarity like this. Keep the method or use ClientTicksEntity.is_new().

Same as for contains above.

src/server/replication_messages/mutations.rs

src/server/replication_messages/updates.rs

UkoeHB · 2025-11-04T23:06:52Z

src/server.rs

+                                .extend(updates.drain_changed_entity_ids());
+                        }
+                        Entry::Vacant(entry) => {
+                            let mut components = pools.component_sets.pop().unwrap_or_default();


Suggested change

let mut components = pools.component_sets.pop().unwrap_or_default();

let mut components = pools.component_sets.pop().unwrap_or_default();

components.clear();

Either here or when pushing into the pool. Test to catch this?

We clear all data before pushing into the pool.
In the hashmap replacement commit I added a method that clears it.

Pushed a change that properly abstracts this, shouldn't be any confusion anymore.

Shatur · 2025-11-05T00:25:17Z

Pushed 2 commits.
First is a small, but related refactor. And second replaces all hashmap lookups with a bitmask check.

I'll address your suggestions in a moment.

Reduces the unsafety and the mount of passed arguments.

This significantly increases performances.

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

Since it's used in many places, it's better to abstract it to avoid forgetting to call `clear`.

Shatur requested a review from UkoeHB November 2, 2025 18:50

Shatur force-pushed the component-tracking branch from 800f5b6 to 2066211 Compare November 3, 2025 16:11

UkoeHB requested changes Nov 4, 2025

View reviewed changes

Add SerdeFns wrapper to simplify code

495328b

Reduces the unsafety and the mount of passed arguments.

Shatur force-pushed the component-tracking branch from a77a706 to 42d8392 Compare November 5, 2025 00:26

Shatur and others added 3 commits November 5, 2025 11:20

Replace hash lookups with a dynamically growing bitmask.

78f1cd0

This significantly increases performances.

Fix docs [skip ci]

fac569d

Co-authored-by: UkoeHB <37489173+UkoeHB@users.noreply.github.com>

Properly abstract EntityPools

34bb995

Since it's used in many places, it's better to abstract it to avoid forgetting to call `clear`.

Shatur force-pushed the component-tracking branch from a538b5f to 34bb995 Compare November 5, 2025 17:36

Shatur added 2 commits November 5, 2025 20:49

Add comments to clarify

11d66d0

Reuse existing function

0bd3b9f

Shatur requested a review from UkoeHB November 5, 2025 19:08

Move into function

4576d29

Shatur mentioned this pull request Nov 7, 2025

Abstract replication serialization #607

Merged

UkoeHB approved these changes Nov 8, 2025

View reviewed changes

Merge remote-tracking branch 'origin/master' into component-tracking

c9815a0

Shatur merged commit 4628b2e into master Nov 8, 2025
9 checks passed

Shatur deleted the component-tracking branch November 8, 2025 20:37

	&& entity_ticks.components.contains(&component_id)
	&& entity_ticks.components.contains(&component_id)

	let mut components = pools.component_sets.pop().unwrap_or_default();
	let mut components = pools.component_sets.pop().unwrap_or_default();
	components.clear();

Track sent components for each entity #604

Track sent components for each entity #604

Uh oh!

Conversation

Shatur commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

Implementation details

Uh oh!

codecov bot commented Nov 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shatur Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shatur Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Shatur commented Nov 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Shatur commented Nov 2, 2025 •

edited

Loading

codecov bot commented Nov 2, 2025 •

edited

Loading

Shatur Nov 5, 2025 •

edited

Loading

Shatur Nov 5, 2025 •

edited

Loading

Shatur commented Nov 5, 2025 •

edited

Loading