Use Bevy state API for client and server states

[Shatur]

This allows users to utilize things like StateScoped and run systems more efficiently using OnEnter or OnExit without evaluating conditions every frame.

As a result, we now require StatesPlugin to be present. It's included by default in DefaultPlugins, but with MinimalPlugins you have to add it manually. In tests, I had to add StatesPlugin everywhere.

This also changes the behavior a little bit:

• On disconnect, we still apply all received messages and disconnect only after that, because the StateTransitions schedule runs right after PreUpdate. But I think this behavior is actually better.
• We no longer check if RepliconClient or RepliconServer can actually accept messages. I currently preserved the original behavior where we reset them after exiting Connected or Running. But it might be worth considering doing a reset on exiting Disconnected or Stopped to ensure a clean start, like we did with events. I would appreciate opinions on this.

[codecov]

Codecov Report

Attention: Patch coverage is 96.61017% with 2 lines in your changes missing coverage. Please review.

Project coverage is 81.69%. Comparing base (e1586e2) to head (4aead05).

Files with missing lines	Patch %	Lines
src/shared/backend/replicon_client.rs	80.00%	1 Missing ⚠️
src/shared/backend/replicon_server.rs	75.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #487      +/-   ##
==========================================
+ Coverage   81.16%   81.69%   +0.52%     
==========================================
  Files          52       51       -1     
  Lines        2989     2922      -67     
==========================================
- Hits         2426     2387      -39     
+ Misses        563      535      -28     

☔ 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.

[UkoeHB]

I appreciate what this PR is trying to do, but don't like the loss of precision it causes.

• In bevy_replicon_renet2 we have this sequence of systems. As soon as the backend connects a client, any packets that show up after that are immediately read into the replicon client. With this PR, there will be a 1-tick delay in reading those packets.
• In my bevy_girk backend, I have a delicate setup for advancing my own states. With this PR, the precision of that setup is reduced since every transition will waste a tick (and overlap with my custom states erroneously).

[UkoeHB]

This is not equivalent. client_just_disconnected also encompasses the connecting -> disconnected sequence.

[UkoeHB]

Also here

[Shatur]

Those are valid concerns! Let's see if it's possible to address them. I see 3 options:

• Manually run StateTransition, as suggested in the docs. But I think it's intended more for app code than libraries, and users might not expect their conditions to evaluate earlier.
• Add a custom schedule that runs between StateTransition and Update. This reduces parallelizm.
• Adjust the logic inside Replicon - for example, run message logic based on when messages are received rather than on the state. We could expose it via run conditions. So this will be an API to interact with messages and states will be for user logic.

I'm currently leaning toward option 3. Maybe you have other ideas? Closing the PR is also an option 🙂

[UkoeHB]

Adjust the logic inside Replicon - for example, run message logic based on when messages are received rather than on the state. We could expose it via run conditions. So this will be an API to interact with messages and states will be for user logic.

I don't think we should split behavior between replicon internals and the user-facing API. It can lead to subtle bugs and inefficiencies.

What we have right now works even though it's a bit ugly. So I'm in favor of closing the PR.

[Shatur]

I think I may have phrased it poorly - it shouldn't split the behavior. Let me expand this idea a little bit more:

1. Move the NetworkStats field from RepliconClient into a resource
2. With this change, RepliconClient and RepliconServer would only store raw message bytes. I'd rename them to ServerMessages and ClientMessages.
3. These structs would then be similar to buffered Bevy events, but with channels - so we can't use the event API. And I suggesting to process messages immediately upon receipt. And states would mirror the backend's server/client status (RenetServer/RenetClient in your case).
   What do you think?

But I'd implement steps 1-2 in a separate PR to minimize conflicts with open PRs.

[UkoeHB]

And I suggesting to process messages immediately upon receipt. And states would mirror the backend's server/client status (RenetServer/RenetClient in your case).

I don't see how this would be any different than this PR in terms of state change precision.

[Shatur]

Right now when connecting, you pass received messages to Replicon, but they are only applied when in a "ready" state (Connected for the client or Received for the server). The state update happens inside StateTransition which occurs after PreUpdate, but received messages are processed in PreUpdate. This creates a one-frame delay for messages received during connection.

I propose processing received messages whenever they arrive. I.e. inside the Receive state we would check if there is any received messages and if so - process it. I would expect it to work as before, you just schedule your systems inside ReceivePackets and they will be processed in Receive in the same frame.

[Shatur]

Tested the proposed idea locally - too weird 😅
Closing this PR for now.