KSP-KOS / KOS

Fully programmable autopilot mod for KSP. Originally By Nivekk
Other
697 stars 230 forks source link

A PartModule contains two fields with the same name, however PartModule:SETFIELD and GETFIELD can set/get the value of only the first occurance. #2939

Open mg-badman opened 3 years ago

mg-badman commented 3 years ago

In some cases, modules happen to have fields that have the same name.

In such a case, the SETFIELD and GETFIELD functions will only obtain the first encounter of the field, but completely ignore the second one (or any potential other ones).

example: with the '24-77 "Twitch" Liquid Fuel Engine'

image

there are two fields named "Throttle"

when i run the following command on my ship to observe that engine's ModuleEnginesFX module print ship:parts[13]:getmodule("ModuleEnginesFX").

the output shows:

ModuleEnginesFX, containing: LIST of 13 items: [0] = ["value"] = "(settable) fuel flow, is Single" [1] = ["value"] = "(settable) prop. requirement met, is Single" [2] = ["value"] = "(settable) thrust, is Single" [3] = ["value"] = "(settable) specific impulse, is Single" [4] = ["value"] = "(settable) status, is String" [5] = ["value"] = "(settable) throttle, is Boolean" [6] = ["value"] = "(settable) throttle, is Single" [7] = ["value"] = "(settable) thrust limiter, is Single" [8] = ["value"] = "(callable) shutdown engine, is KSPEvent" [9] = ["value"] = "(callable) toggle engine, is KSPAction" [10] = ["value"] = "(callable) shutdown engine, is KSPAction" [11] = ["value"] = "(callable) activate engine, is KSPAction" [12] = ["value"] = "(callable) toggle independent throttle, is KSPAction"

From the above, entries 5 and 6 are both "throttle". When I attempt to use SETFIELD or GETFIELD, only affects entry 5, the Boolean. Unfortunatelly, because of this I cannot set the throttle of the engine.

I suspect that this may also be the case with KSPAction/KSPEvent entries, if their names repeat within a single PartModule object

mgalyean commented 3 years ago

You can access those by array index (iirc, I figured it out once), but the main aspect of this issue is valid in that multiple similar items are not handled well. Perhaps multiple similar items should be wrapped each in their own array. Which they may be, but the interface is lumping them together in an awkward way

Dunbaratu commented 3 years ago

I really do hate when mods do this, because it's asking kOS to support a massively horrible UI design on the other mods' part. Showing the player two things with the exact same name is awful.

I guess one possible solution is to let you alternatively use the underlying secret hidden field identifier the game normally never shows the player, which does indeed have to be unique because those field names are two members of the same PartModule class.

nuggreat commented 3 years ago

This particular issue related to the independent throttle is likely to be partly solved by PR #2881 specifically the "CUSTOMTHROTTLE" suffix for the structure being added. But the general issue of several things in the same module of the same name might be an issue going forward though. But as I don't think we have had it in the past and the independent throttle is a stock thing we are unlikely to see it in other mods.

EDIT: The simple longer term solution would be to do the same thing we did with modules and add a way to get the action/event/field by index as apposed to by name.

JonnyOThan commented 3 years ago

I really do hate when mods do this, because it's asking kOS to support a massively horrible UI design on the other mods' part. Showing the player two things with the exact same name is awful.

I think this isn't actually a mod, it's just a really advanced, relatively new stock feature.

mg-badman commented 3 years ago

hello,

I really do hate when mods do this, because it's asking kOS to support a massively horrible UI design on the other mods' part. Showing the player two things with the exact same name is awful.

It isn't a mod, it's a core game feature to set fixed throttle on that engine using the slider

You can access those by array index (iirc, I figured it out once), but the main aspect of this issue is valid in that multiple similar items are not handled well. Perhaps multiple similar items should be wrapped each in their own array. Which they may be, but the interface is lumping them together in an awkward way

I tried that, but couldn't figure out how to do it, could you please provide an example? The list returned from :GETMODULE is a list of strings, rather than a list of module entries

The simple longer term solution would be to do the same thing we did with modules and add a way to get the action/event/field by index as apposed to by name.

