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.

Sign up for GitHub

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

Jump to bottom

Fix avatar physics capsule size mismatch #3624

Closed
wants to merge 3 commits into from
Closed

Fix avatar physics capsule size mismatch #3624

wants to merge 3 commits into from

Conversation

jakezira
Copy link
Contributor

@jakezira jakezira commented Aug 15, 2022 •
edited
Loading

Describe your changes

  • Updated character controller physx entity's height to cover full body
  • Updated physics collider rendering to match physx

What are the steps for a QA tester to test this pull request?

  • See collider of npc or avatar using tilde to check it's boundary compared to mesh.
  • Jump onto npc or avatar's top to see land height matches it's height

Issue ticket number and link

#3601

Screenshots and/or video

See below

Checklist before requesting a review

  • I have performed a self-review of my code
  • I am not adding any irrelevant code or assets
  • I am only including the changes needed to implement the change
  • I have playtested and intentionally tried to find error cases but couldn't

@jakezira jakezira requested review from avaer and arya3d and removed request for avaer August 15, 2022 00:43
@jakezira jakezira linked an issue Aug 15, 2022 that may be closed by this pull request
CCT Capsule doesn't match the PhysicsMesh capsule #3601
Open
@jakezira jakezira marked this pull request as draft August 15, 2022 00:45
@jakezira
Copy link
Contributor Author

jakezira commented Aug 15, 2022 •
edited
Loading

Related: #3493

Physics collider size:
image

Physx collide:
image
image

@jakezira jakezira marked this pull request as ready for review August 15, 2022 00:50
@jakezira jakezira requested a review from avaer August 15, 2022 00:50
avaer
avaer reviewed Aug 15, 2022
View reviewed changes
character-physics.js Outdated
const radius = (this.characterWidth / CHARACTER_CONTROLLER_HEIGHT_FACTOR) * this.characterHeight;
const height = this.characterHeight - radius * 2;
const radius = this.characterWidth / 2;
var height = this.characterHeight - radius * 2;
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we use var in the codebase. Only const and let.

avaer
avaer approved these changes Aug 15, 2022
View reviewed changes
Copy link
Contributor

@avaer avaer left a comment

Choose a reason for hiding this comment

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

Needs testing.

@alisaad673
Copy link
Contributor

Issues:

1/3-Height wise, seems fine. But what about side wise, The arms aren't being catered in the capsule that makes it possible for other vrms appear merged into on another.

2022-08-15.19-22-01.mp4

image
image

2/3-The other un-equipped vrms have different capsules. Like gear , miku etc.

image
image

3/3-Some of the other vrms don't respect the capsule:

image
image
image

@zenkale
Copy link

zenkale commented Sep 1, 2022

Closing in favor of Lab's PR

@zenkale zenkale closed this Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@avaer avaer avaer approved these changes

@arya3d arya3d Awaiting requested review from arya3d

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

CCT Capsule doesn't match the PhysicsMesh capsule
4 participants
@jakezira @alisaad673 @zenkale @avaer