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

feat: Expose some avatar root APIs #34

Merged
merged 9 commits into from
Oct 6, 2023

Conversation

kaikoga
Copy link
Contributor

@kaikoga kaikoga commented Sep 30, 2023

Related Issue: #49

This PR exposes some utility APIs:

  • RuntimeUtil.IsAvatarRoot()
  • RuntimeUtil.FindAvatarInParents()

Also, avatar root determination is defined in a cross platform way:

  • With VRCSDK: Treat object with VRCAvatarDescriptor as avatar root (unchanged).
  • Without VRCSDK: Treat upmost object with Animator as avatar root (proposed). Less reliable if some users prefer setup with nesting avatar descriptors upon transplanting outfits (as mentioned in MA's EasySetupOutfit), but we don't want to use platform specific components here.

@@ -2,7 +2,11 @@
using System.Collections.Generic;
using JetBrains.Annotations;
using UnityEngine;
using UnityEngine.SceneManagement;

#if VRC_SDK_VRCSDK3
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is it okay to add a version define like NDMF_VRCSDK3_AVATARS for "com.vrchat.avatars" to asmdef? (naming follows lilToon)

Copy link
Owner

Choose a reason for hiding this comment

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

Yes, that would be fine I think.

Copy link
Owner

Choose a reason for hiding this comment

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

In fact, I think it's necessary - I don't think the defines auto-set by the VRCSDK are a stable API.

@kaikoga kaikoga marked this pull request as draft September 30, 2023 08:14
@kaikoga
Copy link
Contributor Author

kaikoga commented Oct 2, 2023

Discussions: #39

@kaikoga kaikoga marked this pull request as ready for review October 2, 2023 15:09
bdunderscore
bdunderscore previously approved these changes Oct 3, 2023
@bdunderscore
Copy link
Owner

Looks like the CHANGELOG check has failed, please add an appropriate changelog entry and repush.

@bdunderscore bdunderscore merged commit 1151a7c into bdunderscore:main Oct 6, 2023
5 checks passed
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