Add Mirror to bitecs #5919
Add Mirror to bitecs #5919
Conversation
takahirox
force-pushed
the
BitECSMirror
branch
from
1516b93
to
2a61c4c
Compare
| addComponent(world, Mirror, eid); | ||
| return eid; | ||
| } | ||
|
|
There was a problem hiding this comment.
Duplicated code with https://github.com/mozilla/hubs/blob/master/src/components/mirror.js I thought it would be ok because not so many codes and src/components/mirror.js may be removed when the new framework will land.
If we want to avoid duplicated code we can add a new file and let src/inflators/mirror.ts and src/components/mirror.js refer to it.
There was a problem hiding this comment.
I think its ok, since the plan is to remove the old code once we remove the newLaoder flag and flip to using the new system by default.
|
|
||
| export function inflateMirror(world: HubsWorld, eid: number, params: MirrorParams) { | ||
| params = Object.assign({}, DEFAULTS, params); | ||
| const geometry = DEFAULT_MIRROR_GEOMETRY; |
There was a problem hiding this comment.
In the old code, this.el.object3DMap?.mesh?.geometry is used if defined.
https://github.com/mozilla/hubs/blob/40bbcc41c6b5c2eac2982b0a8bf2bbebd44db195/src/components/mirror.js#L24
In the new framework, is there a chance that geometry is already defined when inflating, too?
There was a problem hiding this comment.
Since mirror is currently only exposed in Spoke, and Spoke doesn't do this, I don't think we need to support that right now.
Just to think through what it would look like if we wanted to support this later): It would be slightly tricky since in infiltrators you can not access the Object3D for the entity you are inflating on (since inflators may be what creates the Object3D) so we would need to instead write a system that swaps the existing mesh with a Reflector using the same Geometry.
takahirox
force-pushed
the
BitECSMirror
branch
from
2a61c4c
to
e1ec671
Compare
|
|
||
| export function inflateMirror(world: HubsWorld, eid: number, params: MirrorParams) { | ||
| params = Object.assign({}, DEFAULTS, params); | ||
| const geometry = DEFAULT_MIRROR_GEOMETRY; |
There was a problem hiding this comment.
Since mirror is currently only exposed in Spoke, and Spoke doesn't do this, I don't think we need to support that right now.
Just to think through what it would look like if we wanted to support this later): It would be slightly tricky since in infiltrators you can not access the Object3D for the entity you are inflating on (since inflators may be what creates the Object3D) so we would need to instead write a system that swaps the existing mesh with a Reflector using the same Geometry.
| const geometry = DEFAULT_MIRROR_GEOMETRY; | ||
| const reflector = new Reflector(geometry, { | ||
| color: params.color, | ||
| textureWidth: DEFAULT_TEXTURE_WIDTH, |
There was a problem hiding this comment.
This is not new and we don't necessarily need to fix this right now, but I quite dislike that we make this so high resolution... Not sure what a more sane value would be.
There was a problem hiding this comment.
Yes, probably lower resolution will be better. Added a comment for now.
| addComponent(world, Mirror, eid); | ||
| return eid; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think its ok, since the plan is to remove the old code once we remove the newLaoder flag and flip to using the new system by default.
takahirox
force-pushed
the
BitECSMirror
branch
from
e1ec671
to
73dd914
Compare
This PR adds
Mirrorto bitecs.