refactor: scene object serialization consolidation and improved handling

[NoelStephensUnity]

This consolidates the NetworkObject serialization process for unified serialization and deserialization scene objects during both the approval and scene transitioning process.
Primary files to review over:
NetworkObject.cs
InternalMessageHandler.cs
NetworkSceneManager.cs
NetworkManager.cs --> HandleApproval method

Acceptance Test:
NetworkObjectSceneSerializationTests
This tests the primary functionality aspect of this PR which is that an invalid NetworkObject in a stream being de-serialized during approval or scene transitioning will not impact all NetworkObjects being de-serialized but just skip the invalid NetworkObject (including its NetworkVariable data) and continue processing the rest of the remaining NetworkObjects.

[mattwalsh-unity]

Nit, trailing space

[NoelStephensUnity]

This is no longer part of the PR (it was an artifact from an earlier merge of #749.

[mattwalsh-unity]

Always seems odd to me how in Unity editor code these raw offset numbers show up. I wonder, are the various uses of '5' the same '5'? As in could those be constants? And is the '10' this constant * 2?

[NoelStephensUnity]

This is no longer part of the PR (it was an artifact from an earlier merge of #749.

[0xFA11]

Suggested change

	//Write how much data was written
	writer.WriteUInt32((uint)(buffer.Position - currentStreamPosition));
	// Replace the placeholder 0 with how much data `WriteNetworkVariableData` wrote into the stream
	writer.WriteUInt32((uint)(buffer.Position - currentStreamPosition));

[0xFA11]

IIRC, the suggested order is internal static void instead of static internal void (minor)

[0xFA11]

Suggested change

	Debug.LogError($"Failed to spawn NetowrkObject for Hash {prefabHash}.");
	Debug.LogError($"Failed to spawn {nameof(NetworkObject)} for Hash {prefabHash}.");

[0xFA11]

very cool, ship it!

[mattwalsh-unity]

Thank you for the wonderful comments

[mattwalsh-unity]

This code wasn't here before, seems a bit in an addition to the refactor. Is this just an optimization to not send data if there's nothing to send? Could there be a comment on what this overall is doing?

[NoelStephensUnity]

The entire block of code associated with this:

 //Write whether we are including network variable data
  writer.WriteBool(NetworkManager.NetworkConfig.EnableNetworkVariable);
  
if (NetworkManager.NetworkConfig.EnableNetworkVariable)
{
    var buffer = writer.GetStream() as NetworkBuffer;
    // Mark our current position and set the network variable data size initially to zero (NOT as a packed value)
    var currentStreamPosition = buffer.Position;

    // Write variable data size placeholder initially as zero
    writer.WriteUInt32(0);

    //Write the network variable data
    WriteNetworkVariableData(buffer, targetClientId);

    //As long as something was written
    if (buffer.Position > currentStreamPosition)
    {
        //Get the new updated stream buffer position
        var updatedPosition = buffer.Position;

        //Move back to the position just before we wrote our size
        buffer.Position = currentStreamPosition;

        // Replace the zero value placeholder with the size of data written into the stream by WriteNetworkVariableData
        writer.WriteUInt32((uint)(buffer.Position - currentStreamPosition));

        //Revert the buffer position back to the end of the network variable data written
        buffer.Position = updatedPosition;
    }
}

Basically, if EnableNetworkVariable is true then we will be including the size of the NetworkVariable data written into the stream for the NetworkObject in question. We basically write a place holder (unpacked) UINT32 value, mark the position of the stream, then invoke WriteNetworkVariableData, and if the stream position changed we want to write the delta between the position prior to calling WriteNetworkVariableData and the current stream position. This delta we will then write into the place holder position (i.e. before the WriteNetworkVariableData was invoked) so upon de-serializing this entry we will know the exact size of the Network Variable data in the event the NetworkObject in question fails to be instantiated on the de-serialization end of things (typically the client).

This is the "improved handling" of scene object serialization such that if one NetworkObject fails to be instantiated we can simply "log a warning" and skip to the next NetworkObject in the scene.

I think I provide enough comments for this, but if you have any additional suggestions on how to better improve the comments I am certainly open to suggestions.

[mattwalsh-unity]

Ok, so it sounds like as part of the improved scene object handling, now when we provide the serialization for the object we also include the state of the its associated network variables. I assume this is to avoid timing problems such as we talked about previously?

If so, great; I'd put in a comment to the effect of "force synchronize the state of this object's variables immediately to avoid consistency errors, etc."

Lastly, should make sure @jeffreyrainy sees this for context when we go to snapshot sync

[NoelStephensUnity]

This only assists in the prevents completely asserting out of the current call-stack scope during scene synchronization. This would happen during the InternalMessageHandler.HandleConnectionApproved and during the NetworkSceneManager.OnSceneUnloadClient. The issue was that we couldn't just "skip an entry" in the serialized stream because we didn't know the size of the NetworkVariable data that was written into the stream for the NetworkObject in question that failed to instantiate on the client side.

[mattwalsh-unity]

Ok, this section seems new. That's cool, I just don't know what's going on to review it.

[NoelStephensUnity]

This makes sure that if one NetworkObject fails to construct (for whatever reason) then we can "skip past" that specific NetworkObject but continue processing any remaining serialized NetworkObjects as opposed to just throwing an exception and skipping the remaining (if any) NetworkObjects which was the original behavior.

[mattwalsh-unity]

Guess what I'm going to ask you to do :)

let's add the paragaph you just typed for me into the code

[mattwalsh-unity]

Had some questions, nice cleanup and comments tho