[SDK 3.7.0-beta.1] Please provide a hook for supplying additional animation clips and their relative paths during Auto Fix transformations of constraints
available in future release
bd_
Opening this again as https://feedback.vrchat.com/open-beta/p/add-a-public-vrcsdk-api-method-for-silently-invoking-the-constraint-upgrader is not the same issue and was probably inappropriately merged with Fuka's post.
In 3.7.0-beta.1 there is an auto fix for converting existing constraints to VRC Constraints. This enumerates animation clips in the animator controllers attached to the avatar descriptor, and rewrites their curves to match the new constraints. However, it will not be able to identify animations merged using nondestructive utilities. Thus, when the user runs the auto fix, they'll end up with components converted, but animations left unchanged (or vice versa).
While Senky's post is a useful one (we could invoke the converter at build time to help address this issue), being able to invoke the conversion process from our tools does not resolve the issue of the auto fix action, when invoked manually by the end-user from the VRCSDK UI directly, missing animation clips that need adjustment. As such, an API is needed to allow nondestructive tools help the VRCSDK discover any additional animation clips that need to be processed during this manual auto fix invocation.
Note also that both VRCFury and Modular Avatar perform animation path rewriting, so your API will need to be amenable to this as well. MA currently only does prefix rewriting, but I have plans to add more advanced forms of path rewriting in the future.
Log In
This post was marked as
available in future release
Dexvoid
tracked
This post was marked as
available in future release
Dexvoid
tracked
Dexvoid
Thanks for the info. I'd like to verify that I've understood this correctly.
When the user uses the menu option VRChat SDK/Utilities/Convert Unity Constraints To VRChat Constraints with one or more prefabs/game objects selected, we should invoke a C# function (System.Func) that MA or VRCF can subscribe to and use to return a set of animation clips to convert. The VRCSDK will then convert only those animation clips instead of attempting to find valid clips itself.
MA/VRCF would subscribe to a C# Func exposed by AvatarDynamicsSetup as the editor starts (for example by using a method with the InitializeOnLoadMethod attribute). The Unity constraint currently being converted and the avatar descriptor found in the current game object or its parents could be provided as parameters. If the function is not subscribed to, we will try to find clips from the avatar descriptor as usual.
Does this sound correct? Is there any other info you would need as part of the function?
Additionally, users can convert individual animation clips in their Unity project directly through this feature - can you confirm whether this would also need support for any special handling?
bd_
Dexvoid
> The VRCSDK will then convert only those animation clips instead of attempting to find valid clips itself.
Since MA and Fury can be in the same project at the same time, I'd suggest using an event function to allow both of us to offer clips:
public static event void CollectReferencedAnimations(VRCAvatarDescriptor avatarRoot, Component targetComponent, HashSet<(AnimationClip, string)> clipsAndPaths);
/// Somewhere in MA...
[InitializeOnLoadFunction]
static void Init() {
SomeVRCSDKClass.CollectReferencedAnimations += SupplyAnimationClips;
}
It's probably okay for VRCSDK to handle the avatar descriptor-referenced clips itself; I don't see a strong usecase for excluding those, but it would be good to get Senky's input as well.
With regards to converting specific components - there's the challenge that MA rewrites animation paths, and in fact can have the same animation clip asset merged multiple times with different rewrite rules. One possible way to handle this would be for MA to throw an exception and show an error UI in this case, as I don't see a very clean way to deal with this without user intervention (it would require creating new animator controllers and clips...)
bd_
Also as a bit of context re path rewriting - as far as MA is concerned, any paths in the VRCSDK descriptor refer to the initial (pre-build) state of the avatar. For now. This might not be true forever, if I ever get around to implementing a symlink-like component, but presumably the constraint converter will be less important by then.
Dexvoid
bd_ Thanks very much, this is very helpful. I'm assuming in your event example that the
string
in clipsAndPaths
represents a new path we should assign across all of the curve bindings for this clip (https://docs.unity3d.com/ScriptReference/EditorCurveBinding-path.html) during auto-conversion. If the string is null or empty, then we won't modify the path per the default behavior for the VRCSDK. Please let me know if any of that is incorrect.Based on your comments we can also search the avatar descriptor for animation clips even if a custom collection event function is given, and the results of the two would be combined together ignoring any duplicates.
bd_
Dexvoid Rather, the path would be the path corresponding to the component in question in the clip (thus, if you want to limit your modification to just the one constraint, you can avoid touching other constraints in the same clip). I suppose it might also be good to pass a list or enumerable of components, just so that we can avoid having to walk all of the animator controllers multiple times.
In particular, you would never want to modify the paths in the animation clip - after all, the GameObjects aren't changing. That's MA's job - MA just needs to be able to signal to VRCSDK what paths correspond to the components being modified.
bd_
Also, it would be really appreciated if you could preview the specific final API with Senky and I before throwing it out in a full release that has to be supported forever onward 😅
VexaMoonlight
bd_I think the event could be something like:
public static event void CollectReferencedAnimations(object sender, CollectReferencedAnimationsEventArgs e)
which stores everything inside of it as a class from above that has
get; set;
on most of the properties inside of it. This is how I do events to keep things very simple.Also then the default VRChat handling
could
then also use this event as well and everything would be wonderful for everyone.From writing .NET code for years this is how events
should
be written (according to the .NET Team at Microsoft), everything they expect the event to edit should have set
on the properties in the EventArgs class for the event and then the sender of it should then (after event returns) obtain the value(s) from the event args object for it.Dexvoid
bd_
> Rather, the path would be the path corresponding to the component in question in the clip
Apologies if I'm misinterpreting this, but since a curve binding path gives the path to the containing game object in the hierarchy, that wouldn't be enough information to identify the constraint component affected by this curve since a game object can have multiple constraints of different types on it. You would need the type of constraint in addition to that to identify the component affected by the curve. This is why the currently proposed interface gives a single Unity constraint component, since it allows us to map from each component to the set of animation clips associated with it.
> Also, it would be really appreciated if you could preview the specific final API with Senky and I before throwing it out in a full release that has to be supported forever onward
We're discussing the viability of private SDK builds internally now.
bd_
Dexvoid
> Apologies if I'm misinterpreting this, but since a curve binding path gives the path to the containing game object in the hierarchy, that wouldn't be enough information to identify the constraint component affected by this curve since a game object can have multiple constraints of different types on it. You would need the type of constraint in addition to that to identify the component affected by the curve.
I was envisioning receiving a component from the API and giving back paths, yes. However game objects would also work as I don’t have anything that moves individual constraint components around at the moment. In either case the animation clip discovery process is heavyweight, so I’d want to avoid doing it multiple times if the user has multiple constraints selected.
bd_
I’ll also mention that both me and Senky are considering ways to auto convert the entire avatar, nondestructively at build time, which avoids numerous problems with the auto fix approach. As a general rule MA won’t do something unless the user opts in, but I could see a world where auto fix just adds a “MA Convert Constraints” to the avatar root, given the right VRCSDK hooks.
bd_
As a concrete alternative:
interface IVRCConstraintConversionProvider {
/// Returns true if some nondestructive system will autoconvert VRCConstraints on this avatar root (and therefore the warning should be suppressed)
bool WillAutoConvertVRCConstraints(GameObject avatarRoot);
/// Requests that the provider modify the avatar to convert all VRCConstraints (invoked on auto fix)
void AutoFixConvertVRCConstraints(GameObject avatarRoot);
}
static class SomeAppropriateConfigurationClass {
public static IVRCConstraintConversionProvider VRCConstraintConversionProvider { get; set; }
}
bd_
Due to the risk of breaking prefabs I'm now more inclined to patch auto fix to use a nondestructive method of conversion rather than participate in destructive conversion of existing animations; the above IVRCConstraintConversionProvider strategy would help avoid the need for using Harmony for this.
(Or you could provide a nondestructive component yourself...)
Dexvoid
bd_ SenkyDragon
After reviewing this further and based on the comments here, we're now looking at replacing this with a simpler delegate event that's invoked once as we begin the process of converting to VRChat constraints, or in other words as soon as the user presses the Auto-Fix button or uses the equivalent menu entries. It would have a signature like this:
public delegate bool ConvertUnityConstraintsOnGameObjectsDelegate(List<GameObject> gameObjects);
The list of game objects is simply the set of game objects selected by the user, or the game object containing the avatar descriptor of the current avatar if the auto-fix option is used.
The return value tells us if the VRCSDK should go attempt its own conversion or not. If you return true, we'll immediately stop on the assumption that conversion has been handled by a user integrated tool instead - the warning dialog wouldn't be shown either. If false is returned or nothing is subscribed to the event, we'll continue with the built-in converter as usual. If there are multiple subscribers, we'll run all of them and abort the SDK's converter if any of them have returned true.
This will allow you to implement your own handling of this problem without any intervention from the VRCSDK.
Note that we also allow users to right click on individual animation clips in the project window and convert those individually. We'll expose a similar delegate event for this as well, in case it's needed:
public delegate bool ConvertUnityConstraintsOnAnimationClipsDelegate(List<AnimationClip> animationClips);
Again, the list would contain the clips selected for conversion by the user, and we would abort from the SDK if true is returned.
Please let us know if the above is correct or if further changes are needed. Regarding previewing this before it goes live, we are planning to release a beta SDK with these changes for you to review.
Dexvoid
Additionally:
- If a delegate throws an exception, we will log it and then intepret it as a signal not to run the SDK's converter.
- We are exposing the methods to convert a set of animation clips (ConvertUnityConstraintTracksToVrChatConstraintTracks()) and apply rebinding to a single animation clip (RebindConstraintAnimationClip()) in case they're needed.
bd_
Dexvoid
I'd recommend the addition of one more delegate:
public delegate bool AreUnityConstraintsAutoConverted(List<GameObject> gameObjects);
The intent for this is to suppress the auto fix message (and quest build block) if the conversion is being handled in a non-destructive way (ie - if there is a configuration to convert these constraints at build time). If this is not implemented, I intend to perform Harmony patching to suppress the message, but it would be preferable to have this be a supported API.
Alternately:
public delegate bool IsUnityConstraintAutoConverted(IConstraint constraint);
(this one is probably easier to avoid MA and VRCF stepping on each other's toes...)
Dexvoid
bd_ I've added your second suggestion. It will be called for every Unity constraint we detect on the avatar before the validation message is displayed. If true is returned for all of them, the validation message won't appear.
bd_
Dexvoid that sounds perfect, thanks! Once the beta SDK is out I’ll put together a MA patch to address things on my side.
bd_
Quick feedback - I think we need separate callbacks for "user invoked auto fix" vs "user tried to convert via the VRChat SDK -> Utilities option" vs "a script tried to convert this". I think only the auto fix one really needs to be overridden here.
bd_
https://github.com/bdunderscore/modular-avatar/pull/1010 here's the implementation I'm considering, for your reference
Dexvoid
bd_
Thanks, I'm pleased to see you've been able to integrate with this.
> I think we need separate callbacks for "user invoked auto fix" vs "user tried to convert via the VRChat SDK -> Utilities option" vs "a script tried to convert this"
To avoid creating too many delegates, we could instead pass an additional boolean parameter to
OnConvertUnityConstraintsOnGameObjects
to indicate if this was triggered by the user clicking auto-fix or not. You'd be able to return immediately if it's false.Dexvoid
bd_ SenkyDragon
Heads up, we will be renaming some of these symbols in the next version of the SDK. We're doing this to try and simplify their names a bit and make it clearer which of these display modal dialogs and can be affected by external tooling, since it would be a lot more difficult to do this after the SDK is live.
The new method and event names will be included in our documentation for constraints, but a summary of the renames is below.
Methods
ConvertUnityConstraintsToVrChatConstraints(List<GameObject> targetGameObjects)
was renamed to
ConvertUnityConstraintsAcrossGameObjects(List<GameObject> targetGameObjects)
--
ConvertUnityConstraintTracksToVrChatConstraintTracks(List<AnimationClip> targetAnimationClips)
was renamed to
ConvertUnityConstraintsAcrossAnimationClips(List<AnimationClip> targetAnimationClips)
--
ConvertUnityConstraintsToVrChatConstraints(IConstraint[] unityConstraints, VRCAvatarDescriptor avatarDescriptor, bool convertReferencedAnimationClips)
was renamed to
DoConvertUnityConstraints(IConstraint[] unityConstraints, VRCAvatarDescriptor avatarDescriptor, bool convertReferencedAnimationClips)
Events
OnConvertUnityConstraintsOnGameObjects
was renamed to
OnConvertUnityConstraintsAcrossGameObjects
--
OnConvertUnityConstraintsOnAnimationClips
was renamed to
OnConvertUnityConstraintsAcrossAnimationClips
bd_
Dexvoid thanks for being so responsive! I’ve implemented the auto fix handling in MA 1.9.14.