BRP registry JSON schema endpoint

[Leinnan]

Objective

Resolve #16745

Solution

Provide a way to map AppTypeRegistry types into a JSON Schema that can be used in other applications. I took code from https://github.com/kaosat-dev/Blenvy as a starting point, cleaned up and adapter more for bevy_remote needs. Based on feedback and needs it could be improved, I could add some filtering options, etc.

Testing

• I was comparing results with the ones from code in blenvy
• There is added unit test, could be added more
• I was testing it in my game with this code:

fn types_to_file(world: &mut World) {
    use bevy_remote::builtin_methods::export_registry_types;
    let Ok(Ok(types_schema)) = world.run_system_cached_with(export_registry_types, None) else {
        return;
    };
    let registry_save_path = std::path::Path::new("assets").join("registry.json");
    let writer =
        std::fs::File::create(registry_save_path).expect("should have created schema file");
    serde_json::to_writer_pretty(writer, &types_schema).expect("Failed to save types to file");
}

It can be run by adding it at startup

app.add_systems(Startup, types_to_file);

[NthTensor]

Do you think it will be possible to do the inverse as well eventually? Take a serialized map and load it into the registry? Maybe @MrGVSV knows.

[MrGVSV]

Nit: Personally, I think these should follow the naming conventions in bevy_reflect.

So for example, typeInfo would be kind, long_name would be type_path, properties would be fields, etc.

[Leinnan]

Responded here: #16882 (comment)

[MrGVSV]

I think this endpoint could use some documentation. At first glance, I'm not really sure what additionalProperties means.

[Leinnan]

This one is described here: https://json-schema.org/draft/2020-12/json-schema-core#section-10.3.2.3

But I am not sure about one thing- does it apply only when patternProperties are present, or is it apply also when only properties are.

[MrGVSV]

Nit: Could we store a required property on the field itself rather than duplicating field names here?

[Leinnan]

It is part of the standard: https://json-schema.org/draft/2020-12/json-schema-validation#section-6.5.3

[MrGVSV]

I do think Option tends to imply optionality, but this might not always be true, especially when used by APIs to determine if a value can be "skipped" or not.

Ideally, Bevy would expose a common Required attribute that could be applied to required fields (or vice-versa if we want the default to be that all fields are required) like #[reflect(@Required)].

[Leinnan]

What I would prefer to have right now is being sure that when we are not providing field etc. for value Rust is able to infer the default value, like when you specify the #[serde(default)] for a field. But I am not sure yet how to do it here.

[MrGVSV]

Question: why are some property names camelCase and others snake_case?

[Leinnan]

Responded here: #16882 (comment)

[MrGVSV]

Nit: If you wanted you could also use variant.as_unit_variant().expect("variant is unit variant""). Although, it could be argued that the use of unreachable! is a bit clearer.

[MrGVSV]

I could be wrong, but I think name is always just an identifier. Is this split actually necessary?

[Leinnan]

Yeah, it doesn't seems to make sense, could be a result of moving code from some of the previous versions?

[Leinnan]

Updated, now it's the other way around, I am adding base type path to the variant and combine them:

                        "typePath": format!("{}::{}", type_path, v.name()),
                        "shortPath": v.name(),

[MrGVSV]

It'd be great if we had some more tests for different kinds of types (e.g. enums, tuples, etc.).

Would also be helpful if one of the tests also tested the JSON output (i.e. compared the entire output to a string). This also helps to ensure that everything not only behaves correctly, but looks right too (and it's nice for for future devs to see what the result is actually supposed to look like).

[Leinnan]

Agree, I will try to add some more tests.

[Leinnan]

Do you think it will be possible to do the inverse as well eventually? Take a serialized map and load it into the registry? Maybe @MrGVSV knows.

I mean, maybe? I don't have too big knowledge about reflect in bevy but AFAIK there is a option to register type by providing TypeRegistration into the TypeRegistry (add_registration method). TypeRegistration is made up from TypeInfo, the same from which the JSON Schemas are constructed and the dyn TypeData. ATM you can create TypeRegistration by passing type like TypeRegistration::of::<Option<String>>();, but maybe in the future there could be a way to transform JSON Schema into TypeRegistration object that could be registered.

[MrGVSV]

Do you think it will be possible to do the inverse as well eventually? Take a serialized map and load it into the registry? Maybe @MrGVSV knows.

I actually like the idea of having this sort of endpoint be separate from bevy_reflect for now. It allows BRP work to be unblocked by bevy_reflect work and also allows it to be more flexible in what data it collects/cares about.

Creating the registry the other way around is going to be a bit more complex, involves working it into bevy_reflect, and probably requires a rethink of how we register/recognize types and their info. Some examples of changes that may be needed:

• Completely moving away from TypeId as the de facto type key (favoring TypePath instead)
• Change TypeInfo to no longer be &'static (or work out a way to properly intern it)
• Create a split between compile-time TypeInfo and runtime TypeInfo (probably as two separate types), specifically so we can store function pointers in TypeInfo

So yeah, because of the complexity involved, I think it's better to work out solutions in bevy_remote instead of waiting on bevy_reflect to catch up.

[NthTensor]

The reason I ask is because bsn is reflection-backed and (as far as I can tell) can't even be parsed when it uses a type that's not defined locally. Alice was wondering if this would let us get around that. From that point of view, doing stuff entirely on BRP won't be useful unless bsn is backed by BRP instead of reflection. And that isn't on the table.

[Leinnan]

@MrGVSV For naming I think we should just follow https://json-schema.org/understanding-json-schema/keywords when possible.

But we are providing here some values that are not definied in JSON Schema, this is mostly the ones with snake case, so I think for those we could try to follow Bevy naming scheme, but make them camel case so they are consistent with the rest of the fields.

[MrGVSV]

The reason I ask is because bsn is reflection-backed and (as far as I can tell) can't even be parsed when it uses a type that's not defined locally. Alice was wondering if this would let us get around that. From that point of view, doing stuff entirely on BRP won't be useful unless bsn is backed by BRP instead of reflection. And that isn't on the table.

I see. We can definitely work towards it, but I wasn't planning on trying to get these sort of major changes into 0.16 since reflection just had a massive overhaul in 0.15, and this would likely be another major change. My current goal for 0.16 is just cleanup work, general improvements/fixes, and maybe some cool mostly-additive features (maybe some relatively small breaking changes).

We could try to do it, but ideally most of the work would be completed and merged early into the cycle, and we're already a month in with no progress made yet 😅

[NthTensor]

Yeah no worries, just wondering about how much effort it would take. After some further thought, it's probably simpler just to write a second bsn parser that does use brp/schemas instead of reflection, as you suggested.

[Leinnan]

I did update branch quite a bit, most important changes:

• created JsonSchemaBevyType struct, so the structure is more defined and later on it could be easier to add documentation and make the type public.
• naming pass:
  • short_name -> shortPath
  • typeInfo -> kind
  • long_name -> typePath
• two boolean fields are only serialized when value is true: isComponent and isResource.

[Leinnan]

I've updated structs documentation, added filtering options to the query, removed isComponent and isResource fields and added new reflectTypes vec field that can contain info about reflect types that type is providing.

Example:
Rust definition:

        #[derive(
            bevy_reflect::Reflect, bevy_ecs::system::Resource, Default, Deserialize, Serialize,
        )]
        #[reflect(Resource, Default, Serialize, Deserialize)]
        struct Foo {
            a: f32,
        }

Type schema:

{
  "shortPath": "Foo",
  "typePath": "bevy_remote::builtin_methods::tests::Foo",
  "modulePath": "bevy_remote::builtin_methods::tests",
  "crateName": "bevy_remote",
  "reflectTypes": [
    "Resource",
    "Default",
    "Serialize",
    "Deserialize"
  ],
  "kind": "Struct",
  "type": "object",
  "additionalProperties": false,
  "properties": {
    "a": {
      "type": {
        "$ref": "#/$defs/f32"
      }
    }
  },
  "required": [
    "a"
  ]
}

[Leinnan]

@MrGVSV IMHO now besides the more tests it is quite done, let me know how you feel about it.

[NthTensor]

This looks great!