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

[1.0] Add Humanoid helper #987

Merged
merged 2 commits into from
Jun 23, 2022
Merged

[1.0] Add Humanoid helper #987

merged 2 commits into from
Jun 23, 2022

Conversation

0b5vr
Copy link
Contributor

@0b5vr 0b5vr commented Jun 9, 2022

Description

This PR adds VRMHumanoidHelper which visualizes humanoid bones.

image

This PR was originally done in #889 but since Humanoid Rig changes are going to be difficult I separated the humanoid helper changes into this PR.

@0b5vr 0b5vr added enhancement New feature or request VRM 1.0 labels Jun 9, 2022
@0b5vr 0b5vr added this to the VRM 1.0 milestone Jun 9, 2022
@0b5vr 0b5vr self-assigned this Jun 9, 2022
@0b5vr 0b5vr marked this pull request as ready for review June 9, 2022 10:38
this._boneAxesMap = new Map();

Object.values(humanoid.humanBones).forEach((bone) => {
if (bone) {
Copy link
Contributor

Choose a reason for hiding this comment

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

ここ、boneはfalsyになることあるんですね。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

そんなことないですね。if文外してしまいます。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

あ、Type assertionが必要だった。

ただ、新しめのTypeScriptだと必要ないassertionのため、TODOでその旨コメントを残しておきました。

4fc58ab

Array.from(this._boneAxesMap.entries()).forEach(([bone, axes]) => {
bone.node.updateWorldMatrix(true, false);

bone.node.matrixWorld.decompose(_v3A, _quatA, _v3B);
Copy link
Contributor

Choose a reason for hiding this comment

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

decomposeしたけど_v3aと_quatAは特に使っていない。

後で直接matrixWorldをコピーしている。

Copy link
Contributor

@nyamadan nyamadan left a comment

Choose a reason for hiding this comment

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

レビューしました!この部分の修正だけでしたら再レビューは不要と思います。

https://github.com/pixiv/three-vrm/pull/987/files/908141c13d5c7033268cf55c3fb9384f6dff2fd2#r904470456

Add a type assertion instead

The type assertion is not needed if we use later versions of TypeScript. Remove it later
@0b5vr 0b5vr merged commit 0192653 into 1.0 Jun 23, 2022
@0b5vr 0b5vr deleted the humanoid-helper branch June 23, 2022 04:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request VRM 1.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants