[1.8.x-1.12.x] Module Manager 4.2.3 (July 03th 2023) - Fireworks season

[linuxgurugamer]

Is there a way to test if a file exists?

The issue is that as Squad is doing their updates and revamps, they are deprecating some parts.  This causes problems for mods which depend on those parts for either cloning, textures, etc.  This has affected at least two of my mods, I've had to write batch scripts which the user needs to run manually and causes problems with a non-technical user trying to incorrectly run these scripts.

I've gone through the wiki, and I don't see this ability.  The closest is the NEEDS, but it only looks for the following:

• A plugin .dll with the same assembly name.
• A subdirectory name under GameData. (Names with spaces can be used, just remove the spaces: GameData/My Mod/ => :NEEDS[MyMod]
• A FOR[Blah] defined would allow NEEDS[Blah]

It's close in that it can already look for a subdirectory name under GameData, IMHO that could easily be expanded into a directory or file.

 

If this doesn't exist, can I request that that this be added, and added soon?  This problem becomes worse with each new release of KSP.

Edit:

There is an existing feature request asking for exactly this: https://github.com/sarbian/ModuleManager/issues/144

Edit 2

I just tested the NEEDS with a directory (I was hopeful it ight deal with multiple directories):

@PART[BSLmk12SciencePod]:NEEDS[Squad/zDeprecated/Parts/Command/Mk1-2Pod]
{
	@MODEL
	{
		@model = Squad/zDeprecated/Parts/Command/Mk1-2Pod/model
	}
}

This directory does exist in 1.7, but MM doesn't see it:

[LOG 2019-05-24 07:00:53.314] Deleting root node in file BetterScienceLabsContinued/MM_DeprecatedParts node: @PART[BSLmk12SciencePod]:NEEDS[Squad/zDeprecated/Parts/Command/Mk1-2Pod] as it can't satisfy its NEEDS

 

Edited May 24, 2019 by linuxgurugamer

[Tonka Crash]

3 hours ago, linuxgurugamer said:

The issue is that as Squad is doing their updates and revamps, they are deprecating some parts.  This causes problems for mods which depend on those parts for either cloning, textures, etc.  This has affected at least two of my mods, I've had to write batch scripts which the user needs to run manually and causes problems with a non-technical user trying to incorrectly run these scripts.

Have you considered the case where Squad eventually deletes the deprecated parts? Have you contacted Squad about possibly bundling and distributing the deprecated elements with the mods that need them? It doesn't hurt to ask. 

[linuxgurugamer]

1 hour ago, Tonka Crash said:

Have you considered the case where Squad eventually deletes the deprecated parts? Have you contacted Squad about possibly bundling and distributing the deprecated elements with the mods that need them? It doesn't hurt to ask. 

Yes, yes, and the answer was no

[Lisias]

5 hours ago, linuxgurugamer said:

If this doesn't exist, can I request that that this be added, and added soon?  This problem becomes worse with each new release of KSP.

I think that this problem can be mitigated on a better way.

Module Manager would issue a "meta-name" with the current KSP version, so you can use ":NEEDS" et all to check the current KSP version in the same way we do for Add'Ons.

Something like:

• KSP_<MAJOR>
• KSP_<MAJOR>.<MINOR>
• KSP_<MAJOR>.<MINOR>.<PATCH>

So the author would peek the less restrictive one that works for him.

As an example

@PART[airbrake1]:FOR[TweakScale] // A.I.R.B.R.A.K.E.S
{
    %MODULE[TweakScale]:NEEDS[KSP_1.4.5]
    {
        type = free_square_old_edition
    }
    %MODULE[TweakScale]:NEEDS[KSP_1.6]
    {
        type = free_square_new_edition
    }
    %MODULE[TweakScale]:NEEDS[KSP_1&¬KSP_1.6&¬KSP_1.4.5]
    {
        type = free_square_common_edition
    }

}

Would define a Module with type "free_square_old_edition" for KSP 1.4.5, but would define it to "free_square_new_edition" for KSP 1.6.0 and 1.6.1 (or 1.6.2 if it would be issued).

But "free_square_common_edition" would be issued for every KSP version that starts with 1, except 1.6 and 1.4.5 .

(I'm not sure now about the correct syntax on MM, so I used the following boolean notation : "&" = AND ; "¬" = NOT)

I think this will solve pretty well a lot of issues.

[linuxgurugamer]

Well, that would help, but unfortunately, I forgot that the zDeprecated directory was also not loaded in the DB, so for my case, it doesn't matter, I still need to copy the files

Bummer, the copy process is so full of possible errors....

[cakepie]

18 minutes ago, Lisias said:

Module Manager would issue a "meta-name" with the current KSP version

@linuxgurugamer In case KSP version check would help at least identify whether there's zDeprecated directory and what's in it: it's issue #116 , I wrote up some code to do it a while ago but it's got some loose ends that needs tied up, see here if you want to chip in and help get it implemented and integrated.

[linuxgurugamer]

1 minute ago, cakepie said:

@linuxgurugamer In case KSP version check would help at least identify whether there's zDeprecated directory and what's in it: it's issue #116 , I wrote up some code to do it a while ago but it's got some loose ends that needs tied up, see here if you want to chip in and help get it implemented and integrated.

I'm hip deep in 4 other mods right now.  Besides, since the directory can't be accessed, the files still need to be copied out and somewhere else.  Which means an external script

 

Gaaah, I hate it when an idea for a new mod pops in my head. 

In this case, a simple mod to copy specified files to a common location if they don't exist.  Would be callable by any other mod which needs files copied such as the ones I'm dealing with

Actually would be configured in a text file, if done correctly, could even do it before KSP gets started with processing files

 

[blowfish]

27 minutes ago, linuxgurugamer said:

I'm hip deep in 4 other mods right now.  Besides, since the directory can't be accessed, the files still need to be copied out and somewhere else.  Which means an external script

 

Gaaah, I hate it when an idea for a new mod pops in my head. 

In this case, a simple mod to copy specified files to a common location if they don't exist.  Would be callable by any other mod which needs files copied such as the ones I'm dealing with

Actually would be configured in a text file, if done correctly, could even do it before KSP gets started with processing files

 

If you copy the files immediately on addon startup KSP will actually pick them up.  Reason being that the game database is re-initialized after addons are initialized.  Alternately you can manipulate the game database tree structure a frame later to include the missing files - if you do it then ModuleManager will pick it up (just don't wait any longer)

[linuxgurugamer]

11 minutes ago, blowfish said:

If you copy the files immediately on addon startup KSP will actually pick them up.  Reason being that the game database is re-initialized after addons are initialized.  Alternately you can manipulate the game database tree structure a frame later to include the missing files - if you do it then ModuleManager will pick it up (just don't wait any longer)

That's kind of what I was thinking.  It would be really neat if something like that could be added to MM, since it deals with all that already

 

[blowfish]

1 minute ago, linuxgurugamer said:

That's kind of what I was thinking.  It would be really neat if something like that could be added to MM, since it deals with all that already

 

Why would we want ModuleManager to do this?  We don't want KSP to load everything in zDeprecated.  The idea would be that if you need specific files from there in your mod, your mod would do the setup required for those files to be loaded (either by copying or adding them to the tree at the appropriate time).

[linuxgurugamer]

My (admittedly quick) thought was that it could be modified to respond to a specific statement to copy files, which could be included as part of a MM patch.

 

[Lisias]

Just now, linuxgurugamer said:

Well, that would help, but unfortunately, I forgot that the zDeprecated directory was also not loaded in the DB, so for my case, it doesn't matter, I still need to copy the files

Bummer, the copy process is so full of possible errors....

The problems,we tackle them one by one. And I think that Module Manager can help on it too. I'm mangling GameDatabase for some time now, and it's really not a problem to "brute force" a part into it - so yeah, we can feed GameDatabase with the deprecated parts ourselves.

Ideally, this should be done on Module Manager, as it always have the MMPatchLoader available to itself, and since MM4 outsiders lost the canonical way of invoking it.

[blowfish]

21 minutes ago, Lisias said:

Ideally, this should be done on Module Manager, as it always have the MMPatchLoader available to itself, and since MM4 outsiders lost the canonical way of invoking it.

Why would this require access to MMPatchLoader?

[cakepie]

I agree with blowfish, this all seems out of scope for MM.

If you're going to copy files out of zDeprecated into some other location in GameData, that's something that only needs to be done once, during mod install, and not everytime the game starts up.

If on the other hand you want to force certain files to be read from zDeprecated and made available in the gamedatabase then something like unBlur's code could possibly work.

37 minutes ago, Lisias said:

since MM4 outsiders lost the canonical way of invoking it.

Not sure about your use case, maybe unBlur code and my notes on MM compat here may help.

Edited May 24, 2019 by cakepie

[kcs123]

My 0.02 cents on this:

• MM would be better idea to handle copying files than separate plugin. Possible issue with another plugin is timing, to execute copy command and wait for all files to be copied before MM start to search for other MM commands trough Gamedata folder tree
• MM would need to search trough all config files to see if any of them contain command to copy files before executing the rest of patches. That would be slow down loading game to some degree, depending how many files need to be processed twice.
• MM should restrict available folders for source and destination, otherwise someone could exploit it to copy some malicious files from elsewhere to KSP game folder

Even better idea might be for less educated people to use CKAN and extend CKAN with new feature to copy files from deprecated folder to gamedata folder instead using batch file or doingh that with MM. People who don't use CKAN for whatever reason, already know how to copy files and read instructions what need to copy where, so it is less likely that simple batch file would be marked as virus from some stupid AV software.

I have to deal with people that don't know how to create backup on their USB sticks every day and they either, don't want to learn it or they are to lazy to learn it. It can be strenuous at times to explain it again and again.

[linuxgurugamer]

26 minutes ago, cakepie said:

If you're going to copy files out of zDeprecated into some other location in GameData, that's something that only needs to be done once, during mod install, and not everytime the game starts up.

Yes, and there isn't an easy way to do that.  I've written some scripts, but to make them foolproof, is almost impossible in the time available, also need to write them for 3 OSs.

So either it gets incorporated into MM, or it's yet another mod for someone to load.

[Lisias]

44 minutes ago, blowfish said:

Why would this require access to MMPatchLoader?

Because it is how UbioWelding creates welded parts on GameDatabase. 

Using Mono's Reflection to locate MMPatchLoader for now can be unreliable due Mono's unhappy bugs on Reflection, and I think this is the reason MMPatchLoader extends LoadingSystem : to allow to use UnityEngine's equivalent.

21 minutes ago, kcs123 said:

 

• MM would need to search trough all config files to see if any of them contain command to copy files before executing the rest of patches. That would be slow down loading game to some degree, depending how many files need to be processed twice.
•  

Better idea: each Add'On specifies what it needs, and MM only handles what it was specifically told to do.

This would impose a delay only for the first Add'On to ask for something that's not there yet.

[blowfish]

1 hour ago, Lisias said:

Because it is how UbioWelding creates welded parts on GameDatabase. 

Still not sure why what requires anything involving ModuleManager.  Shouldn't the final (patched) configs be sufficient for this?

[Lisias]

5 hours ago, blowfish said:

Still not sure why what requires anything involving ModuleManager.  Shouldn't the final (patched) configs be sufficient for this?

If you don't mind restarting the game, yes.

But UbioWeld and that hypothetical solution above on deprecated parts would preferably avoid restarting everything and load the config into memory at runtime and then carry on with their business, using the very same ModuleManager code that would be used on KSP loading time.

It's how it was being used on the Module Manager 3 and previous., by the way.

[blowfish]

40 minutes ago, Lisias said:

If you don't mind restarting the game, yes.

But UbioWeld and that hypothetical solution above on deprecated parts would preferably avoid restarting everything and load the config into memory at runtime and then carry on with their business, using the very same ModuleManager code that would be used on KSP loading time.

It's how it was being used on the Module Manager 3 and previous., by the way.

Yes, apparently there were multiple mods hooking into ModuleManager in unintended and unsupported ways...

Could you explain to me exactly what it was doing with ModuleManager?  That still isn't very clear to me.

Edited May 25, 2019 by blowfish

[cakepie]

2 hours ago, blowfish said:

Could you explain to me exactly what it was doing with ModuleManager?  That still isn't very clear to me.

Looks like it uses reflection to emulate DatabaseReloadWithMM() , to apply new patches at runtime:

https://github.com/UbioWeldingLtd/UbioWeldContinued/blob/12aef85739bc7a6deca52c73930456035971e8bd/UbioWeldingLtd/DatabaseHandler.cs#L111
https://github.com/net-lisias-ksp/UbioWeldContinuum/blob/7ef3f5fbfd76b00fc9d01cef4c80caf4215beafc/UbioWeldingLtd/DatabaseHandler.cs#L128  <- lisias' fork

Perhaps that's something that could be exposed to other mods?

 

Edit2: also pulls the ProgressFraction

 

 

Edited May 25, 2019 by cakepie
wrong fork

[Lisias]

11 hours ago, blowfish said:

Yes, apparently there were multiple mods hooking into ModuleManager in unintended and unsupported ways...

Well… I made a research on the matter, and got into this: change, the last change before, oldest mention of the file before a internal refactoring, and the first time MMPatchLoader was extended with LoadingSystem.

This first LoadingSystem commit was made in 2014, so we have 5 years of a public class being served to the general public!!

So I understand the reason Add'Ons choose to hook this way: it's available (it's a public class!) and it was stable for 5 years. It's my understanding that internal classes should be declared… internal…  and that public classes are intended to be consumed by the general public.

Of course, perhaps the MMPatchLoader should never be declared as public at first place - but yet, we have a de facto standard that in an ideal world, should not be changed without a reason.

[cakepie]

The MMPatchLoader class may be public, but you should notice that  DatabaseReloadWithMM()  has been private since MM2.4.0 and it had no access modifier before that, i.e. defaulted to internal.

The way that UbioWeld had to use reflection and copy part of MM's code to emulate it is definitely unsupported.

If it was intended for other mods to be able to trigger a database reload there would have been a much nicer, properly exposed way for us to do it.

---

Edit: @blowfish I think I can fix UbioWeld's code without too much difficulty, but perhaps a properly supported and documented wrapper class would be better?
Some things it could do, off the top of my head:

1.
Check if MM is installed, and if so, which version, and whether MM is ready (finished patching in loading screen)

2.
Since MM 2.6.7 there has been support for other mods to be notified via  ModuleManagerPostLoad() once MM has finished doing it's work in Loading screen, but relying on that makes MM a hard dependency.
A mod might want to:
- if MM is installed, once game database is loaded and MM has finished patching, do_stuff()
- else once game database is loaded, do_stuff()
so in unBlur I had to jump through a few hoops to handle both possibilities

3.
Provide a way to trigger a database reload (e.g. what UbioWeld needs)

 

Edited May 25, 2019 by cakepie

[cakepie]

On 5/24/2019 at 12:40 PM, Lisias said:

I think this is the reason MMPatchLoader extends LoadingSystem : to allow to use UnityEngine's equivalent.

This is where you've misunderstood.

MMPatchLoader was a subclass of LoadingSystem because that is the way to interface with KSP's loading scene.
That part of the job is now taken over by PostPatchLoader.

 

 

MMPatchLoader did not extend LoadingSystem for the sake of providing a way for others to use reflection to access it.
It just so happens AlexW / girka2k were able to leverage this to build a workaround to gain access / emulate DataBaseReloadWithMM() in a way that is unintended and unsupported by MM devs.
This was a long time ago and I don't know why they chose to do it like this, or whether or not there was any discussion at the time with MM side about better ways they might be able to do it.
But there was never any guarantee that this should work or continue to work like that.

 

[Lisias]

2 hours ago, cakepie said:

This is where you've misunderstood.

MMPatchLoader was a subclass of LoadingSystem because that is the way to interface with KSP's loading scene.
That part of the job is now taken over by PostPatchLoader.

What ended up allowing the class to be handled by Unity's reflection nevertheless. Being intentional or not, it ended up being the mechanism people managed to reuse that class for some years. It's what we call a de facto standard.

 

2 hours ago, cakepie said:

MMPatchLoader did not extend LoadingSystem for the sake of providing a way for others to use reflection to access it.
It just so happens AlexW / girka2k were able to leverage this to build a workaround to gain access / emulate DataBaseReloadWithMM() in a way that is unintended and unsupported by MM devs.

The whole Add'On scene is unintended and unsupported. We are at our own here.

All that can be done is trying to work together, or trying to work in separate ways. Both are valid, by the way - but some ways tends to be more useful to the end users, and IMHO the ultimate objective of every Software is to be useful - so I tend to be somewhat grumpy on everything that hinders the usefulness of the software to the end users.

Please observe that I didn't complained on a change on DataBaseReloadWithMM() - had it changed somehow, I would just had coped with the new behaviour and called it a day. The show stopper happened because a change happened that broke down the very mechanism we were using to even safely reach a public class without relying on the buggy Mono's Reflection, so no matter what, I would had to change something that was working safely to something that historically leaded to some grind.

Fixing UbioWeld to cope with the current status quo is easy. Doing it safely without uncontrollable side effects is the problem.

The safest approach to the problem is keep using UnityEngine.Object.FindObjectOfType, what would need only this patch. No MM code stops working by this (I'm testing this for some weeks now), and everything else just keep working as before without any change.

Of course, alternatively it can be decided that MMPatchLoader should not be used by anyone else - I'm Ok with it. Tell us, put an "internal" on the class declaration (so preventing further misunderstandings) and everybody else are free to decide the best way to cope with that.