FirstGearGames / FishNet

FishNet: Unity Networking Evolved.
Other
1.37k stars 146 forks source link

SyncLists not persisting values for scene networked objects #761

Closed NaolShow closed 1 month ago

NaolShow commented 2 months ago

General Unity version: 2022.3.11f1 Fish-Networking version: 4.4.2R Pro Discord link: Talked a bit on the open-development channel

Description On networked scene objects (only scene ones, not happening on dynamically spawned network object), the Synclists are not persisting their changes (locally and on network). If you add an object in the list for example, and then directly print the list size you will have the correct value, however on the next ticks the list is going to be empty.

This is happening locally AND on network for other players. However, this works fine for dynamically networked objects.

This bug was not present in 4.3.8R.

Replication

Sample code to test the behaviour:

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

// Simple test script to show the sync list error
// => Attach this script to a network object in a scene, then start the network and see the logs
// => Count will be '1' on addition, and then for the next ticks 0 (seen in the inspector)
public class SyncListBugTest : NetworkBehaviour {

    private readonly SyncList<int> list = new SyncList<int>();

    // Just a counter that shows in the inspector, readonly
    [SerializeField] private int listCount = 0;

    public override void OnStartNetwork() {
        base.OnStartNetwork();

        if (IsServerInitialized) {
            list.Add(17);
            Debug.Log($"List size is {list.Count} on the same tick of the addition.");
        }

    }

    // Update the list size in the inspector for debug purposes
    void Update() {
        listCount = list.Count;
    }
}

Expected behavior When adding an object in the synclist it should be persisted, and not lost on subsequent ticks.

DevDavey commented 2 months ago

I noticed the same behaviour is happening on SyncVars too with 4.4.2R. This only happens when they are modified within OnStartNetwork() or OnStartServer().

If the value is changed elsewhere like Awake() they change fine. Perhaps this is the same issue.

NaolShow commented 2 months ago

I noticed the same behaviour is happening on SyncVars too with 4.4.2R. This only happens when they are modified within OnStartNetwork() or OnStartServer().

If the value is changed elsewhere like Awake() they change fine. Perhaps this is the same issue.

Ahh now that you mention this, in fact this works for my inventory system in flexible mode (no slots by default, add new items after the initialization). But does not work on the fixed version where I instantiate empty Items in the OnStartServer.

So yeah this might be linked to the initialization of the sync variables

FirstGearGames commented 2 months ago

Most likely linked and it's been resolved. If you have access to the proget you can pull the changes but if not they're coming out tonight anyway

On Thu, Aug 22, 2024, 1:45 PM Loan Jouhannet @.***> wrote:

I noticed the same behaviour is happening on SyncVars too with 4.4.2R. This only happens when they are modified within OnStartNetwork() or OnStartServer().

If the value is changed elsewhere like Awake() they change fine. Perhaps this is the same issue.

Ahh now that you mention this, in fact this works for my inventory system in flexible mode (no slots by default, add new items after the initialization). But does not work on the fixed version where I instantiate empty Items in the OnStartServer.

So yeah this might be linked to the initialization of the sync variables

— Reply to this email directly, view it on GitHub https://github.com/FirstGearGames/FishNet/issues/761#issuecomment-2305310758, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGPJC3RITG5HXP77LWRPZ33ZSYPSHAVCNFSM6AAAAABM33WSBWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBVGMYTANZVHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

FirstGearGames commented 2 months ago

Sorry, ignore the typos. I was using voice to text since I am on mobile. Pro git has fix, will likely be released tonight.

On Thu, Aug 22, 2024, 1:54 PM Ben Daigle @.***> wrote:

Most likely linked and it's been resolved. If you have access to the proget you can pull the changes but if not they're coming out tonight anyway

On Thu, Aug 22, 2024, 1:45 PM Loan Jouhannet @.***> wrote:

I noticed the same behaviour is happening on SyncVars too with 4.4.2R. This only happens when they are modified within OnStartNetwork() or OnStartServer().

If the value is changed elsewhere like Awake() they change fine. Perhaps this is the same issue.

Ahh now that you mention this, in fact this works for my inventory system in flexible mode (no slots by default, add new items after the initialization). But does not work on the fixed version where I instantiate empty Items in the OnStartServer.

So yeah this might be linked to the initialization of the sync variables

— Reply to this email directly, view it on GitHub https://github.com/FirstGearGames/FishNet/issues/761#issuecomment-2305310758, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGPJC3RITG5HXP77LWRPZ33ZSYPSHAVCNFSM6AAAAABM33WSBWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBVGMYTANZVHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

NaolShow commented 2 months ago

Hi, thank you for the update, indeed it fixed the values not persisting. However, it seems to still be the case when modifying a sync list afterwards. (Internal collection has a length of 0), here's a script to test that, same protocol as before:

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

// Simple test script to show the sync list error
public class SyncListBugTest : NetworkBehaviour {

    private readonly SyncList<int> list = new SyncList<int>();

