CitiesSkylinesMods / TMPE

Cities: Skylines Traffic Manager: President Edition
https://steamcommunity.com/sharedfiles/filedetails/?id=1637663252
MIT License
575 stars 85 forks source link

Updates to mod incompatibility checker #608

Open originalfoo opened 4 years ago

originalfoo commented 4 years ago

I'm pondering making some additional changes to mod incompatibility checker after LABS gets updated to v11, so probably part of v11.1 release:

Thoughts?

kvakvs commented 4 years ago

Moving to code is a good idea. Syntax checks, and severity enum will also be checked by the compiler. When you display "severity" to the user, they might not pay attention to what it means and will only care whether this run on their game + mods setup? And if you allow them to click ignore the conflicts, many will ignore (and suffer) without remembering the cause.

originalfoo commented 4 years ago

I was thinking the severity would override user choice to varying degrees.

Example:

originalfoo commented 4 years ago

Another thing I'd like to add: Detect if multiple TMPE assemblies in the app domain. For example, user switches from LABS to STABLE while game is running = likely to suffer settings loss as game doesn't know which assembly is active. Would be useful to check for that and warn user maybe with button to exit to desktop.

Is it possible to check if there are multiple assemblies in RAM? (cc: @dymanoid)

krzychu124 commented 4 years ago

Good idea, you can get list of all assemblies loaded to current AppDomain using AppDomain.CurrentDomain.GetAssemblies() :)

originalfoo commented 4 years ago

I assume Assembly.FullName will be sufficient to check whether there are multiple TMPE in RAM?

krzychu124 commented 4 years ago

FullName is string which also contains version of assembly, so you would get false negative result when you compare across different build versions. I would try to find TrafficManager with other version than caller: Assembly.GetName().Name && Assembly.GetName().Version

dymanoid commented 4 years ago

I'd rather suggest using the main module's GUID instead of the version (as you already do for duplicate detection). FullName is not that great, better go with Name and GUID.

krzychu124 commented 4 years ago

Info 28.7546777: Something went wrong while checking incompatible mods - see main game log for details.

Incompatible mod detector is failing completely when one or more mods were not instantiated correctly on game init(missing dependencies or other problem) - probably needs try...catch or additional check.

originalfoo commented 4 years ago

I have a try..catch around whole thing currently so the errors probably aren't great. I'll try and improve that for the v11.2 release as it might help track down some broken mods. I should at least be outputting the mod path or something as that would give some way to find mod even if it's not loading in to game properly.

originalfoo commented 4 years ago

Getting there....

Info 21.5842923: Assembly: TrafficManager v1.0.6877 in MOD ERROR
Info 21.5849921: Assembly: TrafficManager v1.0.6665 in MOD ERROR
Info 21.5856742: Assembly: TrafficManager v1.0.7080 in MOD ERROR
Info 21.5863450: Assembly: TrafficManager v1.0.7338 in MOD ERROR
Info 21.5870234: Assembly: TrafficManager v11.1.0 in MOD ERROR
Info 21.5882154: Assembly: TrafficManager v11.1.0 in \TrafficManager

Only the last mod (local build) was enabled.

I tried using asm.Location but it was empty for all mods. So then, seeing as I was going to need PluginInfo for the mods anyway (eg. to unsubscribe and other stuff), I tried Singleton<PluginManager>.instance.FindPluginInfo(asm); but it only works for enabled assemblies!

The parent folder (eg. workshop id in most cases) will be useful for logging and maybe also to display in whatever dialog I concoct.

I also, for TM:PE versions prior to 11.1.0, want to try and query the Version field or, if not present, the Name field/property on the TrafficManagerMod class - being able to show versions or name older mods or their name (which sometimes contains version) will be useful for disambiguation. I suspect I can do that via reflection, but haven't quite worked out how to yet.

Being able to get a date (either of build if such a thing exists, or file last changed on disk) would also be most useful.

originalfoo commented 4 years ago

Hrm, I can iterate Singleton<PluginManager>.instance.GetPluginsInfo() and create a dictionary of <Guid, PluginInfo> then I can cross-reference that with asm.ManifestModule.ModuleVersionId to link an assembly to a PluginInfo where possible.

