Kerbal Inventory System (KIS)

Kerbal Inventory System (KIS)

1M Downloads

Find better way to detect if items can be stacked

ihsoft opened this issue ยท 5 comments

commented

Main problem with stacking is that whole stack has just one confing for all the items. I.e. if part has internal state that can change then stacking becomes a problem. Currently KIS simply restricts stacking of parts that:

  • Have resources.
  • Have modules that are not whitelisted in the config.

There could be better ways to deal with it. E.g. checking if config nodes are really different. Or saving configs for every item in the stack.

commented

I don't know about resources, but when it comes to other modules, I think the solution is to allow other mods to declare conditional stackability of their modules, and let them decide when the user actually tries to stack the part.

It is hard for KIS to unilaterally try to determine stackability -- whether the config nodes are identical or not does not necessarily guarantee if it makes semantic sense to stack or not. It really isn't KIS's business to try and interpret the internal state stored in other modules. Let other mods tell KIS how to handle things.

Suppose KIS provides not only whitelist for straightforward stackable modules as per #205 but also has a list of conditionally stackable ones. KIS defines interface (say, IKISConditionallyStackable) that such part modules should implement. It requires:
static bool tryStack(ref ConfigNode tgt, ConfigNode src = null)

KIS will call this when the user tries to drag/stack X number of src onto Y number of tgt -- same part name, possibly different internal states in the module. The counts (X,Y) are unimportant, the question is simply whether or not stacking should be permitted.

tgt is passed by reference and allows the other mod to change/reset the internal state to whatever it considers "consistent" in order for stacking to proceed.
src can be omitted, which is equivalent to "can I stack tgt with itself?" (useful in VAB/SPH)

If the return value is true, then KIS can go ahead and make a stack of X+Y number of tgt, else stacking is disallowed.

Use Cases

kOS

See: KSP-KOS/KOS#1539 KSP-KOS/KOS#1958
The folks over at kOS are concerned about losing data / propagating incorrect data if they allow KIS to stack parts. Looks like they have considered two possibilities for their KOSNameTag module:

  • force the name tag to empty when stacking, or
  • only allow stacking if the name tags match

The proposed solution would allow kOS to implement both, and let their users choose based on user pref settings, rather than forcing a single solution.

Collapsible KIS containers

SEP has a "cardboard box" KIS container part. Its part.cfg declares volumeOverride = 3 and maxVolume = 750 which results in "hammerspace" -- it can contain a larger volume than itself! Based on the flavour text in the part description, it is clear that the developers intended a part that could be "folded" to take up less space when empty (which requires plugin, not just what they put in the part.cfg)
The proposed solution would allow such a plugin to conditionally allow only folded, empty containers to be stacked (probably should also force KIS inventory name to empty string when stacking).

Procedural parts

Suppose you have some simple procedurally parts like:

  • fixed ladder (similar to pegasus), param: number of rungs (ladder height)
  • structural truss, param: length

With parts like these, a player planning to construct a station/base in-situ could want to bring several of each size. Separate stacks for each size makes perfect sense.
Tweakscale probably falls under this category too.

Additional Considerations

It might make sense to let the other mod set a prefix to the part name to help players differentiate between stacks, e.g. Station Truss | 2m vs Station Truss | 5m in the procedural parts use case, or <part name> | <KOSNameTag> for kOS.

An additional out string errMsg param may be useful for KIS to pass on information from other mod to the user about why stacking failed.

commented

A new interface for the new parts is in general a good idea but it's not feasible until the whole mod is refactored. For now the only way to implement a KIS interface is adding the mod's DLL as a reference into the project. This results in a big set of issues because it creates a hard dependency. It's ok when the mod can only work when there is KIS mod installed (e.g. Surface Experiment Pack), but for the mods that consider KIS as an optional mod (e.g. kOS) it's not feasible. There is a long term plan to refactor KIS to solve this problem. The API-like approach is being developed in KAS 1.0 for now.

And there are many stock parts and third party mods that will never support this interface. So, KIS has to have a way to decide which part can be stacked. However, a part needs to have a way to override the decision. For now it can be done via a KISModuleItem module, but this module is not quite convenient for the purpose.

commented

Perhaps GameEvents Extension could be of help then? To avoid the need for hard dependencies / juggling with reflection. If my understanding is correct, it would look something like this:

KIS defines the following event.
public static EventData<Part,Part,Dictionary<string,bool>> onKISTryStack;

The first two Parts analogous to tgt and src as suggested previously, except the whole Part (prefab) is has to be passed along.

The dictionary keys are PartModule.moduleNames of modules contained in the part that have declared themselves to KIS as conditionally stackable (via MM config); values initialized to false (disallow).

KIS initializes the event "somewhere early"
onKISTryStack = new EventData<Part,Part,Dictionary<string,bool>>("onKISTryStack");

Other mods that want to support conditional stacking look for the event:
var onKISStackEvent = GameEvents.FindEvent<EventData<Part,Part,Dictionary<string,bool>>>("onKISTryStack");

If the event is not found, it means that KIS does not exist, so the other mod can simply carry on with their business as usual. But if KIS exists alongside the mod, then they'll want to register for the event:
if (onKISStackEvent != null) onKISStackEvent.Add(onKISStack);

The event handler would be void onKISStack(Part tgt, Part src, Dictionary<string,bool> allowStack)
which would

  • check if there are part modules that they care about, otherwise just ignore the event
  • if there are modules of interest, check their internal state
    • set the corresponding allowStack dictionary entry
    • modify the part module state in tgt if needed for consistency

So it'd behave a bit like stock onAttemptTransfer -- where mods can interrupt and override the transfer by setting overrideTransfer. Except KIS will only stack the part if every module allows it. (And that we use only types that are already known to both KIS and the other mod(s).)

Could this work?

commented

Well, it may work for a few modders. But this solution looks to me a way too complex and error prone. The "events", in general, are not supposed to return a result due to their "fan-out" nature. There are cases when it may work, but if there is a better way to solve it, I'd choose this way.

As I mentioned before, the interface approach in general is OK to me. It just needs to be implemented in the right way. And, what is more important, the solution you propose doesn't eliminate the root of the issue: if a part doesn't explicitly say if it's ok or not ok to stack, then nobody can help KIS to make the decision. I.e. there must be a "default" behavior that is expected to be correct in the majority of the cases. For now it's implemented via the whitelist, which is far from being perfect.

That said, this issue is about the "default behavior". However, all the ideas are welcome :)

commented

Well, just throwing out ideas here since I'd love to see such functionality be supported. =)
Though you're right, in any case what I've brought up only takes care of those cases where the other mod would like to provide closer integration beyond just whitelisting. Sorry if this wasn't quite the right issue to file under!