FIX: InputControl is not updated from the first time [ISXB-1221] #2221
Conversation
This file contains hidden or 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
Codecov ReportAttention: Patch coverage is
@@ Coverage Diff @@
## develop #2221 +/- ##
===========================================
+ Coverage 68.10% 68.12% +0.01%
===========================================
Files 367 367
Lines 53629 53633 +4
===========================================
+ Hits 36524 36535 +11
+ Misses 17105 17098 -7
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes 🚀 New features to boost your workflow:
|
K-Tone
changed the title
K-Tone
changed the title
K-Tone
commented
Aug 7, 2025
| @@ -161,9 +161,11 @@ public void OnGUI(Rect rect, GUIContent label = null, SerializedProperty propert | |||
|
|
|||
| private void ShowDropdown(Rect rect, SerializedProperty serializedProperty, Action modifiedCallback) | |||
| { | |||
| #if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS | |||
| #if UNITY_INPUT_SYSTEM_PROJECT_WIDE_ACTIONS | |||
There was a problem hiding this comment.
This was automatic by VSCode
K-Tone
commented
Aug 7, 2025
| /// 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; } |
There was a problem hiding this comment.
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.
K-Tone
force-pushed
the
anthony/isxb-fix-input-control-picker
branch
from
95dbcf9 to
f38d3ae
Compare
|
I don't think I can make an automated test for this myself, it's rather tricky UI/Editor case involving ImGUI and UIElements among other things. |
K-Tone
commented
Aug 12, 2025
| @@ -17,7 +17,6 @@ internal class InputActionsEditorSettingsProvider : SettingsProvider | |||
| [SerializeField] InputActionsEditorState m_State; | |||
| VisualElement m_RootVisualElement; | |||
| private bool m_HasEditFocus; | |||
| private bool m_IgnoreActionChangedCallback; | |||
There was a problem hiding this comment.
This was never used, dead code, I'm removing as a harmless side effect.
K-Tone
commented
Aug 12, 2025
|
|
||
| private InputControlPickerDropdown m_PickerDropdown; | ||
| private readonly InputControlPickerState m_PickerState; | ||
| private InputActionRebindingExtensions.RebindingOperation m_RebindingOperation; |
There was a problem hiding this comment.
These three private properties were never really used, so I'm just cleaning it up as a harmless side effect on my main change.
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.
Description
This aims fixing the problem where the InputControl picker won't apply the selected item every second time basically (only when auto-save was on). To be technically precise, it wasn't applying the selected item every time when the input actions window was dirty, which normally happens in two cases:
The root cause for these issues is that our input picker drop-down menu is technically a separate EditorWindow that steals focus from the input actions window as it's open. This was unfortunate because the input actions window subscribes to focus loss in order to save changes and reload all the state as it does so. Practically, if the input actions window was dirty, you click on the picker, that changes focus and triggers auto-save right there. It re-creates all the backing for the window, but the picker uses SerializedProperties, it doesn't talk to data directly. Because of that, when the new assets are loaded as part of auto-save, it won't pick the right data up.
Hereby we propose to address that by avoiding auto-save at that sensitive moment in time while we're opening the drop-down thing for the input control picker.
Testing status & QA
Local testing, QA pending
Overall Product Risks
Comments to reviewers
Please describe any additional information such as what to focus on, or historical info for the reviewers.
Checklist
Before review:
Changed,Fixed,Addedsections.Area_CanDoX,Area_CanDoX_EvenIfYIsTheCase,Area_WhenIDoX_AndYHappens_ThisIsTheResult.During merge:
NEW: ___.FIX: ___.DOCS: ___.CHANGE: ___.RELEASE: 1.1.0-preview.3.After merge: