Skip to content
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

733 - Add keyboard navigation in tile palette #764

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

cowmanjoe
Copy link
Contributor

@cowmanjoe cowmanjoe commented Nov 12, 2019

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.

@ilexp ilexp added this to the General milestone Nov 16, 2019
@ilexp ilexp added Feature It doesn't exist yet, but I want it Tilemaps Area: Tilemaps core / editor plugins Usability Related to API and UI usability labels Nov 16, 2019
@ilexp
Copy link
Member

ilexp commented Nov 16, 2019

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.

@cowmanjoe
Copy link
Contributor Author

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
Copy link
Contributor Author

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

@ilexp
Copy link
Member

ilexp commented Apr 26, 2020

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.

@cowmanjoe
Copy link
Contributor Author

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

@Barsonax
Copy link
Member

@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.

@cowmanjoe cowmanjoe force-pushed the feature/733 branch 2 times, most recently from d8e336f to 51813f5 Compare May 1, 2020 01:06
@cowmanjoe
Copy link
Contributor Author

cowmanjoe commented May 1, 2020

@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.

@Barsonax
Copy link
Member

Barsonax commented May 1, 2020

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)?

@cowmanjoe
Copy link
Contributor Author

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

@Barsonax
Copy link
Member

Barsonax commented May 11, 2020

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.

@cowmanjoe
Copy link
Contributor Author

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?

@Barsonax
Copy link
Member

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.

@cowmanjoe
Copy link
Contributor Author

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?

@Barsonax
Copy link
Member

Barsonax commented May 13, 2020

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"

When the tileset had multiple columns, the coordinates for the single column configuration was being used.
@cowmanjoe
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature It doesn't exist yet, but I want it Tilemaps Area: Tilemaps core / editor plugins Usability Related to API and UI usability
Development

Successfully merging this pull request may close these issues.

3 participants