When a player sends an input RPC, the input is serialized, and on receiving the RPC it is deserialized.

[TheYellowArchitect]

Required for the implementation of a replay manager (#220)

Required for the followup PR to serialize/deserialize states as well #254 #264 (hence completing all of #157)

To explain how input serialization works, basically each input's first 4 bytes have the tick timestamp (int), then 5th byte is the size of the serialized properties' values.
So deserialization knows at what byte this input ends, and the next batched input begins (#221). Then 6th byte to the end is the values of the properties.

Since we know the properties and their types via

_record_input_props: Array[PropertyEntry]

and even their order, we only need to serialize their values, and deserialize them in the same order :)

There are 4 use-cases to test (all possible combinations of 2 booleans):

1. enable_serialization == true + enable_input_broadcast == true
2. enable_serialization == true + enable_input_broadcast == false
3. enable_serialization == false + enable_input_broadcast == true
4. enable_serialization == false + enable_input_broadcast == false

I tested locally and found no errors, even with delay 100ms, but ofc test these at least before merging (inb4 unit tests)

---

This PR is required for the below PRs:

• Serializing States When a player sends a state RPC, the state is serialized, and on receiving the RPC it is deserialized. #254 because all the serialization/deserialization code is already implemented here
• Serializing Diff States Add diff state compression #264
• Input Delay Added Input delay #261
• Further polishing input serialization by omitting the tick timestamps of each tick except the first one (since inputs are sent each tick, you need only the first input tick to accurately know the tick of the rest of the inputs received in a batch) Unify input serialization and input dictionary logic (e.g. validation) #266

[TheYellowArchitect]

This solves #250 by making use of that variable which is unused anywhere else, yet its obvious it exists for this reason.

As in, other players must know the node types/scenes at least, so when an input comes, it can be deconstructed. This deconstruction is impossible without having the properties of the RPC'd node.

Without this PR, that variable shouldn't even exist, but it exists for this purpose as explained above: For other players to know what its properties are (since it doesnt make sense for clients to have the same scenes, yet dont know what input properties they accept)

[TheYellowArchitect]

I am this close to making a godotengine pull request for this (basically static constant which returns the byte size of each variant), but even if I did, it would take a while for merge, and more time for netfox to update to latest version.

Anyhow, this class is required for all sorts of serialization, since the rollback properties of netfox are dynamic. The operations here are as cheap as they can be too. Except I guess the function serialize where I could cache the returned objects once they are once constructed. Anyway, encoding 3 floats (Vector3) shouldn't be noticeable.

[TheYellowArchitect]

It is bloat to run _submit_input() via an RPC locally. Because the desired result is setting _inputs[tick] - which is the first thing which happens at the local function _after_tick()

Invoking this RPC locally is negligent normally, but on serialization it's a serious flaw to locally deserialize the input, when it is already deserialized, and set to _inputs[tick] and _serialized_inputs[tick]

[elementbound]

Correct me if I'm wrong, but doesn't encode_var implement the same?

[TheYellowArchitect]

I haven't tried it thinking it was like bytes_to_var
Let me give it a try, because if true, that entire class is needless 👍

[TheYellowArchitect]

I am reading the godot code below because it seems encode_var includes the type at the header (so a u_int32 integer is 8 bytes instead of 4 bytes as in this PR)

if (buf) {
		encode_uint32(header, buf);
		buf += 4;
	}
	r_len += 4;

	switch (p_variant.get_type()) {
		case Variant::NIL: {
			//nothing to do
		} break;
		case Variant::BOOL: {
			if (buf) {
				encode_uint32(p_variant.operator bool(), buf);
			}

			r_len += 4;

		} break;
		case Variant::INT: {
			if (header & HEADER_DATA_FLAG_64) {
				//64 bits
				if (buf) {
					encode_uint64(p_variant.operator int64_t(), buf);
				}

				r_len += 8;
			} else {
				if (buf) {
					encode_uint32(p_variant.operator int32_t(), buf);
				}

				r_len += 4;
			}
		} break;

[TheYellowArchitect]

ok so in the godot internals, the header for strings (NodePath atm) is 12. Let me test this PR with Strings and PackedTypedArrays.
Because if this PR works with them (hadn't tried), then this PR is good. Otherwise, this custom serialization is needless, as it is automatically done by Godot's internals on sending the RPC.

[TheYellowArchitect]

I tested it, and it seems I forgot to take into account the string size, and PackedTypeArray sizes, so they are not serialized properly. For both, I would have to add code which is about serializing arrays, so the header would include the byte length of the content, alongside 1 bit for what type, in value-to-bytes.gd

instead of serializing manually and thus adding extra code to the repo.

I know you are against custom serialization code, so I have to ask: For GDScript serialization to be superior to Godot's Internals, it would have to be written in GDExtension to match its performance.
Assuming I were to make such a PR (this serialization code is moved to GDExtension, and I add new logic for the headers of Strings and PackedArrays), would you accept it? The code in RollbackSynchronizer will stay the same as this PR's. Feel free to deny for any reason, as it is a huge chore for me to write such a thing (I am a newbie on C++)

If it's size, then we can probably get away with much simpler solutions, like not transmitting the input property names, and just putting the values into an array.

It sounds an easy PR given it is part of #264 , but the real bandwidth saver is diff states, not just omitting some strings (properties). Only with diff states, custom serialization becomes overkill.

So, if I make a PR which adds diff states without serialization included (so, half of #264 lol), how do you suggest I solve the problem of the first full state, as explained in #264?

1. Reliable RPC for the first full state, and queue/cache the diff states until the first state is received (skim the code of Diff states fix for missing first full state #267 )
2. Authority sends full states (unreliable) until the client sends an ack that he has received a full state, then authority switches to sending diff states
3. Project settings option where the user puts the amount of diff states to send in fullly (e.g. 40) before sending diff states, so kinda like input redundancy

[elementbound]

I don't follow the point of this PR, please clarify.

From what I see, this PR introduces custom logic to serialize inputs into binary. What is the benefit?

If it's size, then we can probably get away with much simpler solutions, like not transmitting the input property names, and just putting the values into an array. If we ensure that these values are used in the same order on all peers, we basically achieve the same as this PR. The difference would be that we rely on Godot's internal serialization for transmitting data over RPC, instead of serializing manually and thus adding extra code to the repo.

If it's for the recording system, then I think it's a bit early for PR's like this. There is no agreement on how a replay system should look like, where should it reside ( core, dedicated module, something else ), how should it interact with the rest of netfox, or even what would be its exact feature set. With that said, I also don't think that we should change the rollback logic to cater for the replay system - these two are entirely separate systems, and shouldn't affect eachother.

For the dependant PR's:

• When a player sends a state RPC, the state is serialized, and on receiving the RPC it is deserialized. #254 seems to do the same but for state, so it doesn't necessitate this PR
• Add diff state compression #264 going by the title, part of the PR is serialization, which is not mandatory based on the above
• Added Input delay #261 not sure, will need to read through, but please clarify if that can only be done with custom serialization
• Unify input serialization and input dictionary logic (e.g. validation) #266 seems to apply the same thing to both dict and binary versions, so it doesn't necessitate this PR

[TheYellowArchitect]

If it's size, then we can probably get away with much simpler solutions, like not transmitting the input property names, and just putting the values into an array. If we ensure that these values are used in the same order on all peers, we basically achieve the same as this PR.

This is already achieved in this PR though 🤔
Instead of sending it as a generic Array (rip bandwidth), it sends it as PackedByteArray.

The benefit of this PR (and the subsequent ones) is on the size of the packets received in the PR. idk how to use wireshark but the difference is surely big, please give it a test and count the packets. This will prove or disprove that this serialization is preferred to Godot's internal serialization. Though, as mentioned in one of the comments var_to_bytes always has 4 bytes header, while here, with this serialization, it is 0. If an integer is 4 bytes, with Godot's internal serialization, adding the header it becomes 8, so in other words, most values serialized by Godot's internals are at least double the size they should be.
With the followup PRs (diff states), any game state can be supported, regardless of how many entities there are, bandwidth as a problem is completely erased.

Anyway, if it's easy for you to count the packet sizes (the real RPC packet sizes transmitted on the network, including the header which is omitted in Godot's parameters), it will prove if this PR is worthy or not. If it is not, I will implement that Array solution as a seperate PR (which reduces property sizes by a lot, as it omits the string keys) and then the diff states which is the true goal to reduce bandwidth.

like not transmitting the input property names, and just putting the values into an array. If we ensure that these values are used in the same order on all peers, we basically achieve the same as this PR.

This is implemented as part of #264 - since we know the properties - we know which property is received and which wasn't included. And the one which isn't included gets the value of the previous tick's property.

I also don't think that we should change the rollback logic to cater for the replay system - these two are entirely separate systems, and shouldn't affect eachother.

I fully agree. You had mentioned a new Node type at Netfox.extras which takes info from RollbackSynchronizer and I like that idea. Anyway, once the input issue (bombs) is fixed (#253), if you give me the design/UML, I can try implementing it.

#254 seems to do the same but for state, so it doesn't necessitate this PR

It uses all of the serialization logic from this PR - see the 2 classes under the serialization folder. Otherwise it wouldn't be such a small commit

#264 going by the title, part of the PR is serialization, which is not mandatory based on the above

It uses the same serialization code, and the diff states support both vanilla (dictionaries) and serialization (as done in this PR and #254 ) Anyway, made a PR without it: #278

#261 not sure, will need to read through, but please clarify if that can only be done with custom serialization

The real input delay is like 5 lines of code. But it depends on this PR because the input delay there also supports custom serialization for the inputs. Anyway, made input delay without this PR: #276

#266 seems to apply the same thing to both dict and binary versions, so it doesn't necessitate this PR

It is a cleanup of previous PRs to unify the input code for dictionaries and serialization, using the same logic instead of seperate as this one does (submit_serialized_input vs submit_raw_input), see how serialized states are handled into the same codeblock in #254 (which is what #266 does for inputs too, and this PR should have done as well - but didn't backport because it would lead to even more changes in the code)

[TheYellowArchitect]

This is probably needless (rpc_id to all other peers, instead of rpc) because @rpc(call_remote) does exactly that - calls an RPC to everyone except the invoker. I can confirm doing this at the state (#254) is futile since the host is always authority, haven't tested for input but should be the same for input as well.

[TheYellowArchitect]

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)