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

[sarbian]

@CAPFlyer : maybe you should spend some time on the #kspmodders irc channel before telling me that I did not get input first, because "that" idea comes from chats I had with other modders.

@NathanKell We talked before, I know you want the mods to work and keep it easy. don't worry. Same for Starwaster, you keep helping users on the thread I read so I know you mean well.

@regex last version had Final working fine since only one DLL did the work

I'll revert the subdirectory thing. I'll name the DLL with version number, that code is too nice to trash it and it Even if it is indeed unlikely there will be a lot more realease. Just let me catch Majiir on IRC to talk with him.

Edited November 4, 2013 by sarbian

[MedievalNerd]

@MedievalNerd I have to disagree here. Mod management is not easy for all user.

You can't make anything fool proof. And since we aren't talking about rocket science here; making sure you have the right version of module manager in your gamedata directory. I don't see how significant amount of people would be having issues with this?

What I see the most is people not sure which version they are dealing with, then confusion about how dates are kept with their OS. (IE, resets date to when you downloaded). So having a version number in module manager solves all that. Plus trouble shooting would be extremely easy. "Do have more than one MM .dll in your game data folder? Oh you have V13 and v14. You can only have the latest version." Done.

As Nathan points out in a later post, trouble shooting would be increasingly complicated with each additional mod. If we are talking about a average user, I don't see how checking .dll files in all folders will make things easier for them. That combined with potentially outdated versions of MM running all the cfgs in the KSP directories, that alone would probably would lead to some very obscure issues when described in laymen terms. And then trouble shooting will become more of a hassle.

Also, there is a limit as to how low of a bar you want to set for the usability of something. I disagree that something needs to be set at the level of a nigh-clueless PC user, because they need to step up their game, not make things easier for them. I mean, we are talking about version control on the mods they download... not editing files or creating parts here! If somebody can't keep track of their files, as I stated previously, there is a bigger problem in the mix that has nothing to do with KSP or MM. And no amount of 'oversimplifying' will solve it.

As others mentioned, I have yet to see a mod that uses a custom MM version. And I doubt that would be attractive to people who area already using mods that use "regular" MM.

Version numbers will make everything easier, I promise.

[Majiir]

I should comment on the proposed changes since I had some part in their conception. Before that, I want to remind everyone that sarbian is coping with a genuinely difficult design challenge. He doesn't deserve harsh treatment.

So, on to the reason for a change. ModuleManager's license allows it to be customized for specific applications, which is something I've been wanting to do for a few different projects. The problem is that MM's current design doesn't support customization in the larger sense because every loaded MM plugin will execute on all MM configs and modify all config nodes everywhere. So, my desire was to allow custom ModuleManager instances a chance to be the only instance acting on a particular set of MM configs.

This gets complicated because MM is used for a variety of purposes. Here are a few examples (please suggest others):

1. User-created tweaks. These are traditionally tossed into a file in the GameData directory and can apply broadly to parts from multiple mods.
   
2. Tweak/enhancement packs. A config which adds a mod's parts to the tech tree falls under this category. It's not user-created, but it's user-installed and isn't part of a larger mod.
   
3. External changes applied by a mod to other parts. FAR uses MM to add its modules to the stock wing parts. It needs the capability to apply configs from its own directory to parts from other directories.
   
4. Internal changes applied by a mod to itself. I can't think of an example of this in use today, primarily because it's difficult to fit into the current scheme.
   
5. Configuration of a mod by another mod. This is a particularly tricky one. Someone earlier in the thread mentioned that Kethane uses a MM config to add ModularFuelTanks support. I argue that this is actually an anti-pattern and should be eliminated, but for now it's something we need to support.

It's hard to write a hard-and-fast rule that satisfies all of these needs. As evidenced by this thread, you can't just restrict the root GameData/ folder because that makes #1 (and #2 to an extent) more difficult on users. You can't just allow a mod to "hide" MM config nodes in its own directory, because then things like ModularFuelTanks can't see their own nodes. To satisfy #4 (and maybe #3), you can roll a custom MM plugin which doesn't read configs outside its own directory, but the problem here is that the stock MM plugin in the root directory will also apply its rules. There needs to be some mechanism for blocking some MMs from applying a config while allowing others.

Note that this doesn't mean blocking target config nodes. I can't imagine many scenarios where you'd want to restrict a part config being modified by other MM files. What I advocate is the ability to keep certain ModuleManager configs (which describe the actual modifications) executed only by the desired MM plugin.

I have yet to see a mod that uses a custom MM version.

That's because the current ModuleManager design doesn't allow for custom plugins. This is evidence in favor of these design considerations, not against.

Bear in mind that customized ModuleManager plugins aren't necessarily intended for end users. MM is a powerful tool that has a lot of applications, and many will be completely transparent to users.

