-
-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add diff state compression #264
Closed
TheYellowArchitect
wants to merge
14
commits into
foxssake:main
from
TheYellowArchitect:diff-states2
Closed
Add diff state compression #264
TheYellowArchitect
wants to merge
14
commits into
foxssake:main
from
TheYellowArchitect:diff-states2
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…d in future godot version and makes the code cleaner (saves 2 typecasts)
…on) and deserialization for received input properties.
…dded that logic to state synchronizer.
This was referenced Aug 31, 2024
Closed
Closing in favour of #278 which is far cleaner, has no dependencies, and also has implemented the 2nd fix recommended in the OP |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Followup to #254 (yes, it supports serialization)
Before a state is sent, it compares with the properties it had in the previous tick, and if they have the same value, these properties are not sent.
On receiving, the state is reconstructed accurately, because the very first state always sent is full state (no diffs), so all future diff states build off this initial state.
As for serialization, I added an integer (u_32) to the header which supports up to 32 variables/properties.
It's basically a bitfield, which on deserialization says which of the
state_props
indexes are contained in the serialization. And if contained, it deserializes the following value based on its type.There is a very rare use-case this PR bugs: If the packet of the very first state (full state, no diffs) is lost,(its possible because the RPC is "unreliable_ordered") then all future states received will bug too for that player's rewindable, because they build off the first state.
The solution is to make a new RPC where the full state is sent, "RELIABLE" so this will never bug, like so:
This simple solution however causes a race condition. What if the diff state arrives before reliable state 🫠
So the code must become more complicated with either of the solutions:
_submit_state
to store the states and run them only when the full state is received. Basically an array of dictionaries is added, and an if check if received full/initial state to run logic on them. Implemented in Diff states fix for missing first full state #267int
), namedsend_initial_full_states
where the default value is 40, and the first 40 states are sent fully instead of diff. Would end up in less bloated code than the first recommendation, but then again, needs 40 because a lagspike can otherwise cause problemsAfter all, if you send the next 10 states fully (instead of 40), and the player is on wifi and passes under a bridge or something so all internet connection is lost for 3 seconds, this won't work. But will "reliable" solution above work, or will it timeout?
I can do any once this is merged. Honestly, practically, this works no problems. A packet loss is rare, and a packet loss to happen at the very first state of a rewindable is very very very rare.
This PR is tested and confirmed to work for
rollback-synchronizer.gd
but I haven't tested my port tostate-synchronizer.gd
, I may do later as I'm after another PR.Whoever feels like testing
state-synchronizer.gd
with 3 players, high latency, and serialized and non-serialized, it would be helpful to ensure this PR works as intended.After this is merged, and users confirm it works properly, I suggest for serialization, to always have diffs, since the point of serialization is small size, which this achieves. Also serialization methods contain diff parameters. So the boolean only applies to default state sending (aka dictionaries)