But what to do if two PluginInfo.userModInstance.GetType().Assembly.ManifestModule.ModuleVersionId gives same Guid value? (Eg. someone uploads an old TM:PE to workshop without changing anything, there's now two mods with same Guid?)

originalfoo commented 4 years ago

Just dumping some info about old versions TM:PE TrafficManagerMod:

Version + game version available as far back as 1.7.1 (1.7.0 was released as version 1.7.1 by mistake) in 2016. Game version info, or lack thereof, will make any obsolete versions very obvious:

public static readonly string Version = "1.7.1";

public static readonly uint GameVersion = 155313168u;
public static readonly uint GameVersionA = 1u;
public static readonly uint GameVersionB = 5u;
public static readonly uint GameVersionC = 0u;
public static readonly uint GameVersionBuild = 4u;

.Version appears somewhere in .Name as far back as v1.9.0 in 2017:

public string Name => "Traffic Manager: President Edition [" + Version + "]";

Occasionally .Version has other stuff in it - how to parse in to a Version instance for easy comparison?

public static readonly string Version = "1.10.15-stable";

LABS vs. STABLE introduced in 10.21:

public static readonly string Version = "10.21";

#if LABS
        public string Branch => "LABS";
#elif DEBUG
        public string Branch => "DEBUG";
#else
        public string Branch => "STABLE";
#endif
originalfoo commented 4 years ago

Is this a viable way to clean -whatever from a string version number prior to using Version.TryParse()?

string clean = Regex.Replace(strVersion, "[^0-9\.]", string.Empty);

Or this from StackOverflow:

string clean = Regex.Match(strVersion, @"[0-9]+(?:\.[0-9]+)+").Value;

Which goes in to:

if (Version.TryParse(clean, out var version)) {
  // version  extracted
}
else {
  // source doesn't have a match
}
originalfoo commented 4 years ago

Just accidentally did a hot load after instinctively building project upon some changes ...and the assembly checker noticed the zombie!

Info 18.4499741: Assembly: TrafficManager v11.1.0 in MOD ERROR
Info 18.4506880: Assembly: TrafficManager v1.0.7338 in MOD ERROR
Info 18.4513620: Assembly: TrafficManager v1.0.7080 in MOD ERROR
Info 18.4520180: Assembly: TrafficManager v1.0.6665 in MOD ERROR
Info 18.4526769: Assembly: TrafficManager v1.0.6877 in MOD ERROR
Info 18.4536098: Assembly: TrafficManager v11.1.0 in \TrafficManager

// hot reload

Info 261.4814958: Assembly: TrafficManager v11.1.0 in \TrafficManager
Info 261.4823702: Assembly: TrafficManager v11.1.0 in MOD ERROR
Info 261.4830772: Assembly: TrafficManager v1.0.7338 in MOD ERROR
Info 261.4837739: Assembly: TrafficManager v1.0.7080 in MOD ERROR
Info 261.4844594: Assembly: TrafficManager v1.0.6665 in MOD ERROR
Info 261.4851739: Assembly: TrafficManager v1.0.6877 in MOD ERROR
Info 261.4863544: Assembly: TrafficManager v11.1.0 in MOD ERROR

Note 3 x 11.1.0 in second set of results. I just wish I could accurately get paths. Still working on it...

originalfoo commented 4 years ago

Also it looks like the game uses the assembly that's first in the list, assuming that's how hotloading works (it puts the newly detected assembly at top of list). I changed assembly version and did a build to trigger hotload at main menu:

Info 45.4962332: TM:PE 11.1.1 LABS designed for Cities: Skylines 1.12.3
...
Info 45.5013651: Assembly: TrafficManager v11.1.1 in \TrafficManager
Info 45.5022161: Assembly: TrafficManager v11.1.0 in MOD ERROR
Info 45.5028974: Assembly: TrafficManager v1.0.7338 in MOD ERROR
Info 45.5035862: Assembly: TrafficManager v1.0.7080 in MOD ERROR
Info 45.5042713: Assembly: TrafficManager v1.0.6665 in MOD ERROR
Info 45.5049646: Assembly: TrafficManager v1.0.6877 in MOD ERROR
Info 45.5060715: Assembly: TrafficManager v11.1.0 in MOD ERROR

So I'm wondering now if we can fake a hotload to choose which version of TM:PE is active. That would mean we could switch between LABS and STABLE at will - have both subscribed, and so long as only one is enabled it can get added to top of assemblues list...?

originalfoo commented 4 years ago

@krzychu124 @dymanoid do you think this would be viable thing to do?

So long as only one version of the mod is enabled, I can get it's Guid and scan to see if it's at top of asseblies list and, if not, fake a hotload to trick the game in to reloading it and putting it top of list = it's now the active assembly.

originalfoo commented 4 years ago

Finally got reflection mostly working:

Info 21.3403873: -- TrafficManager v11.1.0 LABS - aubergine
Info 21.3410013: -- TrafficManager v1.0.7338 STABLE - krzychu - should be v11.0
Info 21.3418188: -- TrafficManager v10.20 OBSOLETE - LinuxFan
Info 21.3424263: -- TrafficManager v1.10.6 OBSOLETE - tmhardie (alpha/debug)
Info 21.3430252: -- TrafficManager v1.10.12 OBSOLETE - 591857426 (fixed for industry dlc)
Info 21.3439599: -- TrafficManager v11.1.1 LABS - Local dev build

Not sure why 11.0 STABLE is being reported as v1.0.7338 STABLE (reverting to assembly version because it had some problem reading the TrafficManagerMod.Version member.

originalfoo commented 4 years ago

Now reliably getting versions and I've added special case for v10.20 to treat it as STABLE for now (manually annotated for clarity):

Info 21.3293601: Assembly: TrafficManager v11.1.0 LABS    -> aubergine
Info 21.3302959: Assembly: TrafficManager v11.0 STABLE    -> krzychu
Info 21.3309732: Assembly: TrafficManager v10.20 STABLE    -> linuxfan
Info 21.3316470: Assembly: TrafficManager v1.10.6 OBSOLETE    -> rogue upload
Info 21.3323342: Assembly: TrafficManager v1.10.12 OBSOLETE    -> rogue upload
Info 21.3333248: Assembly: TrafficManager v11.1.1 LABS    -> local dev build

Edit: Multiple things we can do now:

Should make it much easier for us to auto-purge dead versions (excluding most recent version), and make it easier for user to decide which verions to disable (assuming we can fake hotload if remaining single active version is not at top of asaseblies list).

dymanoid commented 4 years ago

The "hot load" won't help for old versions if they were already loaded. They will still remain in the process and might still process events causing unexpected behavior. There is no way to get rid of "dead instances" but to restart the game.