[sirklick]

Thank you Majiir for shining some light on the process that lead to Sarbian's original decision. I really do not believe anyone meant to be harsh, though it is obvious the MM holds a special place in all of our rocket fueled hearts. I am part of the camp that was distraught by the change made, but your post has softened my original shock somewhat.

Though I hate to suggest an "Internet Explorer" like way of providing compatibility, perhaps MM could include the ability to parse the first line of each configuration file which includes some information about MM scope.

Ex. (One way)

1. NO LINE or MM selector - assume 1.3 compatibility (at least temporarily) perhaps MM_LOCAL_PREFERED could be the assumption for missing scope.

2. "//MM_LOCAL" - current dir/sub only -ignored by Root assume cfg will be parsed by local MM

3. "//MM_LOCAL_PREFERRED" - Use local/subdir MM if found otherwise assume Root will handle modifications.

4. "//MM_LOCAL_%NAME%" - Named instance of MM within local/subdir

5. "//MM_GLOBAL_%NAME%" - Named instance of MM within dir tree.

5. "//MM_GLOBAL" - Root MM only

This would place a lot of responsibility on moders/developers to properly choose a scope, but would be transparent to the otherwise confused user. The implementation, especially for #3, might be kind of tricky - I have not spent the prerequisite time learning how MM (plugins in general) interact with Unity so this is really just a guess.

[Starwaster]

As others mentioned, I have yet to see a mod that uses a custom MM version. And I doubt that would be attractive to people who area already using mods that use "regular" MM.

That's because the current ModuleManager design doesn't allow for custom plugins. This is evidence in favor of these design considerations, not against.

Bear in mind that customized ModuleManager plugins aren't necessarily intended for end users. MM is a powerful tool that has a lot of applications, and many will be completely transparent to users.

I'm not seeing how replacing the DLL file itself is prevented by program design. That's what's being talked about.

[Majiir]

I'm not seeing how replacing the DLL file itself is prevented by program design. That's what's being talked about.

That only solves a few use cases. If a mod needs custom ModuleManager behavior, telling users to replace their single installation won't work. The custom behavior might conflict with what the regular plugin does, but even if it didn't, you could only have one such mod installed at a time. If you package a custom plugin inside your mod, the unmodified plugin commonly placed in the GameData root will also apply any ConfigNode patches it finds, often breaking configs in the process. The current ModuleManager architecture very much inhibits customized versions.

[NathanKell]

Majiir, thanks for explaining the use cases. Didn't realize this was an issue of wanting customized MM's per mod. Makes more sense now.

The magic-byte solution might work here, as sirklick says (kinda); uncustomized MM would ignore all confignodes starting with a magic character, and the customized MM would process only confignodes starting with one (i.e. trim it and then proceed on the node normally).

It'll be seamlessly backwards-compatible because MM1.3 will ignore the magic-character nodes, and forwards-compatible since old MM configs placed anywhere will still work.

Only issue I can think of off the top of my head is making sure the magic byte is long enough (maybe 1 char not enough?) for all modders, and that modders don't use each others' magic bytes.

Edited November 5, 2013 by NathanKell

[skbernard]

along the lines of what sirklick said, why not use custom named DLLs in gameData and in the first line of configs parse a line to see what custom named DLL to use

ModuleManager_Kethane.dll

Then

(pseudo code)

use (ModuleManager_Kethane.dll)

The "original" MM would need to be modified for that as well and if no dll is specified it reverts to original MM

All the work would be on the devs hands to choose which one to use and general users would just add one extra line to their code and keep the files wherever they like

[NathanKell]

Because then we're back to having to check the URLs of confignodes and then open the files and check their opening lines. Using a magic byte in the confignode name avoids that.

[skbernard]

yeah good point, I didn't see your post before mine as I was typing

[sarbian]

sirklick solution works, but i'll need to list and acces the file. MM don't do that for now since it lets KSP parse the file and use the object KSP create.

If we go with the dll in gamedata then I'd rather use 1 special node name per subdir of gamedata ( @MODULEMANAGER_XXXX or #MM_xxx ). This way I don't have to read files and maintain complex structure to know what I need to parse.

And I would keep it simple either :

- just 1 tag that would stop a MM from a parent directory to process the node of all the files in this dir and subdirs (@MODULEMANAGER_LOCAL).

- or NathanKell solution with something else than the @ for node. Keep in mind that the KSP parser won't read all char so we would need to chose 1 and let the custom MM instance process those only where it's installed.

- something else that don't involve reading file / parsing directory

Or back to the multiple dll thing

[CAPFlyer]

What mods are distributing actual modified ModuleManager.dll files?

