FirstGearGames / FishNet

FishNet: Unity Networking Evolved.
Other
1.39k stars 149 forks source link

Loading scenes in pairs makes sync collections empty #651

Closed Kromsalovo closed 1 month ago

Kromsalovo commented 7 months ago

General Unity version: 2022.3.21f1 Fish-Networking version: 4.2.0

Description if you load scenes in pairs where 1 scene is persistent and never changes, it empties sync collections in this scene

string[] scenesToLoad = new string[] { "MultiplayerStuff", LevelName }; 
// i never change "MultiplayerStuff" as i have persistent data here, only swapping LevelName 
SceneLoadData sld = new SceneLoadData(scenesToLoad);
sld.ReplaceScenes = ReplaceOption.All;
sld.PreferredActiveScene = new PreferredScene(SceneLookupData.CreateData(LevelName));
SceneManager.LoadGlobalScenes(sld);

if i unload scenes manually without touching persistent one , this problem is not appearing

Replication Steps to reproduce the behavior:

  1. Import example package
  2. Load Sample Scene
  3. Press play in the Unity editor and click Server and Client
  4. Press Space 3 times and on 3rd time collection that is inside Bug script on Test GameObject would be lost

Example File Bug.zip

FirstGearGames commented 7 months ago

I'm not sure if I'd qualify this as a bug because it seems like it's unloading all scenes then reloading them. Which ironically is what most people want when you try to single load the same scene, but iirc FishNet rather just keeps the scene loaded.

I'll have to take a look at the code again and see what's making it unload the 'MultiplayerStuff' scene, then apply that to if only a single scene.

So in reality, it might actually be working exactly as it's supposed to. But the single scene load not refreshing is the actual bug.

Still labeling as bug though so I can get the single scene one working the same way. You should be able to also load the scene while disabling auto unload and it won't reload.

Edit: I see plannedpublic bool ReloadScenes; inside SceneLoadData.Options. It shouldn't be reloading in any scenario unless that's set to true. So definitely could be undesired behavior after all.

Kromsalovo commented 7 months ago

i agree that everything in current code suggests that unloadall->loadall is expected behaviour and if you want something to stay you should control it yourself, there also certaint differences between those 2 methods like when you get additional scenepresence changes with manual approach or problems happening with some scenes not unloading properly in build, so its for you to decide if you want to enforce this more clean unloadall-loadall way or add additional functionality to make my way possible(even if it introduces several differences in a way some callbacks gets called and possible bugs)

FirstGearGames commented 1 month ago

This might just be a bug in sync dictionary.

Change method to this and it is working

        internal protected override void ResetState(bool asServer)
        {
            base.ResetState(asServer);

            bool canReset = (asServer || !base.IsReadAsClientHost(asServer));

            if (canReset)
            {
                _sendAll = false;
                _changed.Clear();
                Collection.Clear();
                _valuesChanged = false;

                foreach (KeyValuePair<TKey, TValue> item in _initialValues)
                    Collection[item.Key] = item.Value;
            }
        }

ClientHost was losing observation so ResetState was calling on the object and it's SyncTypes. In result SyncDictionary was resetting values as clientHost when it should not have been.

Edit: looks like the others suffer this issue too. Same easy fix though.