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

World.spawn(), Non-primitive Assignments, and Patching #28

Closed
wants to merge 10 commits into from

Conversation

zicklag
Copy link
Contributor

@zicklag zicklag commented Oct 6, 2022

This PR

  1. Adds a simple world.spawn() method.
  2. Allows you to assign non-primitives to value refs, allowing, in my use case, me to set the id of a Handle<T> in Bevy even though HandleId is not a JS primitive.
  3. Fixes a bug I introduced with the Value.patch() PR where non-primitive assignments stopped throwing an error when they failed.

It's worth noting that non-primitive patching doesn't work yet. That will require a kind of recursive value ref unwrapping helper, which I haven't implemented yet.

@zicklag
Copy link
Contributor Author

zicklag commented Oct 6, 2022

Just pushed an update that adds non-primitive patching.

This let me load ( weak ) asset handles in my game quite ergonomically:

type AnimatedSprite = {
  start: usize;
  end: usize;
  atlas: HandleTextureAtlas;
  flip_x: boolean;
  flip_y: boolean;
  repeat: boolean;
  fps: f32;
};
const AnimatedSprite: BevyType<AnimatedSprite> = {
  typeName: "jumpy::animation::AnimatedSprite",
};

export default {
  preUpdate() {
    for (const entity of MapElement.getSpawnedEntities()) {
      let animated_sprite = Value.create(AnimatedSprite, {
        start: 0,
        end: 4,
        repeat: true,
        fps: 6,
        atlas: {
          id: Assets.getHandleId(
            "../../../resources/default_decoration.atlas.yaml"
          ),
        },
      });

      world.insert(entity, animated_sprite);
    }
  },
};

@zicklag
Copy link
Contributor Author

zicklag commented Oct 6, 2022

If bevyengine/bevy#6187 is merged, I think we'll be able to spawn sprites from scripts, with the nuance that the script can only create weak handles, so the game needs to provide a way for scripts to somehow register assets to load with strong handles.

In my game I have YAML files with a preload_assets field for that, because I preload all the assets in my game anyway.

@zicklag zicklag mentioned this pull request Oct 6, 2022
@zicklag zicklag changed the title Non primitive assign World.spawn(), Non-primitive Assignments, and Patching Oct 6, 2022
@zicklag zicklag changed the title World.spawn(), Non-primitive Assignments, and Patching World.spawn(), Non-primitive Assignments, and Patching Oct 6, 2022
@jakobhellermann
Copy link
Owner

Have you seen bevyengine/bevy#5923? It would let you insert new assets and get back a handle.

@zicklag
Copy link
Contributor Author

zicklag commented Oct 6, 2022

No I haven't, that's awesome! 🎉

I think that pretty much fixes the bulk of the things I was worried about for asset handling in scripts.

@@ -22,9 +22,13 @@ declare interface BevyScript {

declare type RawValueRef = unknown;

type RecursivePartial<T> = {
[P in keyof T]?: RecursivePartial<T[P]>;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doesn't this need some kind of recursion base condition? RecursivePartial<{x: number, y: number, z: number}> would become {x: RecursivePartial<number>, y: RecursivePartial<number>, z: RecursivePartial<number> }, at which point you get keyof number which apparently is "toString" | "toFixed" | ..

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could do this:

type RecursivePartial<T> = {
  [P in keyof T]?: T extends object ? RecursivePartial<T[P]> : T[P];
};

But interestingly, when I hover over the type inference in VSCode, it seems like it's correct, even without the T extends object check.

For instance, without changing it, this doesn't work:

const a: RecursivePartial<number> = { toString: (test) => ""};

And when I hover over q in this example in VSCode, it shows the type of name to be string, not RecursivePartial<string>:

const test: RecursivePartial<{
  x: number;
  y: number;
  z: number;
  q: {
    name: string;
  };
}> = {
  x: 3,
  q: {

  }
};

image

And this won't work:

const test: RecursivePartial<{
  x: number;
  y: number;
  z: number;
  q: {
    name: string;
  };
}> = {
  x: 3,
  q: {
    toString: (_) => {""}
  }
};

So it seems like TypeScript is doing some kind of automatic transformation on the type or something, making it do what we'd expect.

@jakobhellermann
Copy link
Owner

Can you rebase this? Otherwise it looks good. Sorry for leaving these PRs uncommented for so long.

@zicklag
Copy link
Contributor Author

zicklag commented Oct 29, 2022

Yep, I'll rebase this today, and no problem about the delay. I was totally fine working off my fork.

@zicklag
Copy link
Contributor Author

zicklag commented Oct 29, 2022

Pushed the rebase.

@jakobhellermann
Copy link
Owner

Thanks a ton for the PR and the patience for having this unmerged for so long.
I rebased the PR on main and merged it in #36.

@zicklag
Copy link
Contributor Author

zicklag commented Nov 16, 2022

No problem! :D

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.

2 participants