avoid reload if game is already loaded. by kianzarrin · Pull Request #764 · CitiesSkylinesMods/TMPE

kianzarrin · 2020-03-01T16:13:15Z

#730 (comment) : if TMPE is already loaded and another mod is hot reloaded, CS calls OnCreate() again.

For this reason I made the following changes:

moved Hot Reload logic to TrafficManagerMod.Enabled()
OnCreated() is skipped if LoadingExtension.IsGameLoaded

These logs prove that my changes are successful:
output_log.txt
TMPE.log

moved hot reload logic to OnEnabled()

originalfoo · 2020-03-01T16:22:09Z

+
+        internal static AppMode currentMode => SimulationManager.instance.m_ManagersWrapper.loading.currentMode;
+
+        internal static bool InGame() {


Would it not be better to use SceneManager.GetActiveScene().name == "Game"?

I've been testing that as part of #699 and it's proved extremely reliable. (Note: PR #699 is currently non-functional as the UI stuff is a big mess - I'm waiting to see if kvakvs new Form Builder stuff can be used rather than spending ages messing with raw CO UI)

Not to mention that this will throw exception when used in menu (mentioned about that earlier)

it was not possible from OnCreated()
Now that I moved my stuff into Enable() ... yes

I have catched the exception so its fine :). I am going to change it to GetActiveScene() because its just cleaner that way

@aubergine10 I suspect that SceneManager.GetActiveScene().name == "Game" is not as reliable as we thought... Problem with it is that you will get true inside LoadingExtension.OnCreated()...
It seems that LoadingManager.instance.m_loadingComplete works way better for that case.

originalfoo · 2020-03-01T16:59:08Z

                }
            }
+
+            Instance = this;


Where is this getting used exaclty? When I look at the code in VS2019, it's not showing any references to it.

I can see down on line 110 (in updated TrafficManagerMod.cs) that it gets set to null in the OnDisabled() method, but other than that it's not getting used.

Was it supposed to be referenced in line 105 in the OnDisabled() method?

see
TLM/TLM/State/SerializableDataExtension.cs :29
VersionLabel.cs :14

originalfoo

See comments above.

kianzarrin · 2020-03-01T18:58:01Z

I rebuild TMPE and HideCrossings multiple time. No problem. no exception. only two path find objects created.

krzychu124

I'm getting various exceptions in that scenario:

load savegame,
hot-reload,
back to main menu,
load save again (you should see null pointer after loading)
if you click back to desktop you will see another nullpointer because of missing main ui (not instantiated because of previous exception)

1:

NullReferenceException: Object reference not set to an instance of an object
  at TrafficManager.UI.ModUI..ctor () [0x00000] in <filename unknown>:0 
UnityEngine.GameObject:Internal_AddComponentWithType(Type)
UnityEngine.GameObject:AddComponent(Type) (at C:\buildslave\unity\build\artifacts\generated\common\runtime\GameObjectBindings.gen.cs:382)
UnityEngine.GameObject:AddComponent() (at C:\buildslave\unity\build\artifacts\generated\common\runtime\GameObjectBindings.gen.cs:387)
TrafficManager.LoadingExtension:OnLevelLoaded(LoadMode)
TrafficManager.State.SerializableDataExtension:OnCreated(ISerializableData)
SerializableDataWrapper:OnSerializableDataExtensionsCreated()
SerializableDataWrapper:GetImplementations()
SerializableDataWrapper:.ctor(SimulationManager)
SimulationManager:CreateRelay()
LoadingManager:MetaDataLoaded()
System.Reflection.MonoMethod:InternalInvoke(Object, Object[], Exception&)
System.Reflection.MonoMethod:Invoke(Object, BindingFlags, Binder, Object[], CultureInfo)
System.Reflection.MethodBase:Invoke(Object, Object[])
LoadingScreenMod.Util:InvokeVoid(Object, String)
LoadingScreenMod.<LoadLevelCoroutine>d__23:MoveNext()
UnityEngine.SetupCoroutine:InvokeMoveNext(IEnumerator, IntPtr) (at C:\buildslave\unity\build\Runtime\Export\Coroutines.cs:17)

and then:

NullReferenceException: Object reference not set to an instance of an object
  at TrafficManager.LoadingExtension.OnLevelLoaded (LoadMode mode) [0x00000] in <filename unknown>:0 
  at TrafficManager.State.SerializableDataExtension.OnCreated (ISerializableData serializableData) [0x00000] in <filename unknown>:0 
  at SerializableDataWrapper.OnSerializableDataExtensionsCreated () [0x00000] in <filename unknown>:0 
Rethrow as ModException: The Mod C:\Users\krzychu\AppData\Local\Colossal Order\Cities_Skylines\Addons\Mods\TrafficManager [0Harmony.dll, CSUtil.CameraControl.dll, CSUtil.Commons.dll, TMPE.GenericGameBridge.dll, TMPE.API.dll, TMPE.CitiesGameBridge.dll, TrafficManager.dll] has caused an error

UnityEngine.DebugLogHandler:Internal_LogException(Exception, Object)
UnityEngine.DebugLogHandler:LogException(Exception, Object)
UnityEngine.Logger:LogException(Exception, Object)
UnityEngine.Debug:LogException(Exception)
SerializableDataWrapper:OnSerializableDataExtensionsCreated()
SerializableDataWrapper:GetImplementations()
SerializableDataWrapper:.ctor(SimulationManager)
SimulationManager:CreateRelay()
LoadingManager:MetaDataLoaded()
System.Reflection.MonoMethod:InternalInvoke(Object, Object[], Exception&)
System.Reflection.MonoMethod:Invoke(Object, BindingFlags, Binder, Object[], CultureInfo)
System.Reflection.MethodBase:Invoke(Object, Object[])
LoadingScreenMod.Util:InvokeVoid(Object, String)
LoadingScreenMod.<LoadLevelCoroutine>d__23:MoveNext()
UnityEngine.SetupCoroutine:InvokeMoveNext(IEnumerator, IntPtr) (at C:\buildslave\unity\build\Runtime\Export\Coroutines.cs:17)

3 (tmpe.log)- look at call time:

Info 12.6415353: OnLevelLoaded complete.
Info 12.9638589: Setting main menu button position to [118,900]
Info 12.9819596: Setting main menu position to [46,777]
Debug 13.0347918: Public transport info view panel found.
Info 13.1384836: ThreadingExtension.OnBeforeSimulationFrame: First frame detected. Checking detours.
Info 13.1495116: ThreadingExtension.OnBeforeSimulationFrame: First frame detected. Detours checked. Result: 0 missing detours
Debug 505.6433201: CustomPathManager: OnDestroy
Debug 512.9205270: ModUI destructor is called.
Debug 512.9363504: ModUI.DisableTool: called
Warning 512.9478318: ModUI.DisableTool: tool is null!
   at CSUtil.Commons.Log.LogToFile(System.String log, LogLevel level)
   at CSUtil.Commons.Log.Warning(System.String s)
   at TrafficManager.UI.ModUI.DisableTool()
   at TrafficManager.UI.ModUI.DestroyTool()
   at TrafficManager.UI.ModUI.ReleaseTool()
   at TrafficManager.UI.ModUI.Finalize()

Debug 36.8922696: Current tool: Tool Controller (DefaultTool)
Debug 37.0623220: TrafficManagerTool.ShowAdvisor(MainMenu) called.
Debug 37.0766636: ModUI.EnableTool: called
Info 37.0856128: Initializing traffic manager tool...
Debug 37.0955545: TrafficLightTool: Awake -1390331904

[edit4] This might be cause:

Info 0.3470659: Scan complete: 0 incompatible mod(s) found
Info 0.3554350: Mono version: 2.0 (Visual Studio built mono)
Debug 0.3659219: LoadingExtension.OnCreated() called
Debug 0.9809369: SerializableDataExtension.OnCreated() called
Debug 0.9894320: HOT RELOAD ...
Info 0.9983126: Loading Traffic Manager: PE Data
Info 1.0066368: Initializing flags
Info 1.0206870: OnBeforeLoadData: ExtNodeManager

kianzarrin · 2020-03-01T23:32:14Z

I reset InGameHotRelaod when quiting to main menu. I tested hot reloading TMPE and/or another mod multiple times each time I quite to main menu and load game again. No exception, no error.

originalfoo · 2020-03-01T23:35:00Z

This is looking better now.

