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

fix: Fix combineSkeletons #1575

Merged
merged 1 commit into from
Jan 23, 2025
Merged

fix: Fix combineSkeletons #1575

merged 1 commit into from
Jan 23, 2025

Conversation

0b5vr
Copy link
Contributor

@0b5vr 0b5vr commented Jan 20, 2025

This PR fixes the VRMUtils.combineSkeletons.

The same skinIndex might be shared between different skeleton groups.

The condition to use the same skin index attribute:

  • the same skin index attribute
  • and the skeleton is the same
  • and the bone set is the same

In order to create a map from those three conditions to new skin index attributes, I implemented ObjectIndexDispatcher.
(low confidence implementation. would you come up with a better implementation/name?)
I hope this improved the readability.

TODOs

  • Is it working nicely?

Points need review

  • Is the implementation clear enough?

the same skinIndex might be shared between different skeleton groups

the condition to use the same skin index attribute:

- the same skin index attribute
- and the skeleton is same
- and the bone set is same

In order to create a map from those three conditions to new skin index attributes, I implemented `ObjectIndexDispatcher`
(low confidence implementation. would you come up with a better implementation/name?)
I hope this improved the readability
@0b5vr 0b5vr added the bug Something isn't working label Jan 20, 2025
@0b5vr 0b5vr added this to the next milestone Jan 20, 2025
@0b5vr 0b5vr self-assigned this Jan 20, 2025
@0b5vr 0b5vr marked this pull request as ready for review January 21, 2025 05:22
@0b5vr 0b5vr requested a review from ke456-png January 21, 2025 05:24
@0b5vr 0b5vr merged commit 51d6edb into dev Jan 23, 2025
5 checks passed
@0b5vr 0b5vr deleted the fix-combine-skeletons-2 branch January 23, 2025 08:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants