DelSkayn / rquickjs

High level bindings to the quickjs javascript engine
MIT License
431 stars 58 forks source link

Native object with native objects as properties are unintuitively copied #314

Open thefakeplace opened 1 month ago

thefakeplace commented 1 month ago

Imagine the following scenario:

#[class()]
struct Vec3 {
  #[qjs(get, set)]
  x: f32,
  #[qjs(get, set)]
  y: f32,
  #[qjs(get, set)]
  z: f32,
}

#[class()]
struct Entity {
  #[qjs(get, set)]
  position: Vec3,
}

if you run the following JS:

const ent = new Entity();
print(ent.position.y); // output: "0";
ent.position.y += 1;
print(ent.position.y); // output: "0";

You can see that ent.position is never updated, because rquickjs clones it before passing the value to the runtime, like you can see in the generated code to create the Entity class.

/* ... */

        let proto = rquickjs::Object::new(ctx.clone())?;
        proto.prop(
          "translation",
          rquickjs::object::Accessor::new(
            |this: rquickjs::function::This<rquickjs::class::OwnedBorrow<'js, Self>>| {
              this.0.translation.clone() // <- here
            },
            |mut this: rquickjs::function::This<rquickjs::class::OwnedBorrowMut<'js, Self>>,
             v: Vec3| {
              this.0.translation = v;
            },
          ),
        )?;

/* ... */

I know this is probably by design, because any kind of pointer could be invalidated on the native side accidentally and then (presumably) some kind of segfault.

However, it's unintuitive and I'm looking for a solution, even if it means pinning memory on the native side.

Any ideas?

DelSkayn commented 1 month ago

This is indeed by design. The lifetime of objects on the JavaScript side can't be known in advance so an object must have ownership of it's rust value.

With ent.position you probably want the same object to be returned every time, one solution is to store the object you want to return. So:

struct Entity<'js>{
    #[qjs(get,set)]
    position: Class<'js, Vec3>,
}

You can also solve this some other ways but ultimately you will have to use either some sort of shared ownership, with Rc, Arc, or rquickjs::Class which is kinda like a Rc<RefCell<T>>.