Workspace.CollisionGroups doesn't serialize anymore, and other collision group woes

[kennethloeffler]

This is a bit of a multilayered issue, so bear with me....

Changes to the collision group API

The potential for problems with collision groups seems to begin in September 2022, when Roblox made changes to the collision groups API. As part of this change, it looks like Workspace.CollisionGroups was superseded by Workspace.CollisionGroupData.

User reports of problems with collision groups

I can find reports of problems in the Rojo ecosystem involving Workspace.CollisionGroupData going back to January 2023. I'll provide an illuminating excerpt from a conversation in the Roblox OSS Discord server:

User1 — 01/13/2023 4:32 PM
    Since the new collision group API, is it still possible to set collision groups at build time?
    I'm having trouble writing out the property by hand
    Searching here the format is something like
    [User1 shows the format for `Workspace.CollisionGroups`]

    "Default^0^-1\\Bullet^1^-1"

User2 — 01/13/2023 4:39 PM
    you can change the collisions and save it as rbxlx* and check what it gives for collisiongroup

User1 — 01/13/2023 4:40 PM
    Save the workspace as a model file?

    Wont let me do that.. I did try saving the place as an xml and got something hashed

    [User1 shows a snippet from a .rbxlx file, showing the Base64 form of Workspace.CollisionGroupData]

    <BinaryString name="CollisionGroupData"><![CDATA[AQcABPP///8HRGVmYXVsdAEEi////wZCdWxsZXQCBIT///8HVHJpZ2dlcgMEgv///whXZWFyYWJsZQQEof///wVUZWFtQQUEkf///wVUZWFtQgYEwf///wVXb3JsZA==]]></BinaryString>

User2 — 01/13/2023 5:16 PM
    ok so
    [User2 again shows the Base64-encoded form of Workspace.CollisionGroupData, this time embedded in a Rojo project]

        "Workspace": {
          "$properties": {
            "CollisionGroupData": {
              "BinaryString": "AQcABPP///8HRGVmYXVsdAEEi////wZCdWxsZXQCBIT///8HVHJpZ2dlcgMEgv///whXZWFyYWJsZQQEof///wVUZWFtQQUEkf///wVUZWFtQgYEwf///wVXb3JsZA=="
            }
          }
        }

    works but idk why it doesn't give in a more readable format like Default^0^...

User1 wanted to use collision groups in their Rojo project, learned about the Workspace.CollisionGroups format, and also discovered Workspace.CollisionGroupData.

Just for more background, I attempted a special representation for collision groups for rbx-dom (in the same vein as Attributes) a while ago (before the collision group API changes) in #206. Some of the tests show the format of Workspace.CollisionGroups. The format is different than Workspace.CollisionGroupData!

Introduction of rbx_reflector, and continued reports

Fast forward to now: we started using rbx_reflector. We had another user report a problem while using Workspace.CollisionGroups (with the correct format and everything), in their Rojo project, Rojo building, then using Lune 0.7.2 (which uses a rbx_reflector database) to do some additional processing and serialize the place back out. Workspace.CollisionGroups did not serialize, when it did before. What gives?

rbx_reflector uses Roblox Studio's -FullAPI option to obtain a reflection metadata dump, whereas the older generate_reflection used -API. I bet that Workspace.CollisionGroupData was not present in the normal dump, but is present in the full dump. Using this hypothesis, it looks like #276 was the first time we committed rbx_reflector results to the repo. In this PR, we can see from the changes in rbx_dom_lua/src/database.json that Workspace.CollisionGroupData was added, and that Workspace.CollisionGroups no longer serializes, which ultimately explains the problem.

Conclusions

The only question left is what rbx-dom is supposed to do about all this. I think an easy fix is to patch Workspace.CollisionGroups so that it serializes again (I'm certain Roblox still supports reading this property, and maybe they will indefinitely). If we do this, I don't think there will be problems unless a user does something really weird like insert CollisionGroups and CollisionGroupData properties into the same Workspace, but I'm open to being proven wrong!

Beyond that, I'm not sure; a special CollisionGroups property + representation for Workspace.CollisionGroupData (like Tags or Attributes) is not out of the question, but there are compat concerns for users who already use CollisionGroups with the old format, and it's out of scope for this issue anyway. We could also think about migrating Workspace.CollisionGroups to Workspace.CollisionGroupData.

As for for what lessons can be learned from this? We should probably scrutinize changes to the database more closely, and take special note of when we start using new tech.

[Dekkonot]

So, I've done some digging and have discovered that this is not actually rbx_reflector's fault, at least not directly. In PR #249, all patches that were added to the database were either deleted or changed to aliases, depending upon the patch. As a general rule, additions that had clear aliases were changed to just be aliases and pure additions (like PartOperation.AssetId) were removed. The patch file for Workspace.CollisionGroups fell into the latter category, so it was removed.

I've verified the other patches all applied to the current (rbx_reflector generated) database while writing this, since I was concerned. It turns out Workspace.CollisionGroups is a unique patch that we didn't account for. It was the only one that added a property addition where our declared serialization differs from the actual serialization Roblox does.

This patch file has been present for at least 3 years, so we've ended up insulated from the switch to CollisionGroupData in Rojo. Now that the patch is gone though, we're seeing the actual serialization of CollisionGroups, which could have been anything this whole time.

The solution to this should be to add a patch that changes serialization, because that restores the old behavior, which is probably fine; I don't imagine that anyone will be loading files with two Workspaces anytime soon, so a migration isn't necessary for now.

For posterity, here is the old patch for CollisionGroups:

rbx-dom/patches/workspace.yml

Lines 1 to 9 in 82e636e

	Add:
	Workspace:
	# Collision groups in a place are serialized as a string that looks like:
	# Default^0^1
	CollisionGroups:
	Serialization:
	Type: Serializes
	DataType:
	Value: String

---

As an addendum, I know that I should have caught this because the serialization was changed with my PR. However, going forward I will be more careful with database updates and hopefully everyone else will be too.

[kennethloeffler]

It was the only one that added a property addition where our declared serialization differs from the actual serialization Roblox does [...] we're seeing the actual serialization of CollisionGroups, which could have been anything this whole time.

To be clear, the format of Workspace.CollisionGroups was known, and it was a Roblox serialized property. There is evidence of it in rbx-test-files. After the collision group API changes, Workspace.CollisionGroups is superseded by Workspace.CollisionGroupData, which has a different format.

Anyway, the solution seems straightforward: we should patch Workspace.CollisionGroups so that it serializes again. After that, we can think about adding in a first-class representation for collision groups (I'll open a different issue about it)

[Dekkonot]

To be clear, the format of Workspace.CollisionGroups was known, and it was a Roblox serialized property. There is evidence of it in rbx-test-files. After the collision group API changes, Workspace.CollisionGroups is superseded by Workspace.CollisionGroupData, which has a different format.

Yes, I'm aware. But what I'm saying is it changing was invisible to us as a fluke. It could have (and did) happened at any point and we wouldn't have noticed until now since it was being corrected anyway. It is a consequence of swapping to rbx_reflector but only because we deleted a patch that was accidentally correcting it. 😄

I agree though. We should prioritize giving first class support to collision groups but that's out of scope for fixing this problem. A quick fix is to repatch CollisionGroups to serialize.