RobustToolbox icon indicating copy to clipboard operation
RobustToolbox copied to clipboard

Box2, YAML Box2 (De)Serialising and Box2 VV Display disagree on the order of the bounds arguments

Open RemieRichards opened this issue 4 years ago • 2 comments

Box2's constructor takes L,B,R,T Box2's YamlObjectSerializer parses B,L,T,R Box2's VVPropEditorUIBox2 displays the bounds as L,T,R,B or L,B,R,T depending on if it's a UI Box2 or a world Box2

This method is a crime against humanity, as the author saw the difference between the YAML serializer they were writing and the Box2 constructor, but did not fix it. https://github.com/space-wizards/RobustToolbox/blob/1815e6dac979a4102a5cc1648857990997e43470/Robust.Shared/Serialization/YamlObjectSerializer.cs#L1269

Can we standardise on something? Please?

Easiest would be B,L,R,T as that's present in all the YAML files (and would require a separate content PR to fix) However, I don't think this makes any sense.

Personally T,R,B,L makes sense as it's clockwise from north/top.

I don't really care what we standardise on, so long as we do. (if VVPropEditorUIBox2 must swap its format based on UI or not Box2, then atleast LABEL which format it's currently in) (No the placeholder text does not count, as you'd have to remove the value to see which it is...)

RemieRichards avatar Nov 13 '20 19:11 RemieRichards

UIBox2[i] and Box2[i] should always be separate here since the coordinate systems, by design, work differently. (the UI system uses top left as origin with Y+ going down, world coordinates have Y+ going up). Whether that is a mistake is a different discussion.

LBRT and LTRB for Box2 and Box2i are the correct orders here IMO since they:

  1. both follow the convention of X THEN Y, per corner.
  2. both have the lowest coordinates as the first corner.

PJB3005 avatar Nov 28 '20 00:11 PJB3005

@PJB3005 so what you are suggesting is that someone should change the order of the values getting serialized and then proceed to change EVERYTIME a hitbox is defined in yml?

PaulRitter avatar Dec 27 '20 22:12 PaulRitter