sarbian / ModuleManager

176 stars 96 forks source link

Fixed issue where MM would crash the game due to a DLL being renamed #177

Closed linuxgurugamer closed 1 year ago

linuxgurugamer commented 1 year ago

before MM was able to get the file version. The rename was from ZeroMiniAVC, which was renaming the MiniAVC.dll and MiniAVC_v2.dll files, and them MM was getting a FileNotFound error

This is a critical issue for anyone trying to start a game with any MiniAVC.dll files

Lisias commented 1 year ago

The problem is that the MiniAVC.DLLs are renamed after KSP enumerates them in memory.

The ideal solution is to restart KSP after renaming the MiniAVC.dlls . Otherwise you will need to make pull requests to every Assembly that uses Reflection - including KSP itself.

linuxgurugamer commented 1 year ago

The fix I put in was a simple try/catch, to avoid the FileNotFound exception.

I tested this in a 1.12.4 install where I was able to replicate the error, with the fix it's not there

Yes, that's the ideal situation, since this is the only mod that has an error with this, it's a good fix

Lisias commented 1 year ago

Are you going to do a pull request on EVERY add-on and also on KSP itself?

because otherwise you are only masking the problem where it's more visible.

Lisias commented 1 year ago

The ideal solution is not install MiniAVC*.dll at all, unless the user wants it.

Since this ship has sailed, you need to realise that by the time you prune the DLL it was already registered by KSP and so everything (and I really mean it) that tries to enumerate the available Assemblies will bork.

The only real fixes for the problem is:

Any other half-baked solution will make 3rd parties to blow up, spreading further the damage that it's already being done.

Lisias commented 1 year ago

I had come to a solution for this problem by pure accident.

Apparently, only the first loaded DLL needs to be present on the filesystem, all the other duplicates can be safely pruned from it (including on KSP < 1.8.0).

So a possible solution to this problem is to have a folder called GameData/999_ZeroMiniAvc where a "reference" MiniAVC.DLL would be placed. This thing must be kept up to date, as KSP >= 1.8 will always use the first one loaded when duplicated are found and you will want people to use the most bug free code.

ZeroMiniAVC must be instrumented to do not run from the 999_ZeroMiniVC folder. It's essentially a dummy Assembly when invoked from this folder.

From this point, removing all the others MiniAVC.DLL from the system will not break the Reflection, as the first one will sill be there to keep the thingy happy. And since any duplicated will be shortcircuited to the first one loaded, the Reflection will be also consistent.

I think some tests are needed to be absolutely sure this will stick to ZeroMiniAVC, but the results I got on my rig looks promising.

(in time, this Pull Request really should be closed without merging)

linuxgurugamer commented 1 year ago

If you are going to leave a DLL on the system, why not just install the latest ZeroMiniAVC, which deletes all of them, AND puts in a MiniAVC.dll which it uses to do the same thing you are describing? Regardless, if you use CKAN to install your mods, it already is removing all the ZeroMiniAVC dlls during installation

Lisias commented 1 year ago

Because by removing every single copy of the DLL will crash the Reflection if anyone tries to use it unless by rebooting KSP, exactly as it happened to Module Manager here.

Leaving a "reference" DLL, dormant, will garantee that the Reflection will be happy after removing the all the other DLLs (since KSP 1.12.x short-circuits all the duplicates to the one it elects as the dominant by looking into the Assembly Version).

The reason Module Manager was crashing was due this problem at first place - and since you found the need to do this P/R even by CKAN removing all the MiniAVC.dlls from the system, it was obvious to me that you was getting some heat from people not using CKAN.

This dirty trick will allow ZeroMiniAVC to delete all the MiniAVC.DLL from the system without the need to restart KSP, as long the one KSP chooses to be loaded still pinpoints to a existent DLL in the file system.

linuxgurugamer commented 1 year ago

Ummm, ZeroMiniAVC DOES have a reference dll as part of it. Its a null dll, does nothing other than solve this problem.