Skip to content

New part module control system draft#3182

Draft
sug44 wants to merge 1 commit into
KSP-KOS:developfrom
sug44:part_module_system
Draft

New part module control system draft#3182
sug44 wants to merge 1 commit into
KSP-KOS:developfrom
sug44:part_module_system

Conversation

@sug44
Copy link
Copy Markdown
Contributor

@sug44 sug44 commented May 17, 2026

In general this PR adds a more convenient way of accessing the part module system, also allowing to access fields, events and actions by the internal variable name and not the name shown in the GUI. GUI names can sometimes be the same for 2 parameters, making one of them inaccessible(#2939), also GUI names can change depending on the state of the module, while internal variable names stay the same(see example below).

Changes :module suffix on parts from returning a list of module names to returning a new ModuleLexicon structure. The :allmodules suffix alias that does the same thing remains. The new ModuleLexicon structure when printed still gives the same information as the previous suffix, but you can't index and iterate on it the same way as before.
Using ModuleLexicon you can access modules on a part with a suffix syntax, optionally without the leading "Module" string.

Module structures now have :fields, :events, :actions suffixes, returning FieldsLexicon, EventsLexicon, ActionsLexicon. Those lexicons allow to access each parameter by their internal variable name using suffixes.

As an example for the whole thing:
cockpitpart:modules:command:events:changeControlPoint(). does the same thing as cockpitpart:getmodule("ModuleCommand"):doevent("control point: default")..
This example is especially bad for the current module control system, as the event GUI name changes after the event is called, and the name indicates the current state of the control point, not the state of the control point after calling the event.

As this is a rather large addition and there no documentation for it yet, I'm making this a draft until approved by the maintainers.

@JonnyOThan
Copy link
Copy Markdown
Contributor

This is really cool. I don't love changing the meaning of an existing suffix (part:modules) but I was toying with the idea of changing fields/events/actions to using the internal names at some point, which would also be a breaking change.

I don't have any concrete suggestions, except maybe ALLMODULES should just be renamed to MODULENAMES and maybe remove the ModuleLexicon and MODULES suffix entirely? If we're going to break it, break it hard. Is there anything the ModuleLexicon really provides that isn't already available from the existing API?

@nuggreat
Copy link
Copy Markdown

While using the internal names solves some things it introduces new problems. If for example I download a mod that uses Russian names internally for things if kOS uses the UI names would never see that as they would have been localized to English where as trying to use them with just the internal names I would be shooting blind for anything I attempted to do with the module. Also the names changing as a result of a changed state is some times the only way to know the actual state of the part which you also loose by switching to the internal names.

A problem with using a lexicon for this is the same problem you get when getting modules by name where some parts have several copies of the same module such as the ISRU which is why kOS added the ability to get modules by index. To solve this it should be easy enough to also supply a list of modules which could take the place of the :MODULES suffix on parts as I have seen plenty of people assume that was a list of modules as apposed to what it is a list of module names though that would require a change or two to the :GETMODULE() suffix to not break things for those who get modules by indexing the list of module names.

@JonnyOThan
Copy link
Copy Markdown
Contributor

JonnyOThan commented May 18, 2026

If for example I download a mod that uses Russian names internally for things

This seems extremely unlikely because the internal name is derived from the name of the field in C#. Someone would have had to written the code itself in Russian for that to happen. It’s possible, but I’ve never ever seen it.

Good call on multiple copies of the same module, I think that’s a strong reason to not provide them as a lexicon.

@nuggreat
Copy link
Copy Markdown

nuggreat commented May 18, 2026

While I also don't know of any mod that uses something other than English internally out there the reverse case does apply though to a lesser degree. Those who don't have a good grasp of the English language but can muddle through would have much greater friction working with enforced English internal names. That said as kOS is documented exclusively in English it is a less strong case. I also haven't looked at internal vs shown names and while they are likely similar there are also likely to be cases where there isn't a clear link between the internal name and the shown name which will be a problem for some people.

I remember kOS going to some pains to use localized names for things like planets as some planet packs use kerbin as the internal name despite the fact the planet they are adding is not kerbin because some mods break because they hard coded "Kerbin" so they used localization to mask the internal name.

I am also not against getting modules with a lexicon just pointing out that like with our current way of getting them by name there should be a similar pattern to get them by index.

@sug44
Copy link
Copy Markdown
Contributor Author

sug44 commented May 18, 2026

ModuleLexicon doesn't provide anything new other than sweeter syntax and a bit of code readability. It's also pretty easy to add a way to index ModuleLexicon, so that it does the same thing :getmodulebyindex suffix does, solving the problems nuggreat mentioned with multiple modules on a part and people trying to get modules by indexing the modules suffix. And rename it to ModuleGetter or something to not be confusing. If we mentioned breaking changes, maybe this "ModuleGetter" can be the only way to get modules? It would simplify the API by removing getmodule, getmodulebyindex and possibly hasmodule(modules:haskey) and allmodules(modules:keys) suffixes. All of these can be replaced by the "ModuleGetter" structure.

print part:modules.
ModuleGetter, containing keys:
[0] ModuleCommand
[1] ModuleReactionWheel
set command to part:modules:command.
set reactionwheel to part:modules[1].

If you think this is not worth introducing a breaking change, then I could just remove the ModuleLexicon structure.

@sug44
Copy link
Copy Markdown
Contributor Author

sug44 commented May 18, 2026

There's also an option to do the ModuleGetter thing, and leave it alongside the current system, your call.

@JonnyOThan
Copy link
Copy Markdown
Contributor

Moving all module API under a :modules suffix is fairly attractive, but quite disruptive to existing scripts. I think there's also a middle ground where we try to keep as much backwards compatibility as we can (likely just renaming allmodules to modulenames and then using modules for this new thing). And maybe that can be a transition period before the old stuff just gets removed (or not).

@nuggreat
Copy link
Copy Markdown

The way I see to do a non breaking transition would be to change MODULES to return a list of all modules on the part and also modify GETMODULE() so that it can take either a string or a module which it then internally casts to a string. This is because the way I have seen people use MODULES to get module names has been variations of this basic pattern

LOCAL moduleName TO somePart:MODULES[i].
LOCAL module TO somePart:GETMODULE(moduleName).

Personally I prefer getting modules by name explicitly as it is more robust against different mod sets which is helpful for people sharing code between different installs where as going with explicit indexing is more likely to break between different peoples games/mods especially as in most cases working with name is the better option as it is rare parts that have more than one instance of a module. Which is also why I am not against having the lexicon form available as something new in addition providing a list of modules directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants