Scene SyncList not synchronizing correctly for the host on second initialization

[NaolShow]

Important

General Unity version: 2022.3.11f1 Fish-Networking version: 4.3.5R Discord link: https://discord.com/channels/424284635074134018/1251846863720546334/1251846864949481563

Description When having a SyncList that is placed on a NetworkObject, that also gets populated in the OnStartServer method and the host does:

• Start as Host
• Stop
• Start as Host again The values added inside the OnStartServer will be synchronized in the ClientCollection of the synclist on the first start. But they will not be synchronized on further stop/start of the host.

This results in bugs when modifying the list (for example setting the value on an index that should be present) since SyncList Read method tries to access the index that is not contained in the ClientCollection.

Replication Steps to reproduce the behavior:

1. Have a SyncList on a NetworkBehaviour attached on a NetworkObject (as a scene object)
2. Populate the SyncList in the OnStartServer method
3. Start the network as Host (can use the NetworkHUDCanvas to help)
4. Stop the network as Host
5. Start the network as Host again
6. Try to modify the first index of the SyncList => Exception thrown because we are trying to access the index 0 of the ClientCollection of the SyncList. Realize that the ClientCollection is not synchronized. Values are present on the ServerCollection but not in the Client one.

Exception thrown from SyncList Read method:

ArgumentOutOfRangeException: Index was out of range. Must be non-negative and less than the size of the collection.
Parameter name: index
System.Collections.Generic.List`1[T].get_Item (System.Int32 index) (at <834b2ded5dad441e8c7a4287897d63c7>:0)
FishNet.Object.Synchronizing.SyncList`1[T].Read (FishNet.Serializing.PooledReader reader, System.Boolean asServer) (at Assets/FishNet/Runtime/Object/Synchronizing/SyncList.cs:355)
FishNet.Object.NetworkBehaviour.OnSyncType (FishNet.Serializing.PooledReader reader, System.Int32 length, System.Boolean asServer) (at Assets/FishNet/Runtime/Object/NetworkBehaviour.SyncTypes.cs:168)
FishNet.Managing.Client.ClientManager.ParseReader (FishNet.Serializing.PooledReader reader, FishNet.Transporting.Channel channel, System.Boolean print) (at Assets/FishNet/Runtime/Managing/Client/ClientManager.cs:489)
FishNet.Managing.Client.ClientManager.ParseReceived (FishNet.Transporting.ClientReceivedDataArgs args) (at Assets/FishNet/Runtime/Managing/Client/ClientManager.cs:384)
FishNet.Managing.Client.ClientManager.Transport_OnClientReceivedData (FishNet.Transporting.ClientReceivedDataArgs args) (at Assets/FishNet/Runtime/Managing/Client/ClientManager.cs:342)
FishNet.Transporting.Tugboat.Tugboat.HandleClientReceivedDataArgs (FishNet.Transporting.ClientReceivedDataArgs receivedDataArgs) (at Assets/FishNet/Runtime/Transporting/Transports/Tugboat/Tugboat.cs:238)
FishNet.Transporting.Tugboat.Client.ClientSocket.IterateIncoming () (at Assets/FishNet/Runtime/Transporting/Transports/Tugboat/Core/ClientSocket.cs:295)
FishNet.Transporting.Tugboat.Tugboat.IterateIncoming (System.Boolean server) (at Assets/FishNet/Runtime/Transporting/Transports/Tugboat/Tugboat.cs:211)
FishNet.Managing.Transporting.TransportManager.IterateIncoming (System.Boolean server) (at Assets/FishNet/Runtime/Managing/Transporting/TransportManager.cs:705)
FishNet.Managing.Timing.TimeManager.TryIterateData (System.Boolean incoming) (at Assets/FishNet/Runtime/Managing/Timing/TimeManager.cs:1017)
FishNet.Managing.Timing.TimeManager.IncreaseTick () (at Assets/FishNet/Runtime/Managing/Timing/TimeManager.cs:670)
FishNet.Managing.Timing.TimeManager.<TickUpdate>g__MethodLogic|98_0 () (at Assets/FishNet/Runtime/Managing/Timing/TimeManager.cs:342)
FishNet.Managing.Timing.TimeManager.TickUpdate () (at Assets/FishNet/Runtime/Managing/Timing/TimeManager.cs:332)
FishNet.Transporting.NetworkReaderLoop.Update () (at Assets/FishNet/Runtime/Transporting/NetworkReaderLoop.cs:28)

