foxssake
Replace property names with indexes in state sync
I can grab this after #278 is merged. After all, the keys often cost more bandwidth than their values lol
Replacing keys with indexes is very easy, but there is only 1 problem once strings are replaced with indexes: Adding new properties on runtime. No idea how that should be handled in detail.
When a new state property is added on runtime, the authority sends full states, until a new_property ack is received from the client. When a state property is removed on runtime, the authority sends full states, until a removed_property ack is received from the client. Does it sound good for a PR?
Or want me to just implement indexes as is, but without covering the above edge use-cases, so diff states will be incompatible with adding/removing state properties on runtime?
Also, since this feature will be done, and input has the same property format [tick][property] perhaps input could also be included in the same PR hmmmm.... Input indexes should be simpler than state indexes. And have the exact same logic.
I'd prefer to pick this up after #321, as it might bring some refactors on the internals of RollbackSynchronizer. Can't say exactly how much time it will take for a PR though, so if it takes too long, I'm ok with a PR. Will ping if that happens.
Ensuring that settings are synced ( if they change ) is up to the user. So for that case, I'd just reset the diff state variables, basically restarting the process - sending the initial full state and then continuing from there.
Good point on the inputs, we should update those to use indexes as well.
Also just thinking out loud for a bit - we could technically use hashes instead of indexes. Both would boil down to sending numbers, but properties having the same hashes between changes might make it more resilient, by reducing index mismatches. A drawback that needs to be considered is hash collisions - not sure how common these would be.
I'd prefer to pick this up after https://github.com/foxssake/netfox/issues/321, as it might bring some refactors on the internals of RollbackSynchronizer
Seeing #347 (realtime adding properties), I agree that you have the perfect insight to implement this cleanly and without bugs (and without back and forth :P )
Can't say exactly how much time it will take for a PR though, so if it takes too long, I'm ok with a PR
For my game, I'm in no rush for this as this is basically a feature for production/release to save bandwidth for the rented machine. And I'm moooooooooooooooonths away from that :sweat_smile:
So for that case, I'd just reset the diff state variables, basically restarting the process - sending the initial full state and then continuing from there.
Sounds simple and clean :+1:
Ensuring that settings are synced ( if they change ) is up to the user.
Let's confirm it after #347
Also just thinking out loud for a bit - we could technically use hashes instead of indexes. Both would boil down to sending numbers, but properties having the same hashes between changes might make it more resilient, by reducing index mismatches. A drawback that needs to be considered is hash collisions - not sure how common these would be.
Since a property - independent of its instance - is to have the same index/hash order, there shouldn't be any risk of hash collisions. So for example the brawlers at forest-brawl will always have the same indexes/hashes for their properties, regardless of how many players there are.
That said, the benefit of indexes would be that it's 1 byte per key, but godot sends 4 bytes (int) anyway, so using hash sounds like a better choice :+1:
Let's confirm it after https://github.com/foxssake/netfox/issues/347
Nothing to confirm. This is a design choice.
Since a property - independent of its instance - is to have the same index/hash order, there shouldn't be any risk of hash collisions.
Two property names in the same config have a chance to hash to the same value, resulting in two different properties having the same index.
That said, the benefit of indexes would be that it's 1 byte per key, but godot sends 4 bytes (int) anyway, so using hash sounds like a better choice 👍
An index is just a number, it has no inherent size. FYI, Godot's int is 64 bits, so 8 bytes. Though, an option is to limit the property count to 255, and send the indexes as a separate PackedByteArray followed by an array of values. This is for diff states only - for full states, we can simply send an array of values.
FYI, Godot's int is 64 bits, so 8 bytes.
AAAA
Though, an option is to limit the property count to 255, and send the indexes as a separate PackedByteArray followed by an array of values. This is for diff states only - for full states, we can simply send an array of values.
I love this idea. Literally optimal bandwidth usage, without bloating code with serialization.
Two property names in the same config have a chance to hash to the same value, resulting in two different properties having the same index.
As a use-case, hopefully I understand this correctly: RollbackSynchronizer.states has two property names, "movement" and "rotation". Let's say the hash for "movement" is 23423474273 and the hash for "rotation" is 234723747
With a hash being 8 bytes (Godot's int), the chances of hashes colliding is astronomically low imo. Especially since these hash collisions don't increase per node/instance, but only for new scenes with RollbackSynchronizers. That said, since there won't be that many properties for RollbackSynchronizer globally, could keep a list of all the property hashes in a static dictionary inside RollbackSynchronizer, to compare against newly created hashes.
Anyway, once this check is added, the code will be bloated compared to the PackedByteArray solution which has no such problems, and has far better bandwidth. The bandwidth is the goal of this PR, and after this is solved I will never care about bandwidth again, because the submit state/input has the most bandwidth cost in netfox (sending array of strings :face_holding_back_tears: )