I think at some point we should consider creating a Lifecycle.cs that contains some sort of state engine for the mod lifecycle management (enabed -> loading -> unloading -> disabled) but that's something for a later date.

kianzarrin · 2020-03-01T23:45:42Z

I think at some point we should consider creating a Lifecycle.cs that contains some sort of state engine for the mod lifecycle management (enabed -> loading -> unloading -> disabled) but that's something for a later date.

I have thought of that but I can't see how it would work.

the only thing I can think of is:

move all code from OnEnabled/OnDisabled, LoadingExtension.cs, and SerializeDataExtension.cs to lifecycle.cs
then all OnEnabled/OnDisabled, LoadingExtension.cs, and SerializeDataExtension.cs do is to call appropriate functions of lifecycle.cs at appropriate times.

So lifeCycle.cs acts as a wrapper for IUserMod, LoadingExtension, and SerializableDataExtension . A bit ugly if you ask me.

The best thing to do is a mod for hot reload to patch CS so that it does not do weird stuff.

originalfoo · 2020-03-02T00:00:14Z

Regarding the lifecycle stuff, I think we should create an issue to track that idea rather than pollute this PR. I'll create one now and copy stuff in to it.

krzychu124

LGTM, now works without noticeable issues

originalfoo

Tested it only once, in-game, but worked well! LGTM 👍

kianzarrin added 2 commits March 1, 2020 17:52

CitiesSkylinesMods#730 will avoid reload if game is already loaded.

2127976

moved hot reload logic to OnEnabled()

skip on created if level is loaded

063ce1d

originalfoo reviewed Mar 1, 2020

View reviewed changes

krzychu124 requested changes Mar 1, 2020

View reviewed changes

Comment thread TLM/TLM/LoadingExtension.cs

originalfoo reviewed Mar 1, 2020

View reviewed changes

originalfoo requested changes Mar 1, 2020

View reviewed changes

kianzarrin added 2 commits March 1, 2020 19:59

added missing return statements.

d3bf225

use scene manger to determine in game state.

d21090a

kianzarrin requested review from krzychu124 and originalfoo March 1, 2020 18:56

krzychu124 requested changes Mar 1, 2020

View reviewed changes

fixed hot-reload and the reloading from main menue

e493ac2

kianzarrin requested a review from krzychu124 March 1, 2020 23:32

originalfoo added this to the 11.2 milestone Mar 1, 2020

originalfoo mentioned this pull request Mar 2, 2020

Lifecycle class #767

Closed

krzychu124 approved these changes Mar 2, 2020

View reviewed changes

originalfoo approved these changes Mar 2, 2020

View reviewed changes

kianzarrin merged commit d89441a into CitiesSkylinesMods:master Mar 2, 2020

originalfoo mentioned this pull request Mar 2, 2020

Hot reload of dev builds seems broken #762

Closed

kianzarrin deleted the 730-custompath-loading branch March 2, 2020 09:24


		internal static AppMode currentMode => SimulationManager.instance.m_ManagersWrapper.loading.currentMode;

		internal static bool InGame() {

Uh oh!

Conversation

kianzarrin commented Mar 1, 2020

Uh oh!

originalfoo Mar 1, 2020

Choose a reason for hiding this comment

Uh oh!

krzychu124 Mar 1, 2020

Choose a reason for hiding this comment

Uh oh!

kianzarrin Mar 1, 2020

Choose a reason for hiding this comment

Uh oh!

kianzarrin Mar 1, 2020

Choose a reason for hiding this comment

Uh oh!

krzychu124 Mar 1, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

originalfoo Mar 1, 2020

Choose a reason for hiding this comment

Uh oh!

kianzarrin Mar 1, 2020

Choose a reason for hiding this comment

Uh oh!

originalfoo left a comment

Choose a reason for hiding this comment

Uh oh!

kianzarrin commented Mar 1, 2020

Uh oh!

krzychu124 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kianzarrin commented Mar 1, 2020

Uh oh!

originalfoo commented Mar 1, 2020

Uh oh!

kianzarrin commented Mar 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

originalfoo commented Mar 2, 2020

Uh oh!

krzychu124 left a comment

Choose a reason for hiding this comment

Uh oh!

originalfoo left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

krzychu124 left a comment •

edited

Loading

kianzarrin commented Mar 1, 2020 •

edited

Loading