It's not necessarily "custom" versions as much as some mods use different self-compiled versions of it and those don't always play nicely. It's been a while since I've had it happen because everyone seems to be more "in line", but I do note that several mods are packaged with outdated modules of MM or they've opened and saved them and that also causes confusion because it gives the file a newer file date and thus makes users think they're getting a newer version even if they're not.

[CAPFlyer]

@CAPFlyer : maybe you should spend some time on the #kspmodders irc channel before telling me that I did not get input first, because "that" idea comes from chats I had with other modders.

Maybe, but #IRC chat is neither a permanent record nor is it somewhere that both users and modders frequent. It's great that you talked to a few modders in this, but you also needed to put this out to the community via the thread that you have specifically for that purpose so that both users and other modders who may not frequent that #IRC channel have a chance to give input, especially when you're talking about a MAJOR change like this that breaks compatibility. As you've seen just by the last few pages, other and non-compatibility breaking solutions have been suggested by others in this thread simply by making it known what you're trying to do and why. That's one of the things with development (and the end I am typically on - testing/QA) that gets missed by even the biggest developers - communication of major changes to a wide group to get suggestions prior to implementation. What you do for one group may fix their problem, but you hurt the bigger group because you accidentally break something else that causes them work they didn't ask for or agree with.

[sarbian]

Why do you thing I made the post ? Did you miss that in the first post : "This version had many change so it may have bugs, don't distribute it with your mods yet but please test it and give me feedback about the changes."

I still beelive my first idea is the better one, even if the transition is painfull. It won't stop me from impementing something else.

[CAPFlyer]

Right, but to test it, anyone who downloads it has make major changes to their installation to get it to work. That is where the problem comes in. Again, you talk to people first about what you're trying to do on the thread instead of releasing it and then expecting everyone to be okay with it. Your first idea may be the better one, but you didn't let people discuss it in this thread and understand why you were wanting to make the change. As you saw, once the reason behind the change was available to everyone, several people not only were okay with it, but those who disagreed with the implementation suggested alternate ways of getting the same result without breaking compatibility. That's how community modding is supposed to work. That's how AAA game development works (within the company obviously and not on public forums). You talk about changes that affect everyone with everyone before you change it.

[Majiir]

May I suggest a sort of "wrapper" node where rules inside it have different parsing rules? I'm not advocating this over any other solution in particular, but I thought I'd throw it into the mix.

Right, but to test it, anyone who downloads it has make major changes to their installation to get it to work. That is where the problem comes in.

This is absurd. It's an opt-in beta test. Of course it's not guaranteed to be painless.

Again, you talk to people first about what you're trying to do on the thread instead of releasing it and then expecting everyone to be okay with it. Your first idea may be the better one, but you didn't let people discuss it in this thread and understand why you were wanting to make the change.

Also absurd. He talked to people (IRC) and then made a thread so people could talk about it more. There's absolutely nothing wrong with his methodology here. Testing isn't about developers doing exactly what you think should be done, and ModuleManager isn't nearly complex enough to warrant a public review of a design spec before releasing a prototype.

[regex]

Right, but to test it, anyone who downloads it has make major changes to their installation to get it to work. That is where the problem comes in.

It's not a problem. Make a copy of your install folder and make the changes. You then have two installs, one using the beta, one using the production.

[Starwaster]

Why do you thing I made the post ? Did you miss that in the first post : "This version had many change so it may have bugs, don't distribute it with your mods yet but please test it and give me feedback about the changes."

I still beelive my first idea is the better one, even if the transition is painfull. It won't stop me from impementing something else.

That post wasn't sufficient. It doesn't change that a small group decided on changes that most were not privvy to because most don't frequent the venue where this was all discussed. This is something you need to proceed very cautiously on. This is development, not maintenance and I'm sorry if this sounds harsh but you should have expected the level of push-back that you got. It was predictable.

[Fractal_UK]

That post wasn't sufficient. It doesn't change that a small group decided on changes that most were not privvy to because most don't frequent the venue where this was all discussed. This is something you need to proceed very cautiously on. This is development, not maintenance and I'm sorry if this sounds harsh but you should have expected the level of push-back that you got. It was predictable.

As far as I can tell then, your issue seems to be that sarbian chose to release his changes as a beta, incorporating suggestions he had received from a subset of the community rather than starting a discussion about the way in which he should proceed.

I would argue, however, that since that discussion is still happening, nothing has been forced upon anyone. The only way the outcome currently differs from your chosen solution is that the beta exists and to my mind, that means you are more informed and more involved not less because you can download the beta, test it out and see what the implications of his changes are, rather than discussing what the changes might be in principle if he adopted each path.

Balancing the needs of different users in this case is obviously complex and I think we need to recognise that at least sarbian is putting some work out there and trying to make some progress.

[Starwaster]

As far as I can tell then, your issue seems to be that sarbian chose to release his changes as a beta, incorporating suggestions he had received from a subset of the community rather than starting a discussion about the way in which he should proceed.

