Open NCommander opened 7 years ago
There is already an issue for this: https://github.com/KSP-KOS/KOS/issues/1539, where I stressed that nametags are persistent data and should not be stackable.
In the meantime, KIS has decided to whitelist the TweakScale module to stop people from complaining.
While technically it is wrong to lose/overwrite name tags inside a container, I have the impression that stacking of untagged parts is the more common usage and lost stackability is perceived as a hard to find bug by many users. So I'll change my vote and say the whitelisting would be a good compromise.
Can you explain this in a way that makes sense to someone who's never used KIS before? I've never used it, and I don't think the other kOS devs have either.
KIS allows an engineer kerbal to remove and attach parts in the flight view. These parts can be stored in the kerbal's inventory or in containers, and are allowed to stack if they have no persistent information, or discard-able persistent information. When a part is removed from a vessel, it no longer exists as part of that ship, but under an inventory slot in the kerbal/container which contains a full copy of its persistence information.
The intent is that items like fuel tanks, and such when placed into inventory would keep the remaining amount of fuel on hand and such. The problem occurs because kOS adds a persistence tag to all parts (KOS's name tag). This additional bit of persistence means that KIS will not allow the item to stack in a slot. This becomes problematic because this tag attaches to struct connectors and other items that are very light but should be able to carry a lot of. At least in my use case, when building space stations, I often send an engineer up with a load of struct connectors after docking to make the thing more ridge; this can't be done any other way because up until docking, they're two separate vessels.
If you've ever played Minecraft, KIS inventories basically work the same way; you have a limited number of slots, but some items can stack as long as the total capability isn't exceeded. The ModuleManager file basically tells kOS that the name tag can be discarded when a part is placed into inventory, which also makes sense from a kOS point of view; a part in a container isn't part of an active vessel.
The only down side to this is if I name a part, remove it from the vessel, and then re-attach it, the name tag gets lost. Given I can reset those by hand in the flight view, that isn't exactly a big deal. Does this help explain it better? I can put images together if a more detailed explanation is still needed.
Just a small correction: putting something in a container preserves persistent data, only when multiple instances of a part are stacked in the same inventory slot they become identical.
So, given the thoughts regarding persistent information, can we some how confirm that there will be no adverse affects by white listing the name tag module? Mostly I want to clarify your final point: if you store a part with a name tag, is the name tag lost, or does one of the name tags in that stack get associated with all of the parts in that stack? If the tag has any persistence, such that it unpredictably gets applied to other parts in the stack, I would not be in favor of adding it to the white list. In that instance, I would prefer we offer an optional MM config that removes the name tag from parts that can be stored using KIS. However, if the worst case scenario is that a name tag that was set on a part is lost when stored I think I'm in favor of adding the name tag module to the white list.
It would really help speed up the adoption of this change if one of you could make a pull request adding the required MM patch. I don't think anything needs to be done in C#, just as a .config
file in our Resources\GameData\kOS
.
Given that @pellinor0 just answered one of my questions, I think I'd lean towards offering a patch to disable nametags on KIS stackable parts. Or perhaps a series of patches for a subset of those parts. I presume that there are some parts that can be stacked for which the name tag is potentially useful (like parachutes, assuming they can be stored) at which point I think it makes sense to leave the decision up to the user what kind of behavior they prefer. I suppose we could even include the white list config file, and give them the option to enable it.
The major complication there would be to document it in a way that lets KIS users know there are advance configuration options via config files, otherwise users won't know that there are methods for making those changes.
If there are parts whose name tags are not "potentially useful", why bother to remove them? They should be empty anyway (unless someone considered them useful and will miss the tagging function).
I'm much less concerned with loosing data than I am with accidentally propagating incorrect data. So if we could guarantee that KIS would always spit out a part with an empty tag, I'd consider that an acceptable loss.
I guess you're right, we can assume that parts for which name tags provide no benefit no name tag is likely to be set on any of those parts. But it doesn't look like we can differentiate between parts that have a tag and may be stacked safely, and parts with a tag which cannot be stacked safely.
As I see it, we have 3 choices:
I'm in favor of the third option, defaulting to the second option.
Am I missing options? Do others see using other options as more beneficial?
I should add: is there anything other than resources/mod modules that prevents a part from being stackable? Like size? I presume the small 0.625m engine for example is able to be stored, and that it is stackable, but I don't know about other larger parts (like engines that don't have an alternater with EC storage). What about landing gear, or structural components?
Restrictions of stacking are not a gameplay mechanic (the gameplay limitation is done by volume) but purely a safeguard against messing up persistent data. And you are right that stacking part tags means propagating wrong data.
I don't see a robust/reasonable way for a MM config to decide if a part will be stackable.
Actually emptying the tags might be a good solution. And the best place to do this is probably KIS (ping @ihsoft ).
You compared it to Minecraft's inventory, but in Minecraft if two things have the same "extra data" value they can get stacked in some cases (i.e blue wool won't stack with white wool, but it will stack with other blue wool. Wool is stackable if and only if its color field (which is really the damage field, weirdly enough) matches.)
Do two parts which both have an empty string as their kOS name tag stack in KIS? I'd rather see a solution where parts do NOT stack if their tags differ, but DO stack if their tags match. The majority of the time the kos name tag is the empty string. Only the parts you actually give a name to would refuse to stack that way.
Is there a config option that would make it work like that?
Moved ahead to v1.1.1 because this is currently do-able by a modulemanager workaround patch instead of having to change the kOS code, and we are trying to get a v1.1.0 release out.
kOS adds a new persistence tag for naming modules so they can be used by CPUs. Unfortunately, this additional persistence information causes kOS to break KIS because the former does not declare itself whitelisted. The whitelist means that if a part is stored into a container, it can forget the kOS name tag and such (which is fine, since we can edit those in the flight view).
There's information on KIS how to update MM https://github.com/ihsoft/KIS/issues/205 for it here. I don't have a C# development environment to do a pull, but whitelisting KOS should work. Untested, but I think a block like this will allow kOS and KIS to play nicely with each other
@KISConfig:AFTER[KIS] { @StackableModule { moduleName = KOSNameTag } }
If someone can shoot me a good guide to building kOS on Linux, I can try to put a pull up myself.