AdamsLair / duality

a 2D Game Development Framework
https://adamslair.github.io/duality
MIT License
1.4k stars 289 forks source link

733 - Add keyboard navigation in tile palette #764

Open cowmanjoe opened 4 years ago

cowmanjoe commented 4 years ago

The tile palette active selection can now be:

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 commented 4 years ago

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.

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

cowmanjoe commented 4 years ago

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 commented 4 years ago

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

ilexp commented 4 years ago

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 commented 4 years ago

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

Barsonax commented 4 years ago

@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 commented 4 years ago

@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 commented 4 years ago

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 commented 4 years ago

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

Barsonax commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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"
cowmanjoe commented 4 years ago

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