Skip to content

Commit

Permalink
Implement property migration (#283)
Browse files Browse the repository at this point in the history
Based on PR #268

Co-authored-by: Shae A <[email protected]>
  • Loading branch information
kennethloeffler and Corecii committed Jun 15, 2023
1 parent f130d64 commit e99f37d
Show file tree
Hide file tree
Showing 33 changed files with 4,178 additions and 174 deletions.
38 changes: 35 additions & 3 deletions docs/patching-database.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
# How to Fix a New Property Added by Roblox
When Roblox introduces new properties, usually tools like Rojo can use them without any additional changes. Sometimes however, properties are added that have a different serialized name or that need to be modified with a special API in Lua.
When Roblox introduces new properties, usually tools like Rojo can use them without any additional changes. Sometimes though, properties are added with different names, multiple serialized forms, need to be migrated to a new property, or aren't listed at all in the reflection dump that Roblox gives us.

This document describes some common scenarios and the necessary steps to fix them.

Expand Down Expand Up @@ -63,7 +63,38 @@ Sometimes a property is added that cannot be assigned directly from Lua. For exa
model.Scale = 2
```

To fix this, first patch the property's `Scriptablity` to `Custom`:
## Roblox added a new property, but it's a migration from an existing property, and the existing property no longer loads
Sometimes Roblox migrates an existing property whose type is too constrained to a new property with a more flexible type.

This can cause problems when binary files containing the old property and binary files containing the new property are placed together in the same DOM, then serialized with `rbx_binary`. In the Roblox binary format, all instances of a class must define the same properties, so for instances from old files (where the new property is missing), `rbx_binary` simply writes the new property with a default value to uphold the invariant. This can result in weird behavior like old text UI all having the Arial font, because the default value of a new property took priority.

To fix this, we need to write a migration (in Rust) and apply it is as a patch (using database patch files), so that the old property is translated to the new property on deserialization.

Note that migration does *not* change the old files by itself - the process occurs only during deserialization, is purely in-memory, and will not overwrite old files with new versions.

First, add your migration to the `MigrationOperation` enum in [`rbx_reflection/src/migration.rs`][migrations]. The migration should be named after the properties it's migrating. For example, migrating from `Font` to `FontFace` would be named `FontToFontFace`.

Next, add code to convert from the old property's type to the new property's type. This code should be a new match arm in the `PropertyMigration::perform` method in [`rbx_reflection/src/migration.rs`][migrations].

Finally, add a patch in the [patches](patches) folder. This patch should change the old property's serialization type to `Migrate`, specifying the new property name and the migration name.

For example, the patch for fonts looks like:
```yaml
Change:
TextLabel:
Font: # Property we're migrating *from*
Serialization:
Type: Migrate
To: FontFace # Name of the property we're migrating to
Migration: FontToFontFace # Name of the migration operation that should convert the old property value to the new one
```

If this property is present on multiple classes, you may need to specify the Serialization change for multiple properties on multiple classes. For example, the `Font` property is present on `TextLabel`, `TextButton`, `TextBox` without being derived from a superclass, so the real patch is approximately 3 times as long since it needs to be applied to each class.

## Roblox added a new property, but modifying it from Lua requires a special API
Sometimes a property is added that cannot be assigned directly from Lua.

First, modify the reflection database to either add or change the property's `Scriptability` to `Custom`:

```yaml
# To change the property:
Expand Down Expand Up @@ -104,4 +135,5 @@ These pull requests outline how we implemented support for Attributes in rbx-dom

[rbx-dom]: https://github.com/rojo-rbx/rbx-dom
[patches]: https://github.com/rojo-rbx/rbx-dom/tree/master/patches
[custom-properties]: https://github.com/rojo-rbx/rbx-dom/blob/master/rbx_dom_lua/src/customProperties.lua
[custom-properties]: https://github.com/rojo-rbx/rbx-dom/blob/master/rbx_dom_lua/src/customProperties.lua
[migrations]: https://github.com/rojo-rbx/rbx-dom/blob/master/rbx_reflection/src/migration.rs
7 changes: 7 additions & 0 deletions patches/screengui.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
Change:
ScreenGui:
IgnoreGuiInset:
Serialization:
Type: Migrate
To: ScreenInsets
Migration: IgnoreGuiInsetToScreenInsets
19 changes: 19 additions & 0 deletions patches/text-gui.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
Change:
TextLabel:
Font:
Serialization:
Type: Migrate
To: FontFace
Migration: FontToFontFace
TextButton:
Font:
Serialization:
Type: Migrate
To: FontFace
Migration: FontToFontFace
TextBox:
Font:
Serialization:
Type: Migrate
To: FontFace
Migration: FontToFontFace
5 changes: 4 additions & 1 deletion rbx_binary/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,10 @@

## Unreleased
* Added support for `UniqueId` values. ([#271])
* [#256]: https://github.com/rojo-rbx/rbx-dom/pull/271
* Added migrations for properties like `ScreenGui.IgnoreGuiInset` and `TextLabel.Font` to their new counterparts (`ScreenGui.GuiInsets` and `TextLabel.FontFace`, respectively). ([#283])

[#271]: https://github.com/rojo-rbx/rbx-dom/pull/271
[#283]: https://github.com/rojo-rbx/rbx-dom/pull/283

## 0.7.0 (2023-04-22)
* Added support for `Font` values. ([#248])
Expand Down
4 changes: 3 additions & 1 deletion rbx_binary/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -483,7 +483,9 @@ fn find_serialized_from_canonical<'a>(
match serialization {
// This property serializes as-is. This is the happiest path: both the
// canonical and serialized descriptors are the same!
PropertySerialization::Serializes => Some(canonical),
PropertySerialization::Serializes | PropertySerialization::Migrate { .. } => {
Some(canonical)
}

// This property serializes under an alias. That property should have a
// corresponding property descriptor within the same class descriptor.
Expand Down
Loading

0 comments on commit e99f37d

Please sign in to comment.