search

rojo-rbx / rbx-dom

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

Don't serialize `ScriptGuid`, `UniqueId`, and `HistoryId` #327

Closed Dekkonot closed 1 year ago

Dekkonot commented 1 year ago

Closes #314 and resolves #324 for now.

I've decided that LuaSourceContainer.ScriptGuid, Instance.UniqueId, and Instance.HistoryId shouldn't serialize for the time being. All they do is make diff noise and they aren't especially useful for Rojo or Lune users at this time.

In the process, I discovered that rbx_reflector actually wasn't catching UniqueId properties so I've corrected that. I'll add it to my list of things to check in new data type implementations going forward.

Dekkonot commented 1 year ago

I was following in the precedent of not noting patches, but this is unusual and I suppose we should note it down somewhere. Good call.

Packages are the main issue with UniqueId and HistoryId, since that's all they're used for as of this moment. This was confirmed a few versions ago by the engineer who implemented them. While it's possible that's changed, I don't think it has.

With ScriptGuid, as far as I'm aware the only use for it is to preserve the IDE state for scripts. When you close a place in Studio, it saves what scripts you had open and any breakpoints you had for them, and it uses the GUID of scripts to do that. This technically means that we lose what little compatibility we had with the debugger, which would suck, but it only impacts people who were running Studio generated files through rbx-dom and then reopening them in Studio for debugging. Mind you, this also means that those same people had to not be using a workflow that duplicated ScriptGuid, since that'd also break it.

I think those are acceptable compatibility concerns because they're not really something we support beyond it being a coincidence.