It sounds like a decent solution (using both name and index) - the entries under a specific part and module always seem to appear in order, and given that it remains the case, the user would iterate to find the second occurance of "throttle" which would always be the slider. It's not elegant or anything, but it would get the job done

mgalyean commented 3 years ago
You can access those by array index (iirc, I figured it out once), but the main aspect of this issue is valid in that multiple similar items are not handled well. Perhaps multiple similar items should be wrapped each in their own array. Which they may be, but the interface is lumping them together in an awkward way
I tried that, but couldn't figure out how to do it, could you please provide an example?
The list returned from :GETMODULE is a list of strings, rather than a list of module entries

I could have been more clear, mea culpa. I know you can do a SHIP:MODULESNAMED(...) to get the modules of a given name in the craft into a list, then iterate through looking at module:PART to see if it is the part you want, then accumulate them into an array for that part and use that. Grindy to be sure. Let me poke around at the prompt to remember how I iterated more directly. I remember I was dealing with an engine with two modes and a separate thrust limiter and separate gimbal control for each. I'm pretty sure I found a more direct way to iterate over those. I may be misremembering, but I'll get back to you either way.

Ok, the best way is to use the MODULESNAMED suffix on the part itself (not from ship level). This will return an array of same-named modules for that part. I haven't found the order of of the modules to ever vary, but since there is no way to uniquely identify the module other than its rank order in this array you'd need to sort out which is which and stick to that. In my case it was two ModuleGimbal modules on a multimode engine and I first had to figure out that the first entry was for the CenterOnly mode and the second entry was the AllEngines mode. Hope this helps

mg-badman commented 3 years ago
You can access those by array index (iirc, I figured it out once), but the main aspect of this issue is valid in that multiple similar items are not handled well. Perhaps multiple similar items should be wrapped each in their own array. Which they may be, but the interface is lumping them together in an awkward way
I tried that, but couldn't figure out how to do it, could you please provide an example?
The list returned from :GETMODULE is a list of strings, rather than a list of module entries

I could have been more clear, mea culpa. I know you can do a SHIP:MODULESNAMED(...) to get the modules of a given name in the craft into a list, then iterate through looking at module:PART to see if it is the part you want, then accumulate them into an array for that part and use that. Grindy to be sure. Let me poke around at the prompt to remember how I iterated more directly. I remember I was dealing with an engine with two modes and a separate thrust limiter and separate gimbal control for each. I'm pretty sure I found a more direct way to iterate over those. I may be misremembering, but I'll get back to you either way.

Ok, the best way is to use the MODULESNAMED suffix on the part itself (not from ship level). This will return an array of same-named modules for that part. I haven't found the order of of the modules to ever vary, but since there is no way to uniquely identify the module other than its rank order in this array you'd need to sort out which is which and stick to that. In my case it was two ModuleGimbal modules on a multimode engine and I first had to figure out that the first entry was for the CenterOnly mode and the second entry was the AllEngines mode. Hope this helps

Hey, thanks for the reply! I think we are talking about different things though - in my case i've managed to obtain the correct module (named "ModuleEnginesFX") from the engine part, however, then it's a matter of obtaining the correct "throttle" entry/property belonging to that module, so that i can independently control that engine's throttle using KOS.

in short - i haven't encountered repeating module names within a ship part yet (though that may be a problem too on other parts),

but i have encountered the "throttle" repeating property name within the "ModuleEnginesFX" module.

given that there are two "throttle" properties on the same module, i can only read/modify the first one which is a boolean/toggle button on the panel in the screenshot. but to control the engine throttle, i need to be able to affect the second one, which is a slider/float value. currently i haven't found a way to do that in KOS

also, about the ordering - It's in reference to the properties within a module, rather than modules within a part. The properties within a module seem to always appear in the same order as they show up on the part's info panel (as in the screenshot), meaning that if a user had a way to obtain a property from a module using the property's index, the user can write a loop to iterate over the property names and they would only need to make sure to compare the property name and obtain the index for the second occurance of "throttle" in order, and that would guarantee to always be the slider in the screenshot.

