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 MA Merge VRM0+1 FirstPerson #602

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

kaikoga
Copy link
Contributor

@kaikoga kaikoga commented Dec 30, 2023

Discussion: #467
Merge base PR first: UniVRM components support #569

image

This PR introduces MA Merge VRM0+1 FirstPerson component, which adds FirstPerson settings to VRMFirstPerson for VRM0 / VRM10Object for VRM1, respectively.
This component supports both VRM0 and VRM1 because the data structure is very similar.
This component does nothing when the target avatar is not VRM0 or VRM1.

I think there is nothing specific to this component that requires localization. Perhaps there could be some warning like "This component is ignored; supports VRM0 or VRM1 avatar only", but many other components that targets VRCSDK only have similar issue.

I have not yet added documents. Merging this PR reveals UniVRM support to end users, so documentation would block the next release once this PR is merged.
I don't know yet where to add documents. (It would be under docs~/, but I worry that throwing VRM specific components into the Component reference section makes little sense to VRC users, when the entire components list is even more growing).

@kaikoga kaikoga force-pushed the vrm_merge_firstperson branch 2 times, most recently from a9f82b3 to 950cbd0 Compare December 30, 2023 05:06
@kaikoga kaikoga marked this pull request as ready for review December 30, 2023 06:10
@bdunderscore
Copy link
Owner

There should probably be a separate docs section for VRM components. We could set up something where VRM components are in a beta state, requiring a special define until they're in a usable state.

Probably I won't be able to merge this until 1.10.

@bdunderscore bdunderscore added this to the 1.10 milestone Jan 28, 2024
@kaikoga
Copy link
Contributor Author

kaikoga commented Jan 29, 2024

requiring a special define

Fair point. Should every #if MA_VRMx be #if MA_VRMx && MA_VRM_BETA (because if we would want to hide runtime components for users without opt-in, then the components would be wrapped in #if MA_VRM_BETA and become missing) then?

@kaikoga
Copy link
Contributor Author

kaikoga commented Jan 29, 2024

Another point of view: does VRM support deserve dedicated asmdefs, which would probably require major code reorganization?

@kaikoga
Copy link
Contributor Author

kaikoga commented Feb 12, 2024

Memo before rebase; maybe better name: VRM0+1 First Person Renderers ?

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

2 participants