rojo-rbx / rbx-dom

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

Temporary workaround for MeshPart.MeshId writes #390

Closed MaximumADHD closed 7 months ago

MaximumADHD commented 7 months ago

Not an ideal solution, but it works and takes a similar hacky measure to patching the Source of a Script via the ScriptEditorService. This can be killed off from rojo once the game:GetObjects hack is implemented.

kennethloeffler commented 7 months ago

Hi Max,

So this PR may seem like it works, but there is some hidden complexity around MeshPart.CollisionFidelity that it does not address.

MeshPart.CollisionFidelity is not serialized to file. Collision fidelity information is instead stored in a property called MeshPart.PhysicalConfigData, which is not reflected to Lua. This property contains a blob describing the collision geometry of the mesh. I think it's likely that you already know something about it, but there's some more information in this devforum post if you're interested: https://devforum.roblox.com/t/some-info-on-sharedstrings-for-custom-collision-data-meshparts-unions-etc/294588. This format is unstable, and will incur yet more maintenance burden, so rbx-dom will probably not accept PRs to parse it.

This being said, the implementation here will leave collision fidelity properties at the default if they're not specified in the project file, and may fail to reproduce the correct collision fidelity depending on the order in which Rojo's reconciler sets the mesh's properties. Quenty also added this functionality to his own fork of Rojo a little while ago, and used the same APIs. But his version seems like it also requires the user to specify MeshPart.CollisionFidelity in their project and also add tags to the meshes: https://github.com/Quenty/rojo/commit/e29b35d689ae0b847dc7f8a77a1826b6847aedf8. If you require MeshPart serving from Rojo right now, you may be able to use Quenty's fork, but failing that, you'll have to wait for https://github.com/rojo-rbx/rojo/pull/786 in Rojo 7.5. Sorry! 🙁