-
Notifications
You must be signed in to change notification settings - Fork 112
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
Turn DMF properties into DMFProperty types #1757
Conversation
This is now finished, in that I'm not intending to add anymore to it. However, our positioning code is kinda fucky (see: the buttons in the winget test window), and that probably ought to be fixed in a separate PR because this is already huge. Also I'm not doing the .x/.y components of size/pos DMF props, because it'd be another rewrite of winget processing and ceebs. Requires space-wizards/RobustToolbox#5120 to work. |
All required prs are merged, this is now good to go after review |
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.
InspectCode found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.
break; | ||
case MenuDescriptor menuDescriptor: | ||
InterfaceMenu menu = new(menuDescriptor); | ||
|
||
Menus.Add(menu.Id, menu); | ||
Menus.Add(menu.Id.Value, menu); |
Check warning
Code scanning / InspectCode
Possible null reference argument for a parameter. Warning
break; | ||
case WindowDescriptor windowDescriptor: | ||
ControlWindow window = new ControlWindow(windowDescriptor); | ||
|
||
Windows.Add(windowDescriptor.Id, window); | ||
Windows.Add(windowDescriptor.Id.Value, window); |
Check warning
Code scanning / InspectCode
Possible null reference argument for a parameter. Warning
@@ -821,17 +844,17 @@ | |||
case MacroSetDescriptor macroSetDescriptor: | |||
InterfaceMacroSet macroSet = new(macroSetDescriptor, _entitySystemManager, _inputManager, _uiManager); | |||
|
|||
MacroSets[macroSet.Id] = macroSet; | |||
MacroSets[macroSet.Id.Value] = macroSet; |
Check warning
Code scanning / InspectCode
Possible null reference argument for a parameter. Warning
|
||
[DataField("id")] | ||
protected string _id; | ||
protected DMFPropertyString _id; |
Check warning
Code scanning / InspectCode
Inconsistent Naming Warning
|
||
[DataField("name")] | ||
protected string? _name; | ||
protected DMFPropertyString _name; |
Check warning
Code scanning / InspectCode
Inconsistent Naming Warning
} | ||
} | ||
|
||
[Virtual, ImplicitDataDefinitionForInheritors] | ||
public partial class ElementDescriptor { | ||
[DataField("type")] | ||
public string _type; | ||
public DMFPropertyString _type; |
Check warning
Code scanning / InspectCode
Inconsistent Naming Warning
TG contains this in its DMF: The number property doesn't recognize this and throws an exception, leading to the interface never loading. |
This is actually an error in TG's DMF - it should really be EDIT: done |
winget
stuff is more complicated than OD can handle right now, and it needs some type information needs to be passed along to process it properly. Notably pos and size vars can be accessed by their individual components, or formatted as json like{"x":123, "y":321}
. Also pretty much all of theas blah
features have different behaviour depending on the type of the var.Requires: #1789
This PR implements:
as escaped/json/etc.
winget
commands (fixes Goon: Say messages which contain spaces don't send #1730)TryGetProperty()
and default values for descriptorsI'm not doing this:
thing.pos.x
winget
accessbecause nobody uses it and it'd be a huge pain. I guess we'll do it later if it's needed.