    // Just a counter that shows in the inspector, readonly
    [SerializeField] private int listCount = 0;

    public override void OnStartNetwork() {
        base.OnStartNetwork();

        if (IsServerInitialized) {

            // Here there is no problem
            list.Add(17);
            Debug.Log($"List size is {list.Count} on the same tick of the addition.");
            list[0] = 9;
            Debug.Log($"List size is {list.Count} on the same tick of the addition and modification");
        }

    }

    // Update the list size in the inspector for debug purposes
    void Update() {
        listCount = list.Count;

        // On subsequent ticks this throws on host (did not try on remote clients)
        list[0] = Random.Range(0, int.MaxValue);

    }
}

It throws in the internal protected override void Read(PooledReader reader, bool asServer) method in the Set section:

                //Set
                else if (operation == SyncListOperation.Set) {
                    index = reader.ReadInt32();
                    next = reader.Read<T>();
                    if (!ignoreReadChanges) {
                        Debug.Log($"{collection.Count}"); // gives '0' even though there is an item
                        prev = collection[index];
                        collection[index] = next;
                    }
                }
FirstGearGames commented 2 months ago

Can you let me know if this is still a problem in 4.4.5, and if so update if you are receiving this issue as clientHost, client only, so on. Release was planned for today but there's something else that needs to be looked into first. Hopefully very very soon though.

NaolShow commented 2 months ago

Sure, I'll do complete tests on the week end, can't have time before sorry. Thanks for the fixes and for the message!

NaolShow commented 1 month ago

Can you let me know if this is still a problem in 4.4.5, and if so update if you are receiving this issue as clientHost, client only, so on. Release was planned for today but there's something else that needs to be looked into first. Hopefully very very soon though.

So after some tests, it seems to have fixed the issue, however, now even with the simplest setup and the test script that I gave in the conversation there is a lot that is getting spammed in the console:

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:399)
FishNet.Object.NetworkBehaviour.ReadSyncType (FishNet.Serializing.PooledReader reader, System.Int32 writtenLength, System.Boolean asServer) (at Assets/FishNet/Runtime/Object/NetworkBehaviour.SyncTypes.cs:183)
FishNet.Managing.Client.ClientObjects.ParseSyncType (FishNet.Serializing.PooledReader reader, FishNet.Transporting.Channel channel) (at Assets/FishNet/Runtime/Managing/Client/Object/ClientObjects.cs:294)
FishNet.Managing.Client.ClientManager.ParseReader (FishNet.Serializing.PooledReader reader, FishNet.Transporting.Channel channel, System.Boolean print) (at Assets/FishNet/Runtime/Managing/Client/ClientManager.cs:487)
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:1043)
FishNet.Managing.Timing.TimeManager.IncreaseTick () (at Assets/FishNet/Runtime/Managing/Timing/TimeManager.cs:708)
FishNet.Managing.Timing.TimeManager.<TickUpdate>g__MethodLogic|100_0 () (at Assets/FishNet/Runtime/Managing/Timing/TimeManager.cs:376)
FishNet.Managing.Timing.TimeManager.TickUpdate () (at Assets/FishNet/Runtime/Managing/Timing/TimeManager.cs:366)
FishNet.Transporting.NetworkReaderLoop.Update () (at Assets/FishNet/Runtime/Transporting/NetworkReaderLoop.cs:28)
FirstGearGames commented 1 month ago

In 4.4.5 I applied a different fix which should resolve this. You MUST have Beta enabled in the Fish-Networking menu to use the new fixes; it's enabled by default.

Let me know! Thanks.

NaolShow commented 1 month ago

Hi,

I've reinstalled with the latest beta version 4.4.5-PR-3, double checked if beta is enabled (should be, since I have the "Switch to stable" in the Tools > Fish-Networking submenu.

And sadly the issue is still there, host still gets this error spammed in the console.

FirstGearGames commented 1 month ago

4.4.5 is out of pre release. Try the latest version, which is 4.4.5 (without pre release).

On Sat, Sep 14, 2024, 3:50 AM Loan Jouhannet @.***> wrote:

Hi,

I've reinstalled with the latest beta version 4.4.5-PR-3, double checked if beta is enabled (should be, since I have the "Switch to stable" in the Tools > Fish-Networking submenu.

And sadly the issue is still there, host still gets this error spammed in the console.

— Reply to this email directly, view it on GitHub https://github.com/FirstGearGames/FishNet/issues/761#issuecomment-2350899102, or unsubscribe https://github.com/notifications/unsubscribe-auth/AGPJC3VTCNPS2FB7R5VLU2LZWPTFNAVCNFSM6AAAAABM33WSBWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGNJQHA4TSMJQGI . You are receiving this because you commented.Message ID: @.***>

NaolShow commented 1 month ago

Sorry, I haven't seen that the version was out of pre-release.

After testing with the 4.4.5R-Pro. Everything is now working fine! Synclists behaves perfectly in my inventory system. Can't seem to find any bug.

Thank you so much for the fixes. Closing this issue as completed then!