duality icon indicating copy to clipboard operation
duality copied to clipboard

733 - Add keyboard navigation in tile palette

Open cowmanjoe opened this issue 6 years ago • 15 comments

The tile palette active selection can now be:

  • moved with the shift+arrow keys,
  • expanded on a given side using alt+arrow keys, and
  • shrunk on a given side using shift+alt+arrow keys. This functionality is also available when the tilemap editor is in focus.

This behaviour is slightly different from what was mentioned in ticket #733, but I thought this made more sense. The reason I didn't use the arrow keys by themselves for moving the selection as specified was that the arrow keys are used for a different purpose already in the tilemap editor view.

cowmanjoe avatar Nov 12 '19 01:11 cowmanjoe

First of all, good catch with the already used arrow keys. Since they're already used for switching tilemap drawing layers, we can't use them for changing tile source selection as well.

Since this is an editor feature I did some usability testing on this first before starting to look into the code, and there are some issues that should be addressed before proceeding.

  • In a tile drawing workflow, one hand will always be occupied with the mouse, so all keyboard input that augments mouse operation needs to be comfortably expressible with the remaining (usually left) hand.
  • It works for Shift + Arrows on a standard keyboard, since the right shift key is next to them. It doesn't work for the other combos, since it's not really possible to press Alt + Arrows, or even Alt + Shift + Arrows comfortably with one hand.
  • The different combinations for shrinking and expanding and moving are a bit confusing, because it's three different key combos for one feature, and none of it matches standard keys.
  • The adjusted key binding for the Tilemap Editor feels weird in the context of the Tile Palette, where the arrow keys don't do anything and there is no apparent reason to press Shift in addition.

To address these issues, I'd suggest the following:

  • Let's get the interaction straight in the Tile Palette first, where we're less constrained by other key bindings.
  • Use minimal key binding for Tile Palette, avoiding modifier keys where possible. Also reduce the binding to just two: Moving and Adjusting.
    • Arrows move the selection by one tile in the pressed direction.
    • Shift + Arrows adjust the selection.
      • The first arrow press in each axis (horizontal or vertical) selects which side on that axis (left / right, top / bottom) is moved, so the first press will always be an expansion. All subsequent arrow presses move that side while keeping the other side fixed, so both shrinking and expanding is possible. Releasing the Shift key resets the side lock to its initial undefined state.
      • Example: Expanding bottom right
        • Shift + Right: Select right side and expand by one to the right.
        • Shift + Down: Select bottom side and expand by one downwards.
      • Example: Shrinking bottom right
        • Shift + Right: Select right side and expand by one to the right.
        • Shift + Left: Shrink right side by one to the left. [Back to original size]
        • Shift + Left: Shrink right side by one to the left.
        • Shift + Down: Select bottom side and expand by one downwards.
        • Shift + Up: Shrink bottom side by one upwards. [Back to original size]
        • Shift + Up: Shrink bottom side by one upwards.
      • Example: Adjusting top left
        • Shift + Left: Select left side and expand by one to the left.
        • Shift + Up: Select top side and expand by one upwards.
        • Shift + Down: Shrink top side by one downwards. [Back to original size]
        • Shift + Down: Shrink top side by one downwards.
    • Not perfect, but it somewhat mirrors regular list selection behavior that users know from other software.
  • Figure out a way which keys to use in the Tilemap Editor to express the same behavior.
    • Changing the key bindings for layer selection would be an option as well, would need to think of an alternative for that too.
  • The Tile Palette should auto-scroll if the new selection would otherwise leave the current view.

ilexp avatar Nov 16 '19 09:11 ilexp

Thanks for the very thorough comment! I fully agree. What you're describing sounds more natural UX-wise, even though it could mean more key strokes for certain actions. I think we won't be needing to adjust the selection very often anyway so ease of use is more important than key stroke efficiency. I will start working on this within the coming days.

cowmanjoe avatar Nov 18 '19 06:11 cowmanjoe

For the controls, pretty much did as @ilexp commented earlier. As for the conflicting controls with the tilemap editor, I changed:

  • active tilemap switching to page up and page down instead of up and down, since I felt these would be less frequently used controls
  • deselect game object to be escape instead of left and right since these controls do the same thing anyway and escape seems like a more fitting key for this function

cowmanjoe avatar Dec 29 '19 10:12 cowmanjoe

Sorry for not responding earlier - for time reasons, I wasn't able to continue looking into this as needed.

@Barsonax @SirePi Can you guys pick up this PR? Feel free to ping me if needed down the line.

ilexp avatar Apr 26 '20 13:04 ilexp

Also happy to keep on working on this if there are any more comments

cowmanjoe avatar Apr 27 '20 05:04 cowmanjoe

@cowmanjoe There have been quite a bit of changes in master recently. Can you rebase your branch on master so its up to date again? The changes are mostly in the csproj files so theres a good chance you won't have conflicts.

Rick-van-Dam avatar Apr 27 '20 13:04 Rick-van-Dam

