Closed mattrossman closed 3 years ago
I'm not sure the strategy for retrieving the material from the worker. The current architecture seems to expect that I can retrieve the Material from useMaterial
synchronously, because it needs the Material to exist before running the useBody hook. However the worker makes this an asynchronous process (I ask the worker to make a material, then wait with a listener for its result) which means that when I run useBody, the material is not yet defined. How do you suggest solving this? Refactoring useBody?
I'm also unsure whether I should be extending the existing messaging system in Provider.tsx
to get values from the worker, or if it's ok to just set up my own listener in the hook.
Yeah you'd basically have to refactor useBody. This is the issue with the current useCannon api unfortunately. I'm not sure if a more elegant solution exists, but it would be awesome if we could treat the worker messaging system as a way to communicate more directly with cannon-es (i.e. no intermediate switch statement). I've had the idea in the back of my mind for months but am super busy with work so haven't had any time to experiment.
@marcofugaro @drcmda Do you have any suggestions for a better API for creating Materials and ContactMaterials with use-cannon
?
Hmm, so the issue is that useMaterial
needs to be syncronous.
So this isn't feasible at all with the current api? What about creating a material each time a body is created and exposing it in the api.
const [ref1, api1] = useBox(() => ({ material: { friction: 0, restitution: 1 }, ... }))
const [ref2, api2] = useBox(() => ({ ... })) // default friction and restitution
useContactMaterial(api1.material, api2.material, { ...options })
Also the name
parameter could be used to make sure multiple bodies have the same material.
const elasticMaterial = { name: 'PerfectlyElastic', friction: 0, restitution: 1 }
const [ref1, api1] = useBox(() => ({ material: elasticMaterial, ... }))
const [ref2, api2] = useBox(() => ({ material: elasticMaterial, ... }))
Or even, it is a little bit less clean, but we could use strings:
const [ref1, api1] = useBox(() => ({ material: { name: 'PerfectlyElastic', friction: 0, restitution: 1 }, ... }))
const [ref2, api2] = useBox(() => ({ material: { name: 'Stiff', friction: 1 }, ... }))
useContactMaterial('PerfectlyElastic', 'Stiff', { ...options })
I don't know the use-cannon internals well enough though to know if this is feasible or could lead to some problems, so probably there are other issues I am not seeing.
@marcofugaro Could you clarify what api.material
represents in that first example? I know it can't be a CANNON.Material()
since that will be created asynchronously in the worker. Is it a ref or something?
The string/name
based examples are a little more clear to me but it does seem dangerous to rely on a global store of strings to distinguish between materials. In cannon-es
it's actually legal to have two different materials with the same name.
@marcofugaro Could you clarify what api.material represents in that first example? I know it can't be a CANNON.Material() since that will be created asynchronously in the worker. Is it a ref or something?
Yup, the material would be created in the worker and stay in the worker, maybe accessible from the dictionary you guys were talking about. What would be exposed to the main thread would be an object containing the id
and/or name
property (would need to be unique), the friction
and restitution
.
You would then use that id
to access the dictionary of materials in the worker when calling useContactMaterial(api1.material, api2.material, { ...options })
.
In cannon-es it's actually legal to have two different materials with the same name.
Yeah but it's not advised, in cannon-es the name is used only for debug purposes. Each created material has a unique id. I think it's safe to assume that bodies created with the same material name would share the same material, say if you create multiple bodies in a loop, you'd have no other way to make those bodies share the same material.
Do you guys see any other issues I am not seeing with this api?
What would be exposed to the main thread would be an object containing the id and/or name property (would need to be unique), the friction and restitution
Isn't that the same as a CANNON.Material()
😅 I think an issue with that is how to (synchronously) get the id
into the main thread, since it gets generated when the Material is created (in the worker). Maybe we could keep our own material idCounter
in the main thread to mirror the one on the worker thread, but those could get de-synchronized.
True! Yeah, we would need to create the id
in the main thread and pass it to the worker to use for the materials dictionary
In the interests of housekeeping, I'm going to close this pull request as it hasn't been touched for a year and is quite out of date. Feel free to re-open it with conflicts resolved and we can re-consider merging it.
I have an open PR adding ContactMaterial support and a Friction
example: https://github.com/pmndrs/use-cannon/pull/316
Would appreciate any code reviews!
Edit: New PR: https://github.com/pmndrs/use-cannon/pull/317
Resolves #64
I added an example
ContactMaterials.js
that shows a few balls bouncing with different restitution values. I noticed when setting the restitution to 1, the ball kinda bounces unevenly (sometimes gaining kinetic energy), not sure if that's an issue with my implementation or just the normal behavior of cannon-es. I followed the syntax you suggested:https://github.com/mattrossman/use-cannon/blob/2ab9fb0d1ff0b06c8efbcfb4ac685fcfdc0319d8/examples/src/demos/ContactMaterial.js#L26-L42
I'm not sure if you want to preserve the old way of passing material options directly into the useBody hook, or just use the new
useMaterial()
approach. For now I replaced the old approach which might break other examples (tbh I'm still not sure what the point is of setting individual material options the old way).Edit: also I have no idea how the instanced physics stuff works, so I haven't tested this in that scenario at all. I'm just passing
index=0
to theMaterialFn
since I saw that done somewhere else in the code.