Skip to content

FIX: InputControl is not updated from the first time [ISXB-1221] #2221

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

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions Packages/com.unity.inputsystem/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Due to package verification, the latest version below is the unpublished version
however, it has to be formatted properly to pass verification tests.

## [Unreleased] - yyyy-mm-dd
- Fixed InputControl picker not updating correctly when the Input Actions Window was dirty. [ISXB-1221](https://issuetracker.unity3d.com/product/unity/issues/guid/ISXB-1221)

### Added
- Exposed MediaPlayPause, MediaRewind, MediaForward keys on Keyboard.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,11 @@

private void ShowDropdown(Rect rect, SerializedProperty serializedProperty, Action modifiedCallback)
{
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
#if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS
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 was automatic by VSCode

InputActionsEditorSettingsProvider.SetIMGUIDropdownVisible(true, false);
#endif
#endif
IsShowingDropdown = true;

Check warning on line 167 in Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs#L167

Added line #L167 was not covered by tests

if (m_PickerDropdown == null)
{
m_PickerDropdown = new InputControlPickerDropdown(
Expand All @@ -187,6 +189,8 @@
m_PickerDropdown.SetExpectedControlLayout(m_ExpectedControlLayout);

m_PickerDropdown.Show(rect);

IsShowingDropdown = false;

Check warning on line 193 in Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs#L193

Added line #L193 was not covered by tests
}

private void SetExpectedControlLayoutFromAttribute(SerializedProperty property)
Expand All @@ -206,12 +210,16 @@
private GUIContent m_PathLabel;
private string m_ExpectedControlLayout;
private string[] m_ControlPathsToMatch;
private InputControlScheme[] m_ControlSchemes;
private bool m_NeedToClearProgressBar;

private InputControlPickerDropdown m_PickerDropdown;
private readonly InputControlPickerState m_PickerState;
private InputActionRebindingExtensions.RebindingOperation m_RebindingOperation;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These three private properties were never really used, so I'm just cleaning it up as a harmless side effect on my main change.


/// <summary>
/// This property is only set from this class in order to communicate that we're showing the dropdown at the moment
/// It's employed to skip auto-saving, because that complicates updating the internal SerializedProperties.
/// Unfortunately, we can't use IMGUIDropdownVisible from the setings provider because of the early-out logic in there.
/// </summary>
public static bool IsShowingDropdown { get; private set; }

Check warning on line 222 in Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/ControlPicker/InputControlPathEditor.cs#L222

Added line #L222 was not covered by tests
Copy link
Collaborator Author

@K-Tone K-Tone Aug 7, 2025

Choose a reason for hiding this comment

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

Yes, that is a static property. However, it's only set from one place (private set), and it won't leak memory. Also, if we somehow happen to have two drop-down editor windows at the same time (which QA will correct me is barely possible). Even if somehow somehow opens two drop-down thingies at the same time, I believe we're fine here, as the more drop-down windows we have, the less sense it makes to auto-save during those sensitive moments of time. Hope that makes sense.

}
}
#endif // UNITY_EDITOR
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ internal class InputActionsEditorSettingsProvider : SettingsProvider
[SerializeField] InputActionsEditorState m_State;
VisualElement m_RootVisualElement;
private bool m_HasEditFocus;
private bool m_IgnoreActionChangedCallback;
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 was never used, dead code, I'm removing as a harmless side effect.

private bool m_IsActivated;
private static bool m_IMGUIDropdownVisible;
StateContainer m_StateContainer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,12 @@
// Auto-save triggers on focus-lost instead of on every change
#if UNITY_INPUT_SYSTEM_INPUT_ACTIONS_EDITOR_AUTO_SAVE_ON_FOCUS_LOST
if (InputEditorUserSettings.autoSaveInputActionAssets && m_IsDirty)
Save(isAutoSave: true);
// We'd like to avoid saving in case the focus was lost due to the drop-down window being spawned.
// This code should be cleaned up once we migrate the InputControl stuff from ImGUI completely.
// Since at that point it stops being a separate window that steals focus.
// (See case ISXB-1221)
if (!InputControlPathEditor.IsShowingDropdown)
Save(isAutoSave: true);

Check warning on line 352 in Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs

View check run for this annotation

Codecov GitHub.com / codecov/patch

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs#L351-L352

Added lines #L351 - L352 were not covered by tests
#endif

analytics.RegisterEditorFocusOut();
Expand Down