Skip to content

FIX: disabling the InputSystemUIInputModule component resets all actions to None #2192

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

Merged
merged 9 commits into from
Jun 27, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -236,7 +236,7 @@ public void Report(string message)
}
}

[Test(Description = "Verifies that the default asset do not generate any verification errors (Regardless of existing requirements)")]
[Test(Description = "Verifies that the default asset does not generate any verification errors (Regardless of existing requirements)")]
[Category(kTestCategory)]
public void ProjectWideActions_ShouldSupportAssetVerification_AndHaveNoVerificationErrorsForDefaultAsset()
{
17 changes: 17 additions & 0 deletions Assets/Tests/InputSystem/Plugins/UITests.InputModuleTests.cs
Original file line number Diff line number Diff line change
@@ -157,6 +157,23 @@ public IEnumerator PointerExitChildShouldFullyExit()
Assert.IsTrue(callbackCheck.pointerData.fullyExited == true);
}

[UnityTest]
[Description("Regression test for https://jira.unity3d.com/browse/ISXB-1493")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Great you added a specific test for the regression

public IEnumerator DisablingDoesNotResetUserActions()
{
var actions = new DefaultInputActions();
m_InputModule.actionsAsset = actions.asset;
m_InputModule.cancel = InputActionReference.Create(actions.UI.Cancel);

m_InputModule.enabled = false;

yield return null;

Assert.IsNotNull(m_InputModule.cancel, "Disabling component shouldn't lose its data.");

actions.Dispose();
}

public class PointerExitCallbackCheck : MonoBehaviour, IPointerExitHandler
{
public PointerEventData pointerData { get; private set; }
2 changes: 2 additions & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -28,6 +28,8 @@ however, it has to be formatted properly to pass verification tests.
- Fixed Gamepad stick up/down inputs that were not recognized in WebGL. [ISXB-1090](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1090)
- Fixed reenabling the VirtualMouseInput component may sometimes lead to NullReferenceException. [ISXB-1096](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1096)
- Fixed the default button press point not being respected in Editor (as well as some other Touchscreen properties). [ISXB-1152](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1152)
- Fixed actions being reset when disabling the InputSystemUIInputModule component [ISXB-1493](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1493)
- Fixed a memory leak when disabling and enabling the InputSystemUIInputModule component at runtime [ISXB-1573](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1573)
- Fixed PlayerInput component automatically switching away from the default ActionMap set to 'None'.
- Fixed a console error being shown when targeting visionOS builds in 2022.3.
- Fixed a Tap Interaction issue with analog controls. The Tap interaction would keep re-starting after timeout. [ISXB-627](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-627)
Original file line number Diff line number Diff line change
@@ -1528,6 +1528,17 @@
set => SwapAction(ref m_TrackedDevicePositionAction, value, m_ActionsHooked, m_OnTrackedDevicePositionDelegate);
}

// We should dispose the static default actions thing because otherwise it will survive domain reloads
[RuntimeInitializeOnLoadMethod(RuntimeInitializeLoadType.SubsystemRegistration)]
static void ResetDefaultActions()
{
if (defaultActions != null)
{
defaultActions.Dispose();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess, calling Dispose is just to stay on the safe side since the only data default actions stores is a ref to ScriptableObject that won't survive even with domain reload off. It will be in this intermediate state though, where its native data is gone but the instance is not null until the next gc run (however you'll see it as "null" in debugger, and ==null will be true as well, thanks to Unity's Equals overload for all Object descendants).

Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this comment imply that it should be Object.ReferenceEquals(null, defaultActions) in if statement above? If its disposable, yes it should be disposed, even if the implementation would do nothing IMO.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I wasn't clear, pardon. DefaultActions is fine to check for null like above since it's not destroyed natively. What is in the intermediate state happens to be aggregated in defaultActions (DefaultActions has a ref to ScriptableObject, so it's the ScriptableObject would have to be checked with ReferenceEquals if we needed to, but we don't).

defaultActions = null;
}
}

Check warning on line 1540 in Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Plugins/UI/InputSystemUIInputModule.cs#L1534-L1540

Added lines #L1534 - L1540 were not covered by tests

/// <summary>
/// Assigns default input actions asset and input actions, similar to how defaults are assigned when creating UI module in editor.
/// Useful for creating <see cref="InputSystemUIInputModule"/> at runtime.
@@ -1665,7 +1676,12 @@
InputActionState.s_GlobalState.onActionControlsChanged.RemoveCallback(m_OnControlsChangedDelegate);
DisableAllActions();
UnhookActions();
UnassignActions();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

^ This line is very wrong, it results in user data being lost on component deactivation.


// In the case we've been initialized with default actions, we want to release them
if (defaultActions != null && defaultActions.asset == actionsAsset)
{
UnassignActions();
}

base.OnDisable();
}