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

Add quack and duck to bitecs #6072

Merged
merged 8 commits into from
May 23, 2023
Merged

Add quack and duck to bitecs #6072

merged 8 commits into from
May 23, 2023

Conversation

stalgiag
Copy link
Contributor

@stalgiag stalgiag commented May 11, 2023

This migrates duck and quack to bitecs and recreates what I believe was the original intended behavior:

  • Duck can be generated with '/duck'
  • Duck is networked, grabbable, and responds to physics
  • Duck play quack sound effect locally on grab, occasionally the quack is "special"

I tried to keep everything as near to the original as possible since this is an early change for me. I added comments to the diffs for situations where this was awkward.

part of #5899

src/bit-components.js Outdated Show resolved Hide resolved
src/bit-systems/quack.ts Outdated Show resolved Hide resolved
src/bit-systems/quack.ts Outdated Show resolved Hide resolved
@stalgiag stalgiag mentioned this pull request May 11, 2023
50 tasks
Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

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

LGTM, I just left a few of comments. Feel free to merge or address the feedback.

src/bit-systems/quack.ts Outdated Show resolved Hide resolved
src/bit-systems/quack.ts Outdated Show resolved Hide resolved
src/bit-components.js Outdated Show resolved Hide resolved
src/message-dispatch.js Outdated Show resolved Hide resolved
@stalgiag stalgiag changed the title Add quack to bitecs Add quack and duck to bitecs May 12, 2023
Comment on lines 146 to 148
const obj = APP.world.eid2obj.get(eid);
obj.position.copy(avatarPov.localToWorld(new THREE.Vector3(0, 0, -1.5)));
obj.lookAt(avatarPov.getWorldPosition(new THREE.Vector3()));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These three lines and the avatarPov line are repeated twice now. Feels like they should be a component down the line ("spawn-at-view").

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, in most cases we use this for spawning media in front of the user:
https://github.com/mozilla/hubs/blob/a0e5eee06395c5cfeb604e03dab03e9a33077620/src/scene-entry-manager.js#L244
If we add a component for this we can also refactor those methods.

@Exairnous
Copy link
Contributor

Does this preserve the reverse physics (floating) when the duck is scaled?

@stalgiag
Copy link
Contributor Author

stalgiag commented May 16, 2023

Does this preserve the reverse physics (floating) when the duck is scaled?

Good catch! I wasn't aware of this easter egg feature and thought the physics were similar to the default floatyObject. It looks like this behavior is a unique case exclusively for ducks. We don't currently have a duck system and one shouldn't be made for modifying physics.

I can add a flag on floatyObject which would allow for reuse.

@stalgiag
Copy link
Contributor Author

stalgiag commented May 16, 2023

I modified the FloatyObject component to handle Duck-specific physics. I made a new flag within FloatyObject in case we want to reuse this behavior elsewhere. The FloatyObject component could quickly become unwieldy if there are many different unique physics cases like this one. If this is the situation then I can decouple from floatyObject and make a new new helium component & system.

@Exairnous
Copy link
Contributor

Thanks! Yeah, it's a bit hidden, but really fun and useful. There is a way with the old loader to have arbitrary GLBs behave as ducks, so we've been able to hack in flying/floating objects that way, e.g. Floating_Lantern.zip.

So hopefully either that'll still work, or your suggestion of a helium component sounds good :)

@keianhzo keianhzo self-requested a review May 22, 2023 14:39
Copy link
Contributor

@keianhzo keianhzo left a comment

Choose a reason for hiding this comment

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

I had no idea about this behavior 😅 LGTM!

@stalgiag stalgiag merged commit 0a8edb7 into master May 23, 2023
10 of 12 checks passed
@stalgiag stalgiag deleted the bitecs-quack branch May 23, 2023 13:59
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.

None yet

3 participants