rojo-rbx / rbx-dom

Roblox DOM and (de)serialization implementation in Rust
MIT License
105 stars 42 forks source link

Add support for `Enum`-typed attributes #383

Open kennethloeffler opened 5 months ago

kennethloeffler commented 5 months ago

Roblox added support for Enum attributes in 570. rbx-dom currently lacks support for them, but they're probably uncommon since they're not yet available in Roblox Studio's attribute type picker UI.

Dekkonot commented 5 months ago

It turns out that Enum attributes require us to store the name of the Enum that the EnumItem we're referring to is a part of. This makes sense, in hindsight, but it means that supporting these is going to be potentially complex since we have to start tagging enums with their names in some capacity.

My thoughts are that we modify the Enum type in rbx_types to also hold an Option<String> (or something akin to that) and then rbx_binary and rbx_xml (and other consumers) can fill that in if necessary. Then when serializing enums, we would raise an error if it didn't a name. It feels appropriately simple and doesn't require us to pull reflection info into anywhere new.

Dekkonot commented 2 months ago

Okay I have bad news: Serde's derive macro is not powerful enough to accomplish what I was hoping to above. This means we'll have to implement a potentially complex serializer and deserializer for Enums if we use the existing struct.

My main concern is that Rojo users rely upon the serde format for enums to be stable, so the easy solutions are out the window: if we represent Enum as a struct instead of a newtype, it breaks the serde format.

It's also possible I have massively overcomplicated things so I'm interested to hear others thoughts on it.