I would argue, however, that since that discussion is still happening, nothing has been forced upon anyone. The only way the outcome currently differs from your chosen solution is that the beta exists and to my mind, that means you ...

No my 'issue' is a little more complex than that but at least re-read the second sentence because it sums up a part of it more accurately than your interpretation.

[sirklick]

It seems like my initial idea -as presented- may be ill-conceived in that it would require a bit too much overhead and complexity to reopen all configs just to read that first line.

If Majiir's wrapper solution would allow similar scoping functionality using the built in node system, it would be preferable to my own. As a bonus, this would allow using custom MM and MM proper in the same config without conflict. One concern that this solution would not cover would be the issue of the user having an outdated version of the globally scoped MM. This -from my understanding- is the issue that sarbian originally wished to fix with his solution.

Ultimately, with the ability to specify scope in configs, modders and plugin developers should probably be discouraged from including MM proper in their packages and just link to this thread as a dependency. This way as long as MM proper is kept backwards compatible, the original scope of "MM as a mod to the way KSP handles configuration" remains, while allowing for the use of custom or old locally scoped versions to be used with and distributed as a part of mod/plugin package.

[sarbian]

I have implemented the easiest solution.

Adding "MODULEMANAGER[LOCAL] {}" to a cfg will make MM ignore all node in the same directory and its subdirectory.

I'll keep the version number in the dll name : http://www.sarbian.com/sarbian/ModuleManager_1_5.dll

I added a message if an old MM or MMSarbianExt are present, asking the user to delete them

If this design seems to be agreed on by most then this will be released later this week as version 1.5

Edited November 5, 2013 by sarbian

[CAPFlyer]

This is absurd. It's an opt-in beta test. Of course it's not guaranteed to be painless.

Again, I wasn't talking about it being painless, my point was he was requiring a major change in how installs are done without an EXPLANATION of why he made this change. The reason for the change wasn't made clear until AFTER there was a lot of kick-back against the change, mainly because people couldn't understand why it was made, which is what adults do. When they are asked to change something, they want to know why so they can make a reasoned decision if they want to do it or not. The way it was presented was "this is the change, you must make it if you are going to continue using ModuleManager," not, "this is a possible change but for now is a branch for this reason and I need help testing it." Remember, this is in the RELEASE[/b[ thread, not the development thread. Any new "releases" are going to be interpreted as trunk releases, "BETA" tag or not, unless specifically and clearly stated why it's not. It wasn't done in this case and that is what caused the kickback.

Also absurd. He talked to people (IRC) and then made a thread so people could talk about it more. There's absolutely nothing wrong with his methodology here. Testing isn't about developers doing exactly what you think should be done, and ModuleManager isn't nearly complex enough to warrant a public review of a design spec before releasing a prototype.

Actually, no he didn't. He already had this thread and he made a post that, as I said above, did not clearly explain what the purpose of the release was of the reason for the change. Testing and development is about following a PROCESS that includes communication that wasn't made.

It's not a problem. Make a copy of your install folder and make the changes. You then have two installs, one using the beta, one using the production.

Okay, but why should I do it? Again, the issue isn't that he made a compatibility-breaking change. It's that he did it without clearly communicating what he was trying to achieve from the change, why it was requested, and what we need to do to properly assist in testing it. He followed a "shoot first, ask questions later" approach that doesn't build confidence or cooperation from the community, and he got the exact result expected - most people rejected it.

[Majiir]

I don't think the exception tag is an ideal design, but it solves the immediate issues satisfactorily. When I get some time to work on it again, I'll be taking advantage of this in my internationalization plugin. Cheers, sarbian, for your efforts.

Testing and development is about following a PROCESS that includes communication that wasn't made.

There is a few easy change in this version and a complex one, so please read the post fully before you install or upgrade it.

This version had many change so it may have bugs, don't distribute it with your mods yet but please test it and give me feedback about the changes.

...

I spoke with a few other modders and they had concerns about MM. One of this concern is that having MM always installed in the base dir makes it easy for one mod to overwrite it with an old version. And if mods install it in their folder you get multiple instance of it running, each of them applying all the available patch and making a mess of thing. They had some ideas to avoid this and I implemented them. It makes the upgrade a bit more complex and you'll have to read a bit. I hope I can make it clear enough.

This is the original post in the thread. He goes on to detail the change and gives a set of examples. This is exactly the development process that you keep harping on about. You should instead have responded with "I see that you've given a rationale for these changes, but I don't fully understand it. Could you try explaining it in different terms?" Sarbian is working hard to please a lot of parties and he doesn't deserve your attitude.

[regex]

Okay, but why should I do it?

To help test his changes and offer feedback before he decides on whether to release the features. It's something that adults do with beta software.