@Barsonax I've rebased with master and fixed a problem where TilemapCamViewState was not getting the selected area change updates. I modified a csproj file for FlapOrDie because the build was failing without it. I'm not sure if this was the correct thing to do.

cowmanjoe avatar May 01 '20 01:05 cowmanjoe

I've rebased with master and fixed a problem where TilemapCamViewState was not getting the selected area change updates. I modified a csproj file for FlapOrDie because the build was failing without it. I'm not sure if this was the correct thing to do.

That doesn't seem to be right. I think visual studio was confused maybe because the cache was not cleared? Can you try again without modifying the csproj and first cleaning everything (git clean -fdx can be handy here)?

Rick-van-Dam avatar May 01 '20 06:05 Rick-van-Dam

Ah, thanks! I think I only did git clean -fx before. Build was successful without the change to the FlapOrDie csproj.

cowmanjoe avatar May 01 '20 06:05 cowmanjoe

Seems there is a small bug somewhere. First time I tried to test the arrow navigation I got this exception:

EditorError:   ArgumentException: Height needs to be greater than or equal to zero.
Parameter name: newHeight
CallStack:
   at Duality.Grid`1.ResizeClear(Int32 newWidth, Int32 newHeight) in C:\git\duality\Source\Core\Duality\Utility\Grid.cs:line 425
   at Duality.Editor.Plugins.Tilemaps.SourcePaletteTilesetView.UpdateSelectedTiles() in C:\git\duality\Source\Plugins\Tilemaps\Editor\Modules\SourcePaletteTilesetView.cs:line 444
   at Duality.Editor.Plugins.Tilemaps.SourcePaletteTilesetView.set_SelectedArea(Rectangle value) in C:\git\duality\Source\Plugins\Tilemaps\Editor\Modules\SourcePaletteTilesetView.cs:line 78
   at Duality.Editor.Plugins.Tilemaps.SourcePaletteTilesetView.ExpandSelectedArea(Int32 diffX, Int32 diffY) in C:\git\duality\Source\Plugins\Tilemaps\Editor\Modules\SourcePaletteTilesetView.cs:line 524
   at Duality.Editor.Plugins.Tilemaps.SourcePaletteTilesetView.OnKeyDown(KeyEventArgs e) in C:\git\duality\Source\Plugins\Tilemaps\Editor\Modules\SourcePaletteTilesetView.cs:line 376
   at System.Windows.Forms.Control.ProcessKeyEventArgs(Message& m)
   at System.Windows.Forms.Control.WmKeyChar(Message& m)
   at System.Windows.Forms.Control.WndProc(Message& m)
   at System.Windows.Forms.NativeWindow.Callback(IntPtr hWnd, Int32 msg, IntPtr wparam, IntPtr lparam)

The selection also seems to jump sometimes depending on what you have selected.

After some fiddling around I managed to reproduce this 100% of the time with these steps. Seems the selected tile in step 1 is key to reproducing this so you might have to search a bit.

Steps to reproduce:

  1. Select tile (about at half the width of the tileset?)
  2. Press shift + arrow
  3. Profit

I used the TilemapsSample for this.

EDIT: we have changed some csprojs again so you might want to rebase it again. Should be the last time.

Rick-van-Dam avatar May 11 '20 07:05 Rick-van-Dam

Oh that's strange! I'm unable to reproduce the bug, but I am also not sure how to run the sample projects. I have just been adding a tilemap to the initial empty project and testing that way. Is there any documentation on how to open the sample projects in the editor or am I missing something easy?

cowmanjoe avatar May 12 '20 20:05 cowmanjoe

The way to run the sample projects changed very recently. There is now a SampleRunner project. You have to modify the path in launchsettings json to point to the sample you wanna run and then start this project.

Currently on mobile so cannot give more detailed instructions but maybe this is enough for you to figure it out.

Rick-van-Dam avatar May 12 '20 20:05 Rick-van-Dam

I'm still not able to reproduce the bug you are experiencing. Perhaps you can give a bit more detail? Here's a video of me running the Tilemaps sample. I use shift+arrow for each direction on tiles from various points in the middle of the tileset. Is this what you did to reproduce or am I missing something?

cowmanjoe avatar May 12 '20 21:05 cowmanjoe

Try resizing the tile pallette so it reordens the tiles. Might have to do with the bug.

EDIT: I select this tile and then I press shift arrow key to get the error image

Might have to do with the reordering of tiles causing it to be not a rectangle anymore?

EDIT: to run the tilemaps sample modify launchSettings.json in the SampleRunner project so that the working directory points to the tilemaps sample:

"workingDirectory": ".\\..\\Tilemaps\\Content"

Rick-van-Dam avatar May 13 '20 05:05 Rick-van-Dam

Ah yes, good catch! The problem did have to do with the multicolumn expansion. The problem should be fixed now.

cowmanjoe avatar May 13 '20 22:05 cowmanjoe