Use VRCParentConstraint instead of constraint hack for world fixed objects when available #1326
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
for world fixed objects when available
bdunderscore
reviewed
Nov 2, 2024
Editor/WorldFixedObjectProcessor.cs
Outdated
Comment on lines
89
to
108
| #if MA_VRCSDK3_AVATARS_3_7_0_OR_NEWER | ||
| var isVrcAvatar = avatarRoot.TryGetComponent(out VRC.SDKBase.VRC_AvatarDescriptor _); | ||
|
|
||
| if (isVrcAvatar) | ||
| { | ||
| var constraint = obj.AddComponent( | ||
| System.Type.GetType("VRC.SDK3.Dynamics.Constraint.Components.VRCParentConstraint, VRC.SDK3.Dynamics.Constraint") | ||
| ) as VRC.Dynamics.ManagedTypes.VRCParentConstraintBase; | ||
| constraint.IsActive = true; | ||
| constraint.Locked = true; | ||
| constraint.AffectsPositionX = true; | ||
| constraint.AffectsPositionY = true; | ||
| constraint.AffectsPositionZ = true; | ||
| constraint.AffectsRotationX = true; | ||
| constraint.AffectsRotationY = true; | ||
| constraint.AffectsRotationZ = true; | ||
| constraint.FreezeToWorld = true; | ||
| } | ||
| else | ||
| #endif |
There was a problem hiding this comment.
I'd prefer here to extract this logic to a method and swap out the method as a whole, rather than having an else block spanning an #endif like this...
There was a problem hiding this comment.
IDK if the committed change is OK, it is hard to extract as a method because the case is "when VRCSDK with constraints is imported" (only can be filtered with #if preprocessor) and "we are working on a VRChat avatar" (only can be checked with if (...) ... else block)
There was a problem hiding this comment.
I'm thinking something like:
if (!TryGenerateVRChatConstraint(obj)) GenerateGenericConstraint(obj);
// ...
#if MA_VRCSDK3_AVATARS_3_7_0_OR_NEWER
bool TryGenerateVRChatConstraint(GameObject obj) { ... }
#else
bool TryGenerateVRChatConstraint(GameObject obj) { return false; }
#endif
|
It looks like the unit tests are broken now... |
|
Yes it requires changes on unit test, it is coded to check if I'm not very familiar on modifying unit tests, especially with such conditional flow. |
bdunderscore
approved these changes
Nov 17, 2024
bdunderscore
added a commit
that referenced
this pull request
Nov 20, 2024
…fixed objects when available (#1326)" This reverts commit a984cf8. The prior behavior was to lock world fixed objects at their offset from the origin; however with this change we ended up locking them at a location relative to the avatar spawn location, breaking some gimmicks. I tried experimenting a bit with VRCConstraint to try to replicate this behavior, but it seems a bit non-trivial, so we'll revert this as a hotfix for now.
bdunderscore
added a commit
that referenced
this pull request
Nov 20, 2024
…fixed objects when available (#1326)" (#1363) This reverts commit a984cf8. The prior behavior was to lock world fixed objects at their offset from the origin; however with this change we ended up locking them at a location relative to the avatar spawn location, breaking some gimmicks. I tried experimenting a bit with VRCConstraint to try to replicate this behavior, but it seems a bit non-trivial, so we'll revert this as a hotfix for now.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
As title indicates, this patches
WorldFixedObjectProcessorto useVRCParentConstraintinstead of old-fashioned Unity constraint hack for world fixed objects, only when such components are available and we're working on VRChat avatars.