Add delta-compression using an operation log

[cBournhonesque]

Lighyear implemented delta compression by providing an API to compute diffs between two component states, keeping track of past component states and manually sending diffs from the last ACK-ed state for each client.

This was inefficient for the common case of sending a Vec<Point> or VecDeque<Point>, i.e. an appendable data structure, because we had to compute diffs in an expensive manner (diffs between 2 vecs) and we had to store the full vec state for a lot of past ticks.

This PR implements delta-compression with a different approach:

• the user has to provide methods to manually modify the component that they want to replicate, and use that in their code. This lets us avoid the expensive diff-computation form state since the user directly provides the diff methods!
• replicon can then send those diffs, along with an u64 cursor to make sure that a received diff is not applied multiple times.
  On the send-side: we only send the diffs since the last acked-diff.
  On the receive-size: we store received diffs in a buffer, and only apply them in order.

I've tried it in a personal game where I need to replicate Vec and I got ~80% reduction in bandwidth

[Shatur]

Sounds awesome, I like the idea!

I'll review tomorrow (it's 2:30 AM for me 😅).

[Shatur]

Wait, can't we already do this with ordered events? They're buffered by the backend and can represent operations. For example, what if, instead of using a special apply_op_delta, we register the component as "replicate once" and use server to apply operations?

One downside - right now this won't take visibility into account. If you use Broadcast, this will send the event and log an error on clients that don't see the entity on deserialization. But we can fix that: all events that touch entities implement the MapEntities trait and registered in a special way. We could use this to do "fake" entity mapping that actually just collects the entities touched by the event into a Vec. Then, before sending, we check whether each entity is visible (we have an API for it). This shouldn't be hard to implement. This should be a nice change in general because right now Broadcast is useless when you hide at least one entity.

What do you think about this? Am I missing something?

[cBournhonesque]

I think that in general it's preferred to keep all replication-related code in replication-land instead of message-land, since over time replication might accumulate more changes that only work with replication-messages.

Some issues with implementing this via messages:

• visibility is better handled via replication as you mentioned
• priority accumulation wouldn't work via messages
• I would have to special-case this in my code by reading FromServer messages and re-applying manually on the receiver side to get the correct component, whereas right now on the receiver side I can treat Points as a normal component. The only 'special' handling code is on the sender where I apply an op.
• Prediction/Interpolation rely on ServerMutateTicks to correctly know the ticks where all mutation messages were received. If these delta-ops are sent as separate ordered events they wouldn't impact ServerMutateTicks so we would also need to special-case this

• future replication-only logic might also not work (priority per component, authority change, etc.)

[Shatur]

Agreed. Let's proceed with your approach. I'll finish my review in a few minutes. And I'll open an issue about visibility, I think it's something that we should handle regardless.

I would have to special-case this in my code by reading FromServer messages

Everything you said makes sense, but I want to note that this part wouldn't require any special casing. You can send the event on the server instead of manually editing the component and apply the changes to using FromClient on both client and server. I.e. instead of defining a trait, you define a system/observer that applies the logic. Never mind, I think you're right about this one too.

[Shatur]

I like the idea, left a couple suggestions. Could you also update to the latest master?

[malfuu]

Hi! I think your solution is much stronger than what I was thinking. I tried going for a snapshot solution (think video encoding), by trading possible correctness for a smaller memory footprint (and leaving out diffs for variable length types). For my use case, I would have argued it had its worth; but I am also interested to see how it plays out with this solution.
Perhaps in the future I hope to argue for the snapshot strategy again.
I hope to test it out soon!

[cBournhonesque]

@Shatur should be ready to review!

[Shatur]

Thanks! Left a few small suggestions / questions.

[Shatur]

Left 2 last comments to the previous suggestions, but looks good overall. Once resolved, I'm planning to fix the CI and make a few minor changes myself (mostly just stylistic things and docs), don't want to bother you with this 🙂

[Shatur]

LGTM! Tomorrow I'll do the mentioned changes and merge it 🙂

[Shatur]

Got a bit busy with life stuff today, so I didn't finish, but pushed a few small changes. I'll wrap this up tomorrow 🙂

[Shatur]

Done! Notable changes that might affect you:

• PatchIndex is now a dedicated type that works similarly to tick, but wraps around u16. postcard uses varint encoding, so with the old approach, the first 128 indices would take 1 byte, while the rest would take 2+ bytes. With this approach, the index always takes only 2 bytes.
• PatchHistory and PatchBuffer are no longer components and are accessible from ctx. See the default_* functions. I also moved the default functions to the rule_fns module. Because of this change, we no longer need separate DiffFns and marker overrides.
• PatchIndex inside WireDiff::Snapshot is no longer optional. We always allocate an index if there is a change.
• PatchIndex inside WireDiff::Patches is now the last index, not the first. This makes the math easier and more consistent.
• External mutations are now handled properly by storing Tick. Previously, the detection was based on checking whether the number of missing patches was 0, but the client may not have acked an earlier patch.
• I split queue_and_take_ready into two functions to make tests more convenient: push and drain_ready.
• patches_after no longer allocates and now returns an iterator plus the current patch index. If the iterator is empty, the caller should fall back to a snapshot with the snapshot index. You can still get the first index by subtracting the number of received patches from it if needed.
• I moved some of the tests into unit tests and simplified the integration tests. I removed the test with need_history since it’s not really testing a Replicon feature. I'd suggest to make a similar inside Lightyear.
• Patch storage is now flat. One index now corresponds to one patch. I think this is more efficient and easier to understand.
• TestFnsEntityExt now works with patches. It's helper to test serialization functions.

I’d probably recommend taking another look at the PR diff. It's not big, most of the + lines come from tests. As you can see, there are almost no - lines now 🙂

Please double-check that this approach works for Lightyear.