Example script (just press B to modify the first index):

using FishNet.Object;
using FishNet.Object.Synchronizing;
using UnityEngine;

public class TestSynclist : NetworkBehaviour {

    private readonly SyncList<int> syncList = new SyncList<int>(new SyncTypeSettings() {
        Channel = FishNet.Transporting.Channel.Reliable,
        ReadPermission = ReadPermission.Observers,
        WritePermission = WritePermission.ServerOnly
    });

    [SerializeField] private int syncListServerCount;
    [SerializeField] private int syncListClientCount;

    public override void OnStartServer() {
        syncList.Add(50);
        syncList.Add(75);
        syncList.Add(100);
    }

    private void Update() {
        syncListServerCount = syncList.GetCollection(true).Count;
        syncListClientCount = syncList.GetCollection(false).Count;

        if (Input.GetKeyDown(KeyCode.A)) {

            // Just add values
            // However, if starting, stopping then starting again the host, and adding those values
            // There will be 6 values in the server collection (from OnStartServer + here) and 3 values on the ClientCollection from here
            syncList.Add(50);
            syncList.Add(75);
            syncList.Add(100);

        } else if (Input.GetKeyDown(KeyCode.B)) {

            // Will crash when starting as host, stopping, starting again and then pressing B
            // => There should be the 3 values from the OnStartServer, but the ClientCollection of the synclist has 0 elements
            syncList[0] = 60;
        }
    }

}

Expected behavior Behavior should not change when stopping/starting the host again, and the SyncList should properly synchronize the ClientCollection from the ServerCollection. And our step 6 of modifying the first index should work, because the elements added in the OnStartServer should be in the ClientCollection.

[NaolShow]

Found a solution (might not be the right one, not experienced enough with FishNet codebase):

Issue

On the first time hosting, everything works because (in SyncList for example) ignoreReadChanges is false. So the changes in the ServerCollection are replicated on the ClientCollection for the host.

This underlying issue is from the SyncBase class, that contains "_lastReadDirtyId". On first time hosting, this value is set to "-1" which allows the ReadChangeId method (used to determine if the changes should be ignored or not) to return:

!reset && (id <= _lastReadDirtyId)

(reset is false for a Full Write, always the case, no problem here). But the _lastReadDirtyId IS NOT reset for the host when the network shutdowns

Which mean, on the second and latter starts, the value might not be -1 whereas the id resets. So the condition is true, and we ignore changes.

Solution

It appears that modifying the ResetState method of the SyncBase from:

        internal protected virtual void ResetState(bool asServer) {
            if (asServer) {
                _lastWriteFullLocalTick = 0;
                _changeId = 0;
                NextSyncTick = 0;
                SetCurrentChannel(Settings.Channel);
                IsDirty = false;
            } else {
                _lastReadDirtyId = DEFAULT_LAST_READ_DIRTYID;
            }
        }

to:

        internal protected virtual void ResetState(bool asServer) {
            if (asServer) {
                _lastWriteFullLocalTick = 0;
                _changeId = 0;
                NextSyncTick = 0;
                SetCurrentChannel(Settings.Channel);
                IsDirty = false;
            }
            // Reset the last read dirty id for everyone
            _lastReadDirtyId = DEFAULT_LAST_READ_DIRTYID;
        }

makes everything work again (from my quick tests). Is it the right solution? Might not be, but it seems strange to not reset this specific id for the server? (Errors could also appear in other Sync variants, since this issue is directly in the SyncBase)

Note

I'll not do a PR for now, since I do not know if this solution is valid or not. But I can do it in case its the right one.

[FirstGearGames]

Realistically you should only need to reset the Id for client. Perhaps asServer: false is not being called for any number of clientHost reasons. The server has no use for the lastReadDirtyId so resetting it with asServer: true is safe.

You can go ahead and submit a PR, there's no risk of your changes hurting anything. Thank you!

[NaolShow]

Thank you for your answer, No problem, I'll do a PR over the week end then!

[FirstGearGames]

Resolved in 4.3.8, release date is not known yet. You can still submit a PR for credit.

[NaolShow]

All good, as long as it is fixed. Thank you!