UITK drag and drop (ISX-1141) #1831

ritamerkl · 2024-02-05T14:36:42Z

Description

This PR adds Drag & Drop support for the UITK-Editor. Find the ticket here.
Basically it includes reordering and moving of ActionMaps, Actions and Bindings in the Editor.

Changes made

added:

reordering ActionMaps
moving Actions to other ActionMaps
reordering Actions
moving Bindings and Composites to other Actions or reorder Bindings/Composites in the same Action
moving Composite parts from one Composite to other Composites or reordering them in the same Composite

Checklist

Before review:

Changelog entry added.
- Explains the change in Changed, Fixed, Added sections.
- For API change contains an example snippet and/or migration example.
- FogBugz ticket attached, example ([case %number%](https://issuetracker.unity3d.com/issues/...)).
- FogBugz is marked as "Resolved" with next release version correctly set.
Tests added/changed, if applicable.
- Functional tests Area_CanDoX, Area_CanDoX_EvenIfYIsTheCase, Area_WhenIDoX_AndYHappens_ThisIsTheResult.
- Performance tests.
- Integration tests.
Docs for new/changed API's.
- Xmldoc cross references are set correctly.
- Added explanation how the API works.
- Usage code examples added.
- The manual is updated, if needed.

During merge:

Commit message for squash-merge is prefixed with one of the list:
- NEW: ___.
- FIX: ___.
- DOCS: ___.
- CHANGE: ___.
- RELEASE: 1.1.0-preview.3.

…-and-drop # Conflicts: # Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/InputActionsEditorWindow.cs # Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ActionMapsView.cs # Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ActionsTreeView.cs

…view

…-and-drop

…-and-drop # Conflicts: # Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ActionMapsView.cs

…ctions

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs

graham-huws · 2024-02-06T14:29:26Z

Added a couple minor things, nothing really to add beyond what Håkan mentioned - I'm always a fan of more comments explaining functions though :)

…-and-drop

Pauliusd01

Things I noticed so far:

I can drop composite bindings into non composite actions (and vice versa) -> after dragging them they jump back to where they were. Expected: block symbol on mouse is shown and you can't drag them there at all
Undo naming is not user friendly shown as "Modified Actions Maps.Array..." or "Modified x# properties"
Expected: "Moved Action/Binding x" does not need to go into specifics where it was moved, just has to be human readable of what action was done
Unknown steps to error so far but got it when loading the ProjectWideActions test scene (ignore if cause not clear from error message alone):

NullReferenceException: Object reference not set to an instance of an object
UnityEngine.InputSystem.InputActionMap.Disable () (at ./Packages/com.unity.inputsystem/InputSystem/Actions/InputActionMap.cs:559)
UnityEngine.InputSystem.InputActionAsset.Disable () (at ./Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs:818)
UnityEngine.InputSystem.InputActionAsset.OnDestroy () (at ./Packages/com.unity.inputsystem/InputSystem/Actions/InputActionAsset.cs:919)
UnityEngine.GUIUtility:ProcessEvent(Int32, IntPtr, Boolean&) (at D:/Gitrepo/unity/Modules/IMGUI/GUIUtility.cs:219)

I can move an action to its own map and then it dissapears with this error (steps to repro: Create new custom input actions asset with just one action -> move that action to its own map -> it dissapears. Does not seem to repro if the map has more than 1 action):

NullReferenceException: Object reference not set to an instance of an object
UnityEngine.InputSystem.Editor.CopyPasteHelper.IsComposite (UnityEditor.SerializedProperty property) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs:26)
UnityEngine.InputSystem.Editor.CopyPasteHelper.PasteBindingOrComposite (UnityEditor.SerializedProperty arrayProperty, System.String json, System.Int32 index, System.String actionName, System.Boolean createCompositeParts) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs:263)
UnityEngine.InputSystem.Editor.CopyPasteHelper.PasteAction (UnityEditor.SerializedProperty arrayProperty, System.String jsonToInsert, System.Int32 indexToInsert) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs:253)
UnityEngine.InputSystem.Editor.CopyPasteHelper.PasteBlocks (System.String transmission, System.Int32 indexToInsert, UnityEditor.SerializedProperty arrayToInsertInto) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs:217)
UnityEngine.InputSystem.Editor.CopyPasteHelper.PasteItems (System.String copyBufferString, System.Int32[] indicesToInsert, UnityEditor.SerializedProperty arrayToInsertInto) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs:206)
UnityEngine.InputSystem.Editor.CopyPasteHelper.PasteData (System.String copyBufferString, System.Int32[] indicesToInsert, UnityEditor.SerializedProperty arrayToInsertInto) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs:194)
UnityEngine.InputSystem.Editor.CopyPasteHelper.PasteActionsFromClipboard (UnityEngine.InputSystem.Editor.InputActionsEditorState state, System.Boolean addLast, System.Int32 mapIndex) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs:179)
UnityEngine.InputSystem.Editor.CopyPasteHelper.PasteActionsOrBindingsFromClipboard (UnityEngine.InputSystem.Editor.InputActionsEditorState state, System.Boolean addLast, System.Int32 mapIndex) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs:164)
UnityEngine.InputSystem.Editor.Commands+<>c__DisplayClass12_0.<PasteActionIntoActionMap>b__0 (UnityEngine.InputSystem.Editor.InputActionsEditorState& state) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Commands/Commands.cs:156)
UnityEngine.InputSystem.Editor.StateContainer.Dispatch (UnityEngine.InputSystem.Editor.Command command) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/StateContainer.cs:35)
UnityEngine.InputSystem.Editor.ViewBase`1[TViewState].Dispatch (UnityEngine.InputSystem.Editor.Command command) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ViewBase.cs:78)
UnityEngine.InputSystem.Editor.ActionMapsView.OnDroppedHandler (System.Int32 mapIndex) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ActionMapsView.cs:75)
UnityEngine.InputSystem.Editor.DropManipulator.OnDragPerformEvent (UnityEngine.UIElements.DragPerformEvent evt) (at ./Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/DropManipulator.cs:47)
UnityEngine.UIElements.EventCallbackFunctor`1[TEventType].Invoke (UnityEngine.UIElements.EventBase evt) (at D:/Gitrepo/unity/Modules/UIElements/Core/Events/EventCallback.cs:64)
UnityEngine.UIElements.EventCallbackRegistry+DynamicCallbackList.Invoke (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.BaseVisualElementPanel panel, UnityEngine.UIElements.VisualElement target) (at D:/Gitrepo/unity/Modules/UIElements/Core/Events/EventCallbackRegistry.cs:228)
UnityEngine.UIElements.EventDispatchUtilities.HandleEventAcrossPropagationPath (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.BaseVisualElementPanel panel, UnityEngine.UIElements.VisualElement target, System.Boolean isCapturingTarget) (at D:/Gitrepo/unity/Modules/UIElements/Core/Events/EventDispatchUtilities.cs:150)
UnityEngine.UIElements.EventDispatchUtilities.DispatchToCapturingElementOrElementUnderPointer (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.BaseVisualElementPanel panel, System.Int32 pointerId, UnityEngine.Vector2 position) (at D:/Gitrepo/unity/Modules/UIElements/Core/Events/EventDispatchUtilities.cs:578)
UnityEngine.UIElements.MouseEventBase`1[T].Dispatch (UnityEngine.UIElements.BaseVisualElementPanel panel) (at D:/Gitrepo/unity/Modules/UIElements/Core/Events/MouseEvents.cs:248)
UnityEngine.UIElements.EventDispatcher.ProcessEvent (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.BaseVisualElementPanel panel) (at D:/Gitrepo/unity/Modules/UIElements/Core/EventDispatcher.cs:336)
UnityEngine.UIElements.EventDispatcher.Dispatch (UnityEngine.UIElements.EventBase evt, UnityEngine.UIElements.BaseVisualElementPanel panel, UnityEngine.UIElements.DispatchMode dispatchMode) (at D:/Gitrepo/unity/Modules/UIElements/Core/EventDispatcher.cs:200)
UnityEngine.UIElements.BaseVisualElementPanel.SendEvent (UnityEngine.UIElements.EventBase e, UnityEngine.UIElements.DispatchMode dispatchMode) (at D:/Gitrepo/unity/Modules/UIElements/Core/Panel.cs:634)
UnityEngine.UIElements.UIElementsUtility.DoDispatch (UnityEngine.UIElements.BaseVisualElementPanel panel) (at D:/Gitrepo/unity/Modules/UIElements/Core/UIElementsUtility.cs:511)
UnityEngine.UIElements.UIElementsUtility.UnityEngine.UIElements.IUIElementsUtility.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr, System.Boolean& eventHandled) (at D:/Gitrepo/unity/Modules/UIElements/Core/UIElementsUtility.cs:230)
UnityEngine.UIElements.UIEventRegistration.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr) (at D:/Gitrepo/unity/Modules/UIElements/Core/UIElementsUtility.cs:76)
UnityEngine.UIElements.UIEventRegistration+<>c.<.cctor>b__1_2 (System.Int32 i, System.IntPtr ptr) (at D:/Gitrepo/unity/Modules/UIElements/Core/UIElementsUtility.cs:30)
UnityEngine.GUIUtility.ProcessEvent (System.Int32 instanceID, System.IntPtr nativeEventPtr, System.Boolean& result) (at D:/Gitrepo/unity/Modules/IMGUI/GUIUtility.cs:219)

ritamerkl · 2024-02-07T12:02:46Z

I also wonder why we do not rely on DragEnter and DragExit anywhere since these kind of events are typically what are used to update drag icons and/or evaluate whether drag source and drag targets are a valid combination to e.g. discard or allow drop.

We need to use the Drag and Drop events that are used internally in UITK for the reorderable logic. This is because we rely on the reorderable and append custom (e.g. drag between lists, restricting) implementation.

ritamerkl · 2024-02-07T12:39:28Z

I can drop composite bindings into non composite actions (and vice versa) -> after dragging them they jump back to where they were. Expected: block symbol on mouse is shown and you can't drag them there at all

I left a comment in the code where to block/discard a drag, currently it's handled with a forced reload of the treeview (that's sometimes a bit delayed what explains the jumping element). I would suggest to open a ticket for this as it's just about visuals.

Undo naming is not user friendly shown as "Modified Actions Maps.Array..." or "Modified x# properties"
Expected: "Moved Action/Binding x" does not need to go into specifics where it was moved, just has to be human readable of what action was done

I would suggest a ticket for this one as well as it's not very urgent neither.

…tion map

ritamerkl · 2024-02-07T13:12:40Z

Unknown steps to error so far but got it when loading the ProjectWideActions test scene (ignore if cause not clear from error message alone):

not clear to me, looks like an problem with the asset itself....

I can move an action to its own map and then it dissapears with this error (steps to repro: Create new custom input actions asset with just one action -> move that action to its own map -> it dissapears. Does not seem to repro if the map has more than 1 action):

fixed in 2175b70, was actually reproducable on develop as well, seems we missed that somehow

Pauliusd01 · 2024-02-08T12:13:07Z

Unity_2024-02-08_14-09-54.mp4

Seems like adding new processors/interactions to an action is broken after reordering it. Probably a selection issue as deselecting and re-selecting it fixes it

Pauliusd01

LGTM once bug with processors is fixed. Things checked can be found in the QA card: ISX-1692. Additional bugs filed for later: ISX-1871 and ISX-1872

ritamerkl · 2024-02-08T15:03:06Z

Seems like adding new processors/interactions to an action is broken after reordering it. Probably a selection issue as deselecting and re-selecting it fixes it

This is definitely a selection issue. I wonder if the changes on Håkans branch (continue with -> append the selection of the moved item) would fix this easy. Maybe worth checking after a merge.

Pauliusd01 · 2024-02-08T15:08:50Z

Seems like adding new processors/interactions to an action is broken after reordering it. Probably a selection issue as deselecting and re-selecting it fixes it

This is definitely a selection issue. I wonder if the changes on Håkans branch (continue with -> append the selection of the moved item) would fix this easy. Maybe worth checking after a merge.

Ok added new bug - ISX-1873

ekcoh

LGTM, added some minor comments which are more food-for-thought.

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Commands/Commands.cs

ekcoh · 2024-02-14T10:16:18Z

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ActionMapsView.cs


+        void OnDroppedHandler(int mapIndex)
+        {
+            Dispatch(Commands.CutActionsOrBindings());


I think this is likely fine but I would recommend when we do a sequence of commands like this that they are implemented as a single command instead but is the combination of the two underneath. Otherwise this would yield two Undo steps "Cut" and "Paste" which might be confusing for a Drop? Another aspect is that since dispatched, if CutActionsOrBindings() throws, the new state is not propagated and the Paste will execute on the previous state. I think I have seen this in more places so might be that we overlook all places at a later point for exception-safety and Undo convenience.

Right, I think there your implementation of Dispatch(..,continueWith) would be a great solution, I would wait until this is landed and make use of this.

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ActionsTreeView.cs

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/Selectors.cs

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/DropManipulator.cs

jfreire-unity and others added 30 commits August 14, 2023 13:11

DROP Enable UITK Asset Editor

326a442

WIP Add Drag And Drop UI functionality

c8f3ea5

SQUASH

5d4ae4c

Add drag and drop respective manipulators to deal with UI events

7c18777

WIP Add data manipulation logic based on UI drag and drop actions

083b4d3

fixed visualizations

68565d2

fixed reordering and drag between lists

5605382

fixed focusing MapListItems during drag

b843619

clean up branch

5a4b176

clean up drag/drop

d76cb23

implemented Drag & Drop between action tree view and action map list …

420bf9e

…view

Merge remote-tracking branch 'origin/develop' into isx-1141-uitk-drag…

30282e5

…-and-drop

cleared focus during drag/drop between lists

e9aaa22

fixed freeze after drag

d5500b7

drag and drop of action maps to reorder ActionMap list

9f1bf83

WIP drag & drop of actions, bindings & composites

949077e

Merge remote-tracking branch 'origin/develop' into isx-1141-uitk-drag…

8a18181

…-and-drop # Conflicts: # Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/ActionMapsView.cs

fix after merge

e5f7d54

added moving of bindings and move composite (WIP)

f9d2bea

implemented move part of composites and fixed move binding to empty a…

0a6a44c

…ctions

fixed moving bindings

4e70360

added discard drag on invalid

23fef72

fixed selection after drag & dragging bindings into actionMapView

7fd0078

refactor & fixed not allow drag of composites to action maps

aa60f59

fixed moving of composites

7f1dd90

fixed discard of drag between items

0e0f1e7

change action name for composites

1dbad3a

fixed bindings are moved down one index to far

7c3660b

added action item

1ea2ba9

graham-huws reviewed Feb 6, 2024

View reviewed changes

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/CopyPasteHelper.cs Outdated Show resolved Hide resolved

ritamerkl added 3 commits February 7, 2024 11:00

Merge remote-tracking branch 'origin/develop' into isx-1141-uitk-drag…

eeeb302

…-and-drop

refactor commands & fixed analyzer

0582d99

added comments and fixed accessibility

32a3a64

Pauliusd01 requested changes Feb 7, 2024

View reviewed changes

ritamerkl added 2 commits February 7, 2024 12:48

added comments & named parameters to ActionsTreeView

8031068

fixed formatting

30b7ed7

removed plus sign for dragging action to action map

de92b06

added safety check - fixed nullpointer for adding actions in empty ac…

2175b70

…tion map

ritamerkl requested review from ekcoh and graham-huws February 7, 2024 13:13

This comment was marked as outdated.

Sign in to view

ritamerkl added 2 commits February 8, 2024 12:32

fix picking of elements for dropping on ActionMapView

87faaef

fixed naming and enhanced break conditions for TreeViewDrag

7e507fb

Pauliusd01 approved these changes Feb 8, 2024

View reviewed changes

ekcoh approved these changes Feb 14, 2024

View reviewed changes

ritamerkl added 2 commits February 14, 2024 12:41

added safety assert & debug pasting bindings

b0f2c7d

changed dropManipulator to be internal

b060518

graham-huws reviewed Feb 14, 2024

View reviewed changes

Packages/com.unity.inputsystem/InputSystem/Editor/UITKAssetEditor/Views/DropManipulator.cs Show resolved Hide resolved

Merge branch 'develop' into isx-1141-uitk-drag-and-drop

a1816dc

graham-huws approved these changes Feb 14, 2024

View reviewed changes

ritamerkl merged commit 6db0cc5 into develop Feb 14, 2024

ritamerkl deleted the isx-1141-uitk-drag-and-drop branch February 14, 2024 17:14

UITK drag and drop (ISX-1141) #1831

UITK drag and drop (ISX-1141) #1831

Uh oh!

Conversation

ritamerkl commented Feb 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes made

Checklist

Uh oh!

Uh oh!

graham-huws commented Feb 6, 2024

Uh oh!

Pauliusd01 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ritamerkl commented Feb 7, 2024

Uh oh!

ritamerkl commented Feb 7, 2024

Uh oh!

ritamerkl commented Feb 7, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as outdated.

Pauliusd01 commented Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pauliusd01 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ritamerkl commented Feb 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Pauliusd01 commented Feb 8, 2024

Uh oh!

ekcoh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ekcoh Feb 14, 2024

Choose a reason for hiding this comment

Uh oh!

ritamerkl Feb 14, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ritamerkl commented Feb 5, 2024 •

edited

Loading

Pauliusd01 left a comment •

edited

Loading

ritamerkl commented Feb 7, 2024 •

edited

Loading

Pauliusd01 commented Feb 8, 2024 •

edited

Loading

Pauliusd01 left a comment •

edited

Loading

ritamerkl commented Feb 8, 2024 •

edited

Loading