Sprites support

[makar-al]

Simple imperative example can be found here. Declarative implementation, the way I see it, could look something like this:

const Player = () => {
  const manager = useSpriteManager();

  return <Sprite manager={manager} /* ... */ />
}

const App = () => {
  return (
    <Engine canvasId="babylonJS">
      <Scene>

        {/* camera, light, ets. */}

        <SpriteManager imgUrl="textures/player.png" /* ... */ >

          <Player />

        </SpriteManager>

      </Scene>
    </Engine>
  )
}

[brianzinn]

hi @makar-al - if this works for you, I can do a version release: https://brianzinn.github.io/react-babylonjs/examples/sprites/

[makar-al]

@brianzinn Thanks for implementing this feature. I have a question, why you chose to avoid hooks? I'm not against current implementation, but for me hooks just seem more natural for things like managers. In my opinion, hooks give more freedom in a way one can arrange components, for example:

// this component can be placed anywhere in the tree
const Enemy = () => {
  const manager = useSpriteManager("/path/to/enemy.png"); // may be cached by url or something like this
  return <sprite name="enemy" manager={manager} />
}

On the other hand, babylonjs is not that sensitive to order of components as html is, so maybe grouping players with players, enemies with enemies and so on is ok. I'm new to babylonjs, so I don't have an opinion yet.

[makar-al]

Btw, I've noticed that you added types for packed manager, but missed the actual component. Maybe you could add it too? It seems like it's not very different from simple sprite manager.

[brianzinn]

I think the SpritePackedManager should work as <spritePackedManager ..../> - did you try it and it didn't work?

I am not averse to hooks. Let's look at the one you created. useSpriteManager('path'). How does that work it would use an existing SpriteManager if one already existed with the same path? Then some caching strategy or lookup would need to be implemented (there is one for model loading that uses <Suspense .../>. I can see it being a bit unwieldly to need to nest <Sprite ../> inside a manager, but also the hook would need a counterpart for PackedManager. I could add that if the optional manager prop was passed to Sprite that it would use that field. I try to keep those kinds of hooks out, because it just increases the code base, whereas the declarative syntax like <SpriteManager ...> must be understood by the renderer. There is a way to extend the reconciler yourself, but I try to capture the main classes being created. I'm not really sure here honestly where to draw a line in the sand, because I haven't used Sprites before - outside of GUI - I'm usually more in full 3d.

[makar-al]

No, haven't tried it yet, I just assumed that since SpritePackedManager is not mentioned in generate-code.ts. It turned out that I forgot to read generatedCode.ts, sorry. You're right, it should work, I'll test it a bit later and give my feedback.

Well, if you say that hooks will complicate things, then I'm ok with current implementation. I assume that if there is ever a need to do something with SpriteManager, then one can always get an instance of it using ref and configure it imperative way, right?

Btw, thanks for your work, awesome library :)

[makar-al]

There is one last piece of sprite API left - SpriteMap. I can see that it could be tricky to implement it declaratively. If you are ok with adding it to this library, maybe I could open a separate issue for it, I have some ideas, but I'm not sure if they'll work.

[brianzinn]

hi @makar-al I wouldn't say in those terms that "it would complicate things", I'm just trying to work out a caching strategy and the need to consider how to dispose once unmounted - I'm not convinced it would be easier to use despite how inconvenient it is to need to nest the Sprite. Dispose is handled automatically by the renderer and glossed over in the demos. You are right about the ref for direct access.

I can write a demo for useSpriteManager() hook while leaving it out of the library itself and make manager prop optional. I'll get back to you on that. I can see that being a useful way to implement. Meanwhile if you have a proposal for SpriteMap - kindly post here and we can see how to progress that. Cheers

[makar-al]

@brianzinn Just tested changes in master branch and everything works great. Thanks.

Do you want to release new version now or wait for SpriteMap? I'm ok with either approach.

[brianzinn]

i released 3.1.27. we can add SpriteMap separately. i'll try to make some time for a working hook for the manager.

[makar-al]

Ok, thanks.

[brianzinn]

closing from inactivity, but also at least it's partially solved. open a new Issue if you are missing anything. I haven't had time to work on hooks and also the property for sprite manager won't be picked up by the renderer. If you need it then those can be addressed separately. cheers.