Skip to content

When a player sends a state RPC, the state is serialized, and on receiving the RPC it is deserialized.#254

Closed
TheYellowArchitect wants to merge 11 commits intofoxssake:mainfrom
TheYellowArchitect:state-serialization
Closed

When a player sends a state RPC, the state is serialized, and on receiving the RPC it is deserialized.#254
TheYellowArchitect wants to merge 11 commits intofoxssake:mainfrom
TheYellowArchitect:state-serialization

Conversation

@TheYellowArchitect
Copy link
Copy Markdown
Contributor

@TheYellowArchitect TheYellowArchitect commented Aug 26, 2024

Follow up to #251

Its basically the same, but with 1 more commit to add states, see 9205e78
Merge only after the above is merged. This specific commit requires more testing than the above.

I would love to request an update to the network monitor to learn how many bytes are received as ENET header is unknown :/

…d in future godot version and makes the code cleaner (saves 2 typecasts)
…on) and deserialization for received input properties.
if pe.node.is_multiplayer_authority():
_auth_input_props.push_back(pe)
else:
_record_input_props.push_back(pe)
Copy link
Copy Markdown
Contributor Author

@TheYellowArchitect TheYellowArchitect Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not critical enough to backport it to #251
From what I understood, _record_state_props records the properties of a node, regardless of authority.
_record_input_props was unused (yet I needed it for that exact purpose, to learn the properties of a node I don't have authority on, as the receiver), so I replicated the same as _record_state_props

But this opens a new issue. Why do _auth_input_props and _auth_state_props even exist?!
They are currently used exclusively as booleans, to get if we have authority or not.
And it's not like properties are added realtime - if they are, they could as well be added onto _record_state_props and _record_input_props without problems.

My suggestion is to straight up remove the variables _auth_input_props and _auth_state_props and replace them with booleans (e.g. node.get_multiplayer_authority()) unless I am missing something

else:
# Broadcast as new state
for picked_peer_id in multiplayer.get_peers():
_submit_state.rpc_id(picked_peer_id, state_to_broadcast, tick)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same thing as inputs. Locally, not a single RPC should be sent/received. Because we already have the state locally. So we must simply RPC to other peers, because for this tick we are RPCing, we already have set locally the variables latest_state, _states[tick]

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rpc's call_remote makes the old rpc call work no problems, so this is needless.

if (NetworkRollback.enable_input_serialization):
while _serialized_inputs.size() > NetworkRollback.history_limit:
_serialized_inputs.erase(_serialized_inputs.keys().min())
if (NetworkRollback.serialized_inputs_history_limit > 0):
Copy link
Copy Markdown
Contributor Author

@TheYellowArchitect TheYellowArchitect Aug 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

0 or -1 value is for saving all states/inputs (PackedByteArray format is optimal)
This is required for saving replays (well, the inputs at least, but now states are required instead until that .fire() issue is solved #253)

var state: Dictionary = PropertySnapshot.extract(_props)
_submit_state.rpc(state, tick)
var local_state: Dictionary = PropertySnapshot.extract(_props)
update_last_state_cache(local_state, tick)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested this class at all, nor am I familiar with it. But it should work since rollback synchronizer works with state serialization. The logic is quite simple, so I doubt I missed something.

@TheYellowArchitect
Copy link
Copy Markdown
Contributor Author

Closing in favour of #278 because this custom serialization doesn't currently support Strings + PackedTypedArrays (I could do it but it would double the serialization code), and since it is not in C++ (GDExtension) it costs performance.

Godot's serialization isn't that bad for the common use-case, it leads to simply double the bytesize so it's not the end of the world. The bandwidth is saved by diff states, regardless of the technique (custom serialization or not)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant