Let's talk about part/modules upgrades

[Gotmachine]

Hi all.

I've released the UpgradesGUI plugin, whose function is to complete the part/module upgrades feature introduced in KSP 1.2. Currently it has two main features :

• Provide a better integration of upgrade in the VAB/SPH part tooltips by showing upgraded part/module stats, something that is really lacking in stock, and very confusing to players.
• Provide a GUI interface to allow the player to select which upgrades should be activated. I've done this by hacking the tooltips to make widgets clickable. This allow for a nice stock-integrated GUI.

Of course I encourage mod authors to use upgrades in their parts and to recommend or even bundle my plugin. But maybe we could go further and overcome the current upgrade implementation limitations :

Upgrades can't reliably & easily be used to enable/disable modules :

Being able to set the enabled / isEnabled properties in modules instances would allow upgrades to effectively add/remove modules. This has been requested a few times (by @MysterySloth, @sebi.zzr, @Skalou). Combined with the upgrade selection feature in my plugin, this would provide in practice a "module switching" feature to KSP, something that no common switcher plugin (IFS, B9, Firespitter...) currently allow. I know that Kerbalism by @ShotgunNinja does it, maybe also SSTU by @Shadowmage.

Therefore I propose a "ModuleEnabler" partmodule that would allow this in a simple way :

MODULE
{
  name = ModuleEnabler
  targetType = ModuleResourceConverter // module to control
  targetEnable = false // is this module enabled initialy	 
  showWidget = false // in the tooltip modules list, is the widget visible
  showInfoInUpgrade = true // in the tooltip upgrades list, do the upgrade widget show the whole module info (or just the name)
  // Optional fields used to be sure the right module is targeted if multiple modules of the same type exist in the part :
  targetIdField = ConverterName // targeted module field name
  targetIdValue = Lf+Ox // targeted module field value
    
  UPGRADES
  {
    UPGRADE
    {
      name__ = ISRU_Upgrade
      description__ = Add an ISRU
      targetEnable = true // enable the ISRU module
      showWidget = true // show the ISRU module in the part tooltip module list
      showInfoInUpgrade = false // only show "ModuleResourceConverter" in the part tooltip upgrade list (to unclutter it)
    }
  }
}

Upgrades can't be used to change/switch RESOURCE{} nodes

This could be done in a special "ModuleUpgradeableResources" partmodule. Note that the stock behaviour with nested ConfigNodes is to delete all the nodes of the type used in the UPGRADE (here "RESOURCE" nodes) in the module, and replace them with those defined in the UPGRADE node. Combined with the selection GUI, this allow for resource switching.

MODULE
{
  name = ModuleUpgradeableResources
  
  // Resource added by the module when no upgrade is applied
  RESOURCE
  {
    name = ElectricCharge
    amount = 1000
    maxAmount = 1000
  }
    
  UPGRADES
  {
    UPGRADE
    {
      name__ = Monoprop_tank
      ExclusiveWith__ = resource // mandatory so only one resource set is enabled at a time
      description__ = Add a monopropellant fuel tank
      // replace the resources added to the part by the module with these ones
      RESOURCE
      {
        name = ElectricCharge
        amount = 1000
        maxAmount = 1000
      }
      RESOURCE
      {
        name = MonoPropellant
        amount = 40
        maxAmount = 40
      }
    }
    UPGRADE
    {
      name__ = Xenon_tank
      ExclusiveWith__ = resource // mandatory so only one resource set is enabled at a time
      description__ = Convert the whole part to a xenon tank
      // replace the resources added to the part by the module with these ones
      RESOURCE
      {
        name = xenonGaz
        amount = 40
        maxAmount = 40
      }
    }
  }
}

Also note that I'm planning to give a way for players to open the upgrades selection GUI (the whole part tooltip) for already placed parts in the editor. Still not 100 % sure I can manage this, so no promise.

What do you guys think of all this ?

Would those modules be useful for anybody ?

Edited April 19, 2017 by Gotmachine

[Shadowmage]

One major problem that you are going to run into is that the 'moduleIsEnabled' flag only works on a handful of stock PartModules.  Notably it does not work with ModuleEngines*, and several other 'common' part-modules.

Second, even on the modules that do support 'moduleIsEnabled', they often will do initialization even when disabled.  This can cause problems if, for example, the model transforms that they rely upon are not (yet) present in the part (e.g. from mesh-switching); the module will attempt to initailize, and then null-ref partway through.

I would say that before you could make much meaningful progress on -this- mod/question, those two problems need to be fully fixed in stock.  -All- partmodules need to support the 'moduleIsEnabled' flag, and they need to only initialize when the 'enabled' flag is actually enabled (which means they will need a way to re-initialize when the user toggles that flag in the editor/etc).

[ShotgunNinja]

What I know:

• PartModule.isEnabled is the KSP PartModule flag indicating if the module fields should be added to the part UI
• PartModule.enabled is the MonoBehaviour flag indicating if the update()/fixedUpdate()/onGui() functions should be called - it isn't serialized, so you have to set it to false after load
• Most modules only do work inside update()/fixedUpdate()/onGui() and don't manipulate animations or rendering objects, so they stop doing whatever they usually do
• For the others, you need special hacking. These are the ones I'm aware of:
  • ModuleEngines*: just call the Shutdown() method when disabling it
  • ModuleDeployable*: stop all animations on the part
  • ModuleLight: if animation-driven reset it to frame 0, else set enabled=false in all Light objects.

[Gotmachine]

5 minutes ago, Shadowmage said:

One major problem that you are going to run into is that the 'moduleIsEnabled' flag only works on a handful of stock PartModules.  Notably it does not work with ModuleEngines*, and several other 'common' part-modules.

I need to verify, but seems to me that setting the "enabled" MonoBehaviour property to false lead to Update/FixedUpdate/OnGui not being called. Isn't that enough to prevent all interactions ? Then the only thing that would need to be dealt with are KSPField, KSPEvent and KSPAction, to toggle their UI visibility (but I was under the impression that they don't show up if enabled = false, since OnGui isn't called).

13 minutes ago, Shadowmage said:

Second, even on the modules that do support 'moduleIsEnabled', they often will do initialization even when disabled.  This can cause problems if, for example, the model transforms that they rely upon are not (yet) present in the part (e.g. from mesh-switching); the module will attempt to initailize, and then null-ref partway through.

Of course this wouldn't support modules that do mesh switching, everything needed for the module to work should be present in the part.

29 minutes ago, Shadowmage said:

they need to only initialize when the 'enabled' flag is actually enabled (which means they will need a way to re-initialize when the user toggles that flag in the editor/etc).

The idea is to only prevent them to Update/FixedUpdate/OnGui, not to prevent Awake/Load/Onload/Start/OnStart. By setting enabled = false in the ModuleEnabler own Update/fixedUpdate, so after everything is initialized.

6 minutes ago, ShotgunNinja said:

For the others, you need special hacking. These are the ones I'm aware of:

• ModuleEngines*: just call the Shutdown() method when disabling it
• ModuleDeployable*: stop all animations on the part
• ModuleLight: if animation-driven reset it to frame 0, else set enabled=false in all Light objects.

 

The ModuleEnabler will disable the target module only in the editor (not in flight), wouldn't this will narrow down these issues ?

[ShotgunNinja]

23 minutes ago, Gotmachine said:

The ModuleEnabler will disable the target module only in the editor (not in flight), wouldn't this will narrow down these issues

PartModule.enabled is not serialized. You will need to re-disable the modules every time the vessel is loaded.
So even if you 'configure' only in the editor, you will still need to 'enforce that configuration' in flight.

 

24 minutes ago, Gotmachine said:

but I was under the impression that they don't show up if enabled = false, since OnGui isn't called

You need to use PartModule.isEnabled instead. When it is false, KSP disable the fields and actions of that module.

[Shadowmage]

19 hours ago, Gotmachine said:

The idea is to only prevent them to Update/FixedUpdate/OnGui, not to prevent Awake/Load/Onload/Start/OnStart. By setting enabled = false in the ModuleEnabler own Update/fixedUpdate, so after everything is initialized.

 

19 hours ago, Gotmachine said:

Of course this wouldn't support modules that do mesh switching, everything needed for the module to work should be present in the part.

Well then sadly, I cannot see how this would be of any use for 'module-swtiching'.   (I would probably label it more properly, as a 'module disabler')

At least two things are needed for any proper implementation of 'module switching'.

1.) The module is only present on the part when it is actually supposed to be.  This means it is not 'there but simply disabled'.  It does not 'still run its init code even when disabled'.  The module is either present when it is needed, or it is not present at all.  This is for both performance and compatibility reasons (less modules and update calls = less overhead for disabled modules = higher performance;  modules being added/removed properly = better compatibility with other mods, as those mods won't think a part is an engine just because it has a 'disabled' engine module on it).

2.) Config support -- to enable dynamically-added/enabled/disabled modules to still be patched through MM, and still have access to all proper config-file based utilities.  This is needed for widespread and general compatibility with the modding community.
 

From my various and sundry attempts at developing a proper module-switching implementation I can tell you that there are several problems that will need to be fixed in stock code before it can actually be implemented in a non hacky fashion.  Mostly regarding to part module loading/saving mechanics and how part-modules are restored from a combination of prefab and persistence file.

If you are not working towards a proper implementation of module-swtiching, then I wish you the best of luck.  Not something I would have any interest in or use for, but there might be others willing to live with the constraints.

[Gotmachine]

Indeed, the module name is "ModuleEnabler". I understand your point of view but as you said, proper add/removal of module instances at runtime is near impossible. On the other hand, we have a more or less clean way of enabling/disabling them. From a functional point of view this achieve the same thing.

As for the potential issues :

• I'm pretty sure that the "overhead" (I assume you mean memory footprint) would be very low if measurable at all.
• Since disabled modules don't call Update/FixedUpdate/OnGui, there is no performance difference.
• On your compatibility example, I'm not sure I follow you. Are you talking about code compatibility or MM patching ? In any case, having a module switching system will induce complexity for mods that want to interact with a switchable module, no matter how its done.
• Maybe the specific code issues/features, when identified, could be dealt with in an interface that other plugins could implement in their custom modules.
• Unless there is something I missed, the proposed modules are fully compatible with MM. But I agree that the upgrade configs are a bit tedious to write (compared to what we are used to with IFS/B9/Firespitter).

My point is : since 1.2, we have a stock upgrade system that is able to track what is essentially "part variations". On the other hand, this is is something that the modding community need and already do heavily. Count the number of plugins that provide resource/texture/mesh/module switching : B9, Firespitter, IFS, Kerbalism, BDB, and so on... Don't you think it make sense to try to link those features to the stock upgrade system ?

My idea is to do this in incremental steps and see what are the issues one at a time. For two reasons : I'm not a super skilled programmer, and I don't have a lot of free time.

Also, I was thinking about what you said about mesh switching. I looked into the firespitter code and it doesn't add/remove components/objects, it only enable/disable them. Since they exist, why would referencing them cause nullrefs ?