Clean up Loading extension CustomPathManager

[kianzarrin]

There is some untidy code in loading extension that looks like legacy code which no one has bothered to clean up. The code for Custom Path manger has the following problems:

---

The value assigned to CustomPathManager in OnCreated() is never used. Its overwritten in loading extension. CustomPathManager is unlike other managers and cannot be instantiated in OnCreated(). This line is redundant and should be deleted:

TMPE/TLM/TLM/LoadingExtension.cs

Line 282 in 06eda8d

CustomPathManager = new CustomPathManager();

CustomPathManager = new CustomPathManager();

---

Considering that the Loading extension holds a reference to an Instance of CustomPathManager, accessing the static _Instance field is unnecessary:

TMPE/TLM/TLM/LoadingExtension.cs

Line 330 in 06eda8d

CustomPathManager._instance.WaitForAllPaths();

Use This property instead:

TMPE/TLM/TLM/LoadingExtension.cs

Line 63 in 06eda8d

public static CustomPathManager CustomPathManager { get; set; }

---

CustomPathManager needs OnLevelLoaded() and OnLevelUnLoading(). The following code in loading extension needs to go into these new functions:

• TMPE/TLM/TLM/LoadingExtension.cs

  Line 356 in 06eda8d

  // Custom path manger is destroyed when reloading. That is why the following code

• TMPE/TLM/TLM/LoadingExtension.cs

  Line 497 in 06eda8d

  CustomPathManager = stockPathManager.gameObject.AddComponent<CustomPathManager>();

[kianzarrin]

Also This line should throw an exception for consistency reasons (don't worry ... its in a try..,catch block):

TMPE/TLM/TLM/LoadingExtension.cs

Line 501 in 06eda8d

Log.Error("CustomPathManager null. Error creating it.");

The code for CustomPathManager OnLevelLoaded() and OnLevelUnLoading() could be like this:

       static private FastList<ISimulationManager> GetSimManager =>
            typeof(SimulationManager).GetField("m_managers", BindingFlags.Static | BindingFlags.NonPublic)
                ?.GetValue(null) as FastList<ISimulationManager>;

        public void OnLevelUnloading() {
            Log.Info("CustomPathManager.OnLevelUnloading()");
            // TODO [issue does not exit] uncomment for more clean code and to support ingame unload
            // Custom path manger is destroyed when reloading. That is why the following code is commented out.
            //GetSimManager?.Remove(this);
            //Object.Destroy(this);
            _instance = null;
        }

        static public CustomPathManager OnLevelLoaded() {
            Log.Info("CustomPathManager.OnLevelLoaded() called. Setting up CustomPathManager and SimManager.");
            FieldInfo pathManagerInstance = typeof(Singleton<PathManager>).GetField(
                "sInstance",
                BindingFlags.Static | BindingFlags.NonPublic);
            if (pathManagerInstance == null) {
                throw new Exception("pathManagerInstance is null");
            }

            PathManager stockPathManager = PathManager.instance;
            if (stockPathManager == null) {
                throw new Exception("stockPathManager is null");
            }

            Log._Debug($"Got stock PathManager instance {stockPathManager?.GetName()}");
            // TODO [issue does not exist] restore vanila custom path manager for cleaner code and to support in-game unload 
            _instance = stockPathManager.gameObject.AddComponent<CustomPathManager>();
            Log._Debug("Added CustomPathManager to gameObject List");

            if (_instance == null) {
                throw new Exception("CustomPathManager is null. Error creating it.");
            }

            _instance.UpdateWithPathManagerValues(stockPathManager);
            Log._Debug("UpdateWithPathManagerValues success");

            pathManagerInstance.SetValue(null, _instance);

            Log._Debug("Getting Current SimulationManager");
            var simManager = GetSimManager;
            if (simManager == null) {
                throw new Exception("simManager is null");
            }

            Log._Debug("Removing Stock PathManager");
            simManager.Remove(stockPathManager);

            Log._Debug("Adding Custom PathManager");
            simManager.Add(_instance);

            UnityEngine.Object.Destroy(stockPathManager, 10f);

            Log._Debug("Should be custom: " + Singleton<PathManager>.instance.GetType());

            return _instance;
        }

Then in the loading extension OnLevelLoading():

            try {
                // CustomPathManager must be created while teh level is loading.
                CustomPathManager = CustomPathManager.OnLevelLoaded();
            }
            catch (Exception ex) {
                string error = "Traffic Manager: President Edition failed to load. You can continue " +
                               "playing but it's NOT recommended. Traffic Manager will not work as expected.";
                Log.Error(error);
                Log.Error($"Path manager replacement error: {ex}");

                Singleton<SimulationManager>.instance.m_ThreadingWrapper.QueueMainThread(
                    () => {
                        UIView.library
                              .ShowModal<ExceptionPanel>("ExceptionPanel")
                              .SetMessage(
                            "TM:PE failed to load",
                            error,
                            true);
                    });
            }

[originalfoo]

Please send in a PR for this and #731 so we can test; it's always good to get the code cleaned up and improved.

[kianzarrin]

@kian.zarrin Pathfinding object is leaking... I did maybe 4-6 hot-reloads of my second mod while

TM:PE was just enabled
Debug 1,999.0765170: Waking up CustomPathManager.
Debug 1,999.0863329: Creating 256 custom PathFind objects.
Info 1,999.0955972: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.1054987: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.1156799: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.1258537: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.1390482: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.1506545: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.1607004: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.1709649: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.1814978: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.1920980: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.2021296: Pathfinder logic: Using CustomPathFind_Current
Info 1,999.2125589: Pathfinder logic: Using CustomPathFind_Current

@krzychu124 are you using debug build? is it latest master branch?

[kianzarrin]

kian.zarrinToday at 13:02
@krzychu124 are you using debug build? is it latest master branch?
this might explain the game crashed due too many threads problem
krzychu124Today at 13:07
Full debug, but I am not hot-reloading tm:pe, that's the weirdest thing :)
kian.zarrinToday at 13:12
oh I see!
where is your second mod by the way?
if you have output.log it helps me to see if TMPE was reloaded.
krzychu124Today at 13:21
I think it doesn't matter what other mod you'll try hot-reload
kian.zarrinToday at 13:52
while TM:PE was just enabled
Ambiguine statement. does it mean TM:PE was simply enabled all along or TMPE was enabled at that moment?
krzychu124Today at 13:58
TMPE was enabled all the time, I haven't use it at that time.

I do not observe double hot reload of TrafficManager.dll when I hot reload my HideCrossings.dll
output_log.txt

That said, the traffic manager did load again (maybe after hot reload the serialize data extension is called again?)

Debug 470.5978783: HOT RELOAD ...

TMPE.log

[kianzarrin]

we need something like this:

            if (TrafficManagerMod.Instance.InGameHotReload && !LoadingExtension.IsGameLoaded) {