rbx-dom
rbx-dom copied to clipboard
rojo-rbx
Improve definition of OptionalCoordinateFrame in binary spec
- Make "Optional" a generic type, rather than the specific "OptionalCoordinateFrame".
- Combine the Type ID and Value fields of the PROP chunk into a single "Values" field.
- A Values structure begins with an ID that identifies the type, which determines the shape of the remaining structure (usually an array of elements).
-
The Values field of a PROP chunk is optional. That is, if the chunk ends before the Values field can be read, this can indicate that 1) no values are assigned, or 2) the type is Optional, where each property has no value.
- The correct variation can be determined by constructing an empty PROP chunk for a non-Optional property type. If it succeeds for all non-Optional property types, then the first variation is correct. If it only succeeds for Optional types, then the second variation is correct.
- A Values with an ID of
0x1eindicates the type "Optional". The structure of Optional is two Values. The first has some ID (such as CFrame), and the second must(?) be the type Bool (0x02). Each element in the second array indicates whether the corresponding value in the first array has a value.
A Values with an ID of 0x1e indicates the type "Optional".
That seems like a pretty big assumption based on one property. I don't have any qualms with it but I would be a rather significant departure from the rest of the format in terms of design, so I'd want to be certain it's the truth. How did you come to this conclusion?
EDIT: To be clear, nothing against this idea and it would explain a few things, but it seems to assume a lot and if we reflect this in rbx_binary's implementation, it would be far reaching, so I want to know more.
The evidence for a Values field is the structure of the type. The presence of the first additional type ID suggests the reuse of a code path that parses value arrays. The presence of the second additional ID strongly suggests this, given that it is otherwise entirely unnecessary information (assuming that the second sub-array is always a Bool, which seems reasonable). At the very least, the specification should be updated to reflect this.
It is also reasonable to believe that type 0x1e is a generic type, not only because of the additional type IDs that are included in the structure, but because of nature of the type itself; an option type always involves some other type. Even the XML version suggests it by wrapping content of the value in a distinct inner tag that indicates the type. If OptionalCoordinateFrame was really a stand-alone type, then this tag would be redundant.
The fixation on "OptionalCoordinateFrame" is misguided. This term was derived from the XML tag of the same name. It should be noted that the the name of such tags are completely unused by Roblox's implementation of the XML format; how a value is parsed is determined by the name of the property and its outer class. The tag name should, ideally, be interpreted only as a human-readable representation of the type. I would wager that "OptionalCoordinateFrame" is a concatenation of "Optional" and the tag name associated with the type, since it matches the old-style "CoordinateFrame" tag name used for the CFrame type.
Also, just to clarify, this issue is in regards to the specification, not the implementation by rbx-dom. While there continues to be only one known option type, implementations can get away with structuring it as though it were a single distinct type. There's no difference as long as an implementation produces the specified result.
Regarding the fourth bullet point, it seems that this behavior has been removed. Here's a file that contains a "WorldPivotInternal" property chunk (of Workspace) as evidence that this format existed:
Resaving the file reveals that the property has been replaced by a "WorldPivotData" property, which has a more expected format. It is likely that this behavior was considered a mistake, and has been corrected. It still stands that this format exists in actual files produced by Roblox, and so needs to be handled accordingly. Roblox seems to do this by silently ignoring such chunks.
The fixation on "OptionalCoordinateFrame" is misguided. This term was derived from the XML tag of the same name. It should be noted that the the name of such tags are completely unused by Roblox's implementation of the XML format; how a value is parsed is determined by the name of the property and its outer class. The tag name should, ideally, be interpreted only as a human-readable representation of the type. I would wager that "OptionalCoordinateFrame" is a concatenation of "Optional" and the tag name associated with the type, since it matches the old-style "CoordinateFrame" tag name used for the CFrame type.
Does this apply to all tags? Because if so, I found an inconsistency. Basically, BinaryString tags can be swapped to SharedString format and will be read just fine (even though the value will point to a SharedString md5 key rather than the value directly this way). Or, is this more of an exception for SharedStrings only then?
Roblox allows certain property types to be used interchangeably for compatibility reasons. You can observe this with SharedString being allowed for String properties as well as int for int64 ones. This is the reason for some of our compatibility layers for rbx-dom.
This is mostly an implementation detail and I wouldn't rely upon it though.