Thanks

mgalyean commented 3 years ago

Can you give an example part that has a "ModuleEnginesFX" that has these duplicate field/action/event names? I think I may have seen a module with a field and an event or an action with the same name, but not both being fields, or events, or actions. I'm not sure I've even seen that

JonnyOThan commented 3 years ago

Can you give an example part that has a "ModuleEnginesFX" that has these duplicate field/action/event names? I think I may have seen a module with a field and an event or an action with the same name, but not both being fields, or events, or actions. I'm not sure I've even seen that

It's in the stock module, but only under advanced tweakables. Note here how the guiName for the two fields (which is what shows up in the UI and how KOS identifies the field) is the same.

    [KSPField(advancedTweakable = true, isPersistant = true, guiActive = true, guiActiveEditor = true, guiName = "#autoLOC_900770")]
    [UI_Toggle(disabledText = "#autoLOC_6013010", scene = UI_Scene.All, enabledText = "#autoLOC_6005051", affectSymCounterparts = UI_Scene.All)]
    public bool independentThrottle;

    [KSPAxisField(incrementalSpeed = 20f, isPersistant = true, maxValue = 100f, minValue = 0f, guiFormat = "F1", axisMode = KSPAxisMode.Incremental, guiActiveEditor = true, guiActive = true, guiName = "#autoLOC_900770")]
    [UI_FloatRange(stepIncrement = 1f, maxValue = 100f, minValue = 0f, affectSymCounterparts = UI_Scene.All)]
    public float independentThrottlePercentage;
mgalyean commented 3 years ago

This is really nagging me. I could nearly swear I dealt with a very similar situation and figured a way to either iterate over those or found that the type of the argument to the SETFIELD would differentiate the calls or something. But I may be confusing what worked and what I hoped would work (but never circled back to the issue)

mgalyean commented 3 years ago

Ok, I was definitely misremembering. Along with Dun's approach of maybe using the hidden name, I'd throw in here the possibility of making, say "throttle", unique by appending the typename of the field in parens so the fields would become "throttle (bool)" and "throttle (single)" and kOS would map "throttle" to the first in rank order to keep old code working. Anyway, the point being, that having more than one string mapping to a field is ok. kOS, for the purposes of access a field, could even have "throttle", "throttle (MAINTHROT)", and "throttle (bool)" pointing to the same thing where MAINTHROT would be the presumed hidden name for that field. bleah

Alternatively, as OP put forward, something that allows one to do a SET/GETFIELD by index could be useful. From 10k feet, when one piece of a system (the system being unity, ksp, kos, os, mods, etc) tries to account for all possible future variation in the other pieces it ends up in a straightjacket. I think the best approach is two layered: something along the lines of what kOS does in just reporting the "stable" name of the field, even if it causes the problems discussed here, along side a way for ppl willing to risk the other pieces changing over time to access the functionality in more raw, perhaps more risky, way. Accessing fields by hidden name, or name+type, or raw rank order as delivered from other pieces is risky, but that risk is something for the scripter to evaluate and an overwhelming task for a single piece of the system to evaluate. No person can see the future, and code is even more blind. I think kOS could provide a "stable" pallet while also allowing workarounds, with some risk

Gotbread commented 3 months ago

Is there any current workaround for this? I ran into this issue, trying to set the independent throttle value via kOS, i can enumerate the modules needed, but i dont see a way to access the field throttle (single).

JonnyOThan commented 3 months ago

More generally, this is a consequence of kos using the display names of fields rather than the internal names. Do scripts that access fields and events even work across localizations?

That would be a giant pain to change, but it would also indirectly solve this issue.

SofieBrink commented 3 months ago

Do scripts that access fields and events even work across localizations?

Nope! annoying right!

nuggreat commented 3 months ago

Do scripts that access fields and events even work across localizations?

Nope! annoying right!

You can use fields and events across different localizations if you really need to it just requires you to not hard code the string of the field/event and instead get the list of all fields/events and from the list get the string you want by index. I would not recommend someone doing this as it is very fragile but it is possible.