[VRCSDK 3.9.1-beta.2] PermissionFilter behavioral breaking changes in contentTypes due to default value
available in future release
anatawa12
VRCSDK 3.9.1-beta.x introduced the 'contentTypes' property to the PermissionFilter struct, and it must be -1 to preserve behavior from previous versions.
However, the default value for 'contentTypes' is 0, which changes how PermissionFilters behave with the default value.
This breaks tools that assign PermissionFilter struct with default values.
Simplified code that reproduces the issue
// With this processor, all VRCPhysBoneBase components will only allow self-collision with VRCSDK 3.9.0.
// However, with VRCSDK 3.9.1-beta.x and later, because collisionFilter.contentTypes defaults to 0, it will not allow any global collisions including self-collision and world collisions.
//
// In real-world usage, we can except component to configure collisionFilter and others properties globally.
class AllowSelfOnly : IVRCSDKPreprocessAvatarCallback {
public int callbackOrder => -8192;
public bool OnPreprocessAvatar(GameObject avatarGameObject)
{
foreach (var physBone in avatarGameObject.GetComponentsInChildren<VRCPhysBoneBase>(true))
{
physBone.allowCollision = VRCPhysBoneBase.AdvancedBool.Other;
physBone.collisionFilter = new VRCPhysBoneBase.PermissionFilter
{
allowSelf = true,
allowOthers = false,
};
}
return true;
}
}
Minimum Reproducible Example
I created a simple avatar to test the behavior. The following two builds are build from exactly the same project with only changing the VRCSDK version.
The first one is built with VRCSDK 3.9.0, and global collision completely works, including self-collision.
The second one is built with VRCSDK 3.9.1-beta.2, and no global collision works, including self-collision.
The GitHub link is the assets used to build both avatars. The asset includes simple processor to change allowCollision, allowPosing and allowGrabbing globally to test the behavior.
Log In
This post was marked as
available in future release
Dexvoid
Thanks for reporting this issue to us.
We're currently considering adding a parameterless constructor to the PermissionFilter struct that sets contentTypes to Everything (-1). This would solve the issue for the code sample you've given above and any other use cases that directly construct a PermissionFilter object.
Creating one implicitly using the 'default' keyword would still create one with contentTypes set to Nothing (0), which we feel is valid behavior in this case since that is the data default for an enum type. However, please let us know if you have strong feelings against this as a solution!
From our side, this is preferable to inverting the meaning of the enum so 0 means Everything as this could get confusing for users and would make backwards compatibility more challenging.
anatawa12
Dexvoid
In short: I believe adding a parameterless constructor to the PermissionFilter struct will not resolve the original issue, and in fact is impossible to implement in C# 9.
> We're currently considering adding a parameterless constructor to the PermissionFilter struct
This is not possible in C# 9, which is the version currently used by Unity (2022.x..6.x). Therefore, this approach will not work.
Even if
VRC.Dynamics.dll
is compiled using C# 10 or later, which does support parameterless struct constructors, any code compiled with C# 9 will never invoke that constructor.Moreover, the default value of the struct is implicitly used in many places, including in the original issue and in the GitHub source linked earlier and below. Consequently, adding a parameterless constructor does not address the issue I originally reported.
> From our side, this is preferable to inverting the meaning of the enum so 0 means Everything as this could get confusing for users and would make backwards compatibility more challenging.
From a compatibility and usability perspective, I think the following 3 points are important:
- Existing code and worlds that rely on contentTypesflags being an allowlist.
- Denylists are generally more confusing than allowlists with flags enums.
- Serialized values are not compatible.
The third point is impossible to avoid, requires world authors to re-configure and rebuild their worlds when build-time values are changed.
However, the first two points can be addressed by adding a new field that represents denylist semantics and providing an accessor property that represents allowlist semantics.
EDIT: or you can bump the breaking version.
StormRel
marked this post as
tracked
bd_
Note that because PermissionFilter is a struct, the parameterless constructor will always initialize all of its fields to their default values (in this case, contentTypes is initialized to zero).
One way to resolve this would be to redefine the semantics of DynamicsUsageFlags to have zero mean everything, and to set flags for specific interactions to _deny_.