-
Notifications
You must be signed in to change notification settings - Fork 230
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes #2518 Fix for empty part module field names. #2519
Conversation
@@ -196,7 +196,7 @@ protected virtual ListValue AllFields(string formatter) | |||
{ | |||
returnValue.Add(new StringValue(string.Format(formatter, | |||
IsEditable(field) ? "settable" : "get-only", | |||
field.guiName.ToLower(), | |||
field.guiName.Length > 0 ? field.guiName.ToLower() : field.name.ToLower(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to fix the problem only for the suffix :ALLFIELDS
. But the name it returns still wouldn't be recognized in all the other places where you can use these names, like :ALLFIELDNAMES
, or :HASFIELD
, or :GETFIELD
, or :SETFIELD
, etc.
For those, they're still only looking at the guiName
. So this would make the user think kOS will let them use the name when it won't.
To really fix the problem would require coming up with some kind of method that can be re-used in all the places the guiName
is being used now, to perform this same "use guiName unless it's blank then use not-gui name." technique all over the place.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I also wonder if the same logic needs to be applied not just to Field names but also to Event names (buttons on the menu) and Action group actions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, good point, I'll do a bit more testing on this. I've not seen it for actions and events, but that's not too say it couldn't happen.
I've extended the field name lookup changes to cover all uses of fields and also events and actions. |
The update looks good by eye. I plan to put in a little time to kOS tomorrow, and I'll have a look at it in more detail (as in, install it and test it out) then. |
I don't really have the time to install all of RO to test this right now. (The only mod I know of that has this problem of not using the guiName is realfuels), so I'm going to have to trust you that you tested this and it worked. |
Fixes #2518.
Adds a fall back to BaseField.name for part module field names in the event that BaseField.guiName is empty.