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

[@miniplex/react] Provide an out of the box mechanism for ref capture components #211

Closed
hmans opened this issue Oct 24, 2022 · 4 comments
Closed
Labels
enhancement New feature or request

Comments

@hmans
Copy link
Owner

hmans commented Oct 24, 2022

Instead of doing this:

type Entity = {
  ...,
  transform: THREE.Object3D
}

<ECS.Entity>
  <ECS.Component name="transform">
    <mesh />
  </ECS.Component>
</ECS.Entity>

Let's bake support for captured refs into @miniplex/react! This could look like the following, where the first child of <Entity> is captured into the ref component by default:

type Entity = {
  ...,
  ref: THREE.Object3D
}

<ECS.Entity>
  <mesh />

  <ECS.Component name="health" value={100} />
</ECS.Entity>

Now the ref of the THREE.Mesh is captured into the ref component of the world's entity type, which would need to be typed accordingly (ie. ref: THREE.Object3D.)

For cases where the component name needs to be something different than ref, we could make this configurable through createReactAPI, eg.:

/* Set the default ref capture component to sceneObject */
const ECS = createReactAPI(world, { refComponent: "sceneObject" })

Potential Problems

  • We can't force Entity to only have a single child, because that would block the user from setting additional components through JSX. This means that the remaining heuristic we can apply here is to look for the first child, which feels a little bit iffy/easy to break/etc.
@hmans hmans added the enhancement New feature or request label Oct 24, 2022
@hmans hmans added this to the Miniplex 2.0.0 milestone Oct 24, 2022
@firtoz
Copy link

firtoz commented Oct 24, 2022

How about

<ECS.Entity create={<mesh>...<mesh/>}>
    <ECS.Component name="health" value={100} />
</>

?

@hmans
Copy link
Owner Author

hmans commented Oct 25, 2022

How about

<ECS.Entity create={<mesh>...<mesh/>}>
    <ECS.Component name="health" value={100} />
</>

?

This would sort of go in line with another change I want to implement for 2.0, where you can set components directly as props on the <Entity> component. There'd be another issue, though: sometimes you may want to have a component contain actual JSX nodes (!= refs to the elements they eventually render), so I imagine this could make typing this accurately a little iffy. But I will keep it in mind.

@Ctrlmonster
Copy link

I'm not sure if I like default ref capturing of the first child. I'm currently doing this in my code and am guessing that it would capture the ModelPlaceholder ref - instead of the Model - in that case.

  return (
    <ECS.Entity entity={character}>

      <Suspense fallback={<ModelPlaceholder dimensions={character.dimensions}
                                            position={character.transformTarget.position.toArray()}/>}>
          <ECS.Component name={"sceneObject"}>
            <Model />
          </ECS.Component>
      </Suspense>


    </ECS.Entity>
  )

@hmans
Copy link
Owner Author

hmans commented Oct 27, 2022

Closing this for now until a better pattern emerges. Thanks for the feedback!

@hmans hmans closed this as not planned Won't fix, can't repro, duplicate, stale Oct 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants