Bool deserialization errors for internally-tagged enum variants

[aramperes]

I have a patch with a unit test demonstrating this issue (the test internal_variant_bool fails at deserialization): aramperes@0056f45

thread 'internal_variant_bool' panicked at 'NBT deserialization.: Serde("invalid type: integer `1`, expected a boolean")', src/libcore/result.rs:999:5

Context: Internally-tagged enum representation is a serde feature to define the struct used based on a field inside of said struct. In the patch, I am serializing a deserializing the following "variants" (represented as JSON here):

// internal_variant_string
{
  "data": {
    "variant": "String",
    "data": "test"
  }
}

// internal_variant_bool
{
  "data": {
    "variant": "Bool",
    "data": true
  }
}

There seems to be an incompatibility between this serde feature and hematite_nbt's bool deserialization mechanism. The error message shows that it is happening at the serde-level, which makes me think the deserialize_bool in nbt::de is not executed in this particular test case.

An easy work-around is to use u8 instead of bool for the BoolVariant struct.

[atheriel]

I'm not sure I really understand what's going on here. Internally-tagged enums don't really make sense in the NBT format to me; can you give a more specific example of why you need to use them?

[aramperes]

Sure; Entity storage in chunk data is an example of this. We're using internally-tagged enums for this in feather: https://github.com/caelunshun/feather/blob/a790c57843b111f873fa825ab94e74a35f8bee05/core/src/save/entity.rs#L3-L14

For example, consider how entities are stored in a chunk in Minecraft (represented as JSON here):

{
  // Chunk fields (...)
  "Entities": [
    {
      "id": "minecraft:arrow",
      "Pos": [1.0, 2.0, 3.0],
      "Motion": [4.0, 5.0, 6.0],

      // Arrow-specific fields
      "crit": true,
      "player": false
      // ...
    },
    {
      "id": "minecraft:item",
      "Pos": [1.0, 2.0, 3.0],
      "Motion": [4.0, 5.0, 6.0],

      // Item-specific fields
      "Age": 10,
      "Item": {
          "id": "minecraft:stone",
          "Count": 2
      }
      // ...
    }
  ]
}

As you can see, if each type of entity data was represented as a struct (ArrowEntityData, ItemEntityData, ...), the type is determined by the "id" field present in all entities. This is essentially the variant of each compound (struct) in the list.

The only incompatibility with hematite_nbt I've found for internally-tagged enum is this bool conversion.

[atheriel]

Ah, that's interesting. I'm surprised that works, really, but it's a nice design pattern for sure. It may be possible to fix, but it is also possible that Serde simply does not have enough information in this case to know what the right thing to do is.

bool types are not the only types with special deserialization handling (see line); it may be worth investigating whether this problem crops up with e.g. unit structs.

[atheriel]

@caelunshun Have you come up with a reasonable workaround for this, or is it still a blocking issue? I have a theory that we could implement deserialization of enums directly as a possible way to add this feature.

[caelunshun]

@caelunshun Have you come up with a reasonable workaround for this, or is it still a blocking issue? I have a theory that we could implement deserialization of enums directly as a possible way to add this feature.

We've gotten by fine using u8 and then converting to bool manually, since booleans are represented as TAG_Byte in NBT. Fixing this would mostly be an ergonomic improvement.

Curious to hear about your idea for deserializing enums.