Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Ability to Create and Insert Components #26

Merged
merged 6 commits into from
Oct 5, 2022

Conversation

zicklag
Copy link
Contributor

@zicklag zicklag commented Oct 4, 2022

Now we can create an insert components!

I kind of hacked the example real quick to demonstrate. Not sure if we want to make that nicer or we just leave it for now.

Right now the only way to create components is with Value.default(MyComponent), and then modifying the returned value ref.

I tried to make a version called Value.create(MyComponent, { my: 'components', example: [1, 2, 3] }), but it turns out that Bevy's ReflectDeserializer seems quite verbose as far as I can tell:

{
        "type": "bevy_transform::components::transform::Transform",
        "struct": {
          "translation": {
            "type": "glam::f32::vec3::Vec3",
            "struct": {
              "x": {
                "type": "f32",
                "value": 0.0,
              },
              "y": {
                "type": "f32",
                "value": 0.0,
              },
              "z": {
                "type": "f32",
                "value": 0.0,
              },
            },
          },
          "rotation": {
            "type": "glam::f32::sse2::quat::Quat",
            "value": [0.0, 0.0, 0.0, 1.0],
          },
          "scale": {
            "type": "glam::f32::vec3::Vec3",
            "struct": {
              "x": {
                "type": "f32",
                "value": 1.0,
              },
              "y": {
                "type": "f32",
                "value": 1.0,
              },
              "z": {
                "type": "f32",
                "value": 1.0,
              },
            },
          },
        },
      },

It seems that the Bevy scene loader somehow gets away without as much verbosity, but it's also RON, so I'm not sure if it's able to trim out unnecessary annotation of types.

Either way, I figure using a default implementation to create the component, and then patching the component manually ( especially if we implement a value.patch() method later ), then we're already in pretty good shape and it isn't worth the effort yet.

This opens up a lot more awesome use-cases for scripting!

@jakobhellermann
Copy link
Owner

but it turns out that Bevy's ReflectDeserializer seems quite verbose as far as I can tell:

Bevy 0.9 had a PR merged which significantly reduced the serialized output:
https://github.com/bevyengine/bevy/blob/main/assets/scenes/load_scene_example.scn.ron
https://github.com/bevyengine/bevy/blob/latest/assets/scenes/load_scene_example.scn.ron

@jakobhellermann
Copy link
Owner

I have also been thinking about how creating values should work.
The first question we should answer is whether we want to make

world.insertResource(Vec3, { x: 0, y: 0, z: 0 });

work (assuming Vec3 were a resource).

There is nothing preventing us from doing so, because we know the type when inserting we should be able to construct the actual value from the serde_json::Value somehow.

There are however, some operations where we need to know the type before handing the value to the world, like calling methods. This is where something like your Value.create could come in, like

let value = as(Vec3, { x: 0, y: 1, z: 2 });
value.normalize();

This should work recursively, e.g. for creating a Transform just from JS.

Alternatively a good stopgap solution is just being able to construct a default value, which you can use like

let translation = default(Vec3); // or Vec3.default()?
translation.x = 5;
let transform = default(Transform);
transform.translation = translation;

Typing this, I realized that we can just make Vec.new() work by making Vec.method(a, b, c) equivalent to a.method(b, c), just like it is in rust.

@jakobhellermann
Copy link
Owner

That said I think this PR is a good start and I will merge it to have a good base and improve on that later.

src/runtime/ops/ecs/ecs.js Outdated Show resolved Hide resolved
types/lib.bevy.d.ts Outdated Show resolved Hide resolved
types/lib.bevy.d.ts Outdated Show resolved Hide resolved
@zicklag
Copy link
Contributor Author

zicklag commented Oct 4, 2022

The first question we should answer is whether we want to make ... work

To me I think I'd rather have to create values before inserting them as resources. I like being explicit about the creation step instead of adding an overload to the world.insert().

Supporting Vec3.new() feels like a nice option, but I do think we'll probably want either default(Vec3) or something like it for types that don't necessarily have constructors in Rust, and so that we support types without any reflect functions bound yet.

Alternatively a good stopgap solution is just being able to construct a default value, which you can use like

Yeah, that should be possible now with this PR, and doesn't feel too bad. I think using Reflect.apply(), though we might be able to make it even more ergonomic by allowing you to patch the values with a JSON structure, which could also act as a very convenient constructor for many types, similar to Rusts ..default() setup that's very common in Bevy.

@zicklag
Copy link
Contributor Author

zicklag commented Oct 4, 2022

And I pushed an update fixing your review points. 👍

@zicklag
Copy link
Contributor Author

zicklag commented Oct 4, 2022

Just pushed an update that renames Value.default to Value.create and allows you to patch the fields if desired:

    let vel = Value.create(Velocity, [
        {
          x: -200,
          y: 200,
        },
      ]);

And you can still just leave out the second argument to create a default value:

let ball = Value.create(Ball);

I think that works pretty well.

@zicklag
Copy link
Contributor Author

zicklag commented Oct 4, 2022

Pushed another update that adds Value.patch(), which lets you patch pre-existing values.

    Value.patch(vel, [
        {
          x: 0,
          y: 200,
        },
      ]);

BTW, I liked the idea of a free function called default() but unfortunately default is a keyword so that doesn't work. 🤷

@jakobhellermann
Copy link
Owner

Looks good, thank you. I haven't decided yet how exactly I would ideally want to have the API for dealing with values look like, but when I find something I'll just open a PR or an issue for discussion.

@jakobhellermann jakobhellermann merged commit 4ac541e into jakobhellermann:main Oct 5, 2022
@zicklag zicklag deleted the create-and-insert branch October 5, 2022 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants