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

Terrains UI issues #3015

Closed
eishiya opened this issue Mar 28, 2021 · 2 comments
Closed

Terrains UI issues #3015

eishiya opened this issue Mar 28, 2021 · 2 comments
Labels
bug Broken behavior. usability Generally about making something more intuitive or efficient.

Comments

@eishiya
Copy link
Contributor

eishiya commented Mar 28, 2021

This is several issues but they're all fairly minor and all related to working with Terrains in 1.5. If you'd prefer that I split them up into several issues, I can.

  1. When editing a text/number property in the Tileset Editor while in Terrain edit mode, if the cursor leaves the properties panel and moves over a tile (triggering the terrain placement preview), the text/number field loses edit focus. This is inconsistent with the usual behaviour where merely moving the mouse doesn't change focus.

  2. In the Map Editor, when switching from the Terrain Brush to another tool and then going back to terrains, it's intuitive to click the terrain you want and then paint, and this is the current behaviour when you click a new terrain. However, if that terrain was already selected, Tiled does not go back to the Terrain Brush and keeps the previous tool selected. I think clicking on a terrain should activate the Terrain Brush, even if the terrain was already selected.

  3. When selecting a different terrain set but not selecting a terrain from it, Tiled shows a non-functional preview of the last previewed terrain blob on the side of the editor (the location is probably just due to that being where the preview was last generated). This preview doesn't follow the cursor (probably because there's no terrain to update it with), and clicking to "place" it does nothing. Seems like in this situation, the cursor being in the editor causes the preview to be shown, but not to be updated. Since the Erase Terrain button is highlighted after switching terrains, it looks like that's the intended behaviour? But the erase terrain mode is not actually activated.
    tiledterrainglitch

  4. It's possible for the Terrains list in the Terrain Sets panel to be collapsed. Does it ever make sense for the terrains list to be collapsed while the Terrain Sets panel is in use? A collapsed terrains list means users can't even use the Terrain Brush until they expand it and select a terrain, since the Erase Terrain mode is selected by default when they select a Terrain Set. When it is collapsed, it's also hard to see that it's even there. I think the terrains list should be resizable but not collapsible. This goes for both the Map Editor and the Tileset Editor. The terrains list is just too important for use of the feature, and causes a lot of confusion when it's unintentionally collapsed and the user isn't already aware that it should be there.

  5. On a related note to (4), it's entirely possible for the user to have the entire Terrain Sets panel hidden (whether by default or by accident) while trying to use the Terrain Brush, causing the tool to do nothing. Users not already familiar with how the tool works may not know that they need to switch to or enable this panel. I don't have a good solution for this one. Opening the panel for them or raising a Warning punishes quick misclicks with possibly unwanted clutter and warnings. A modal warnings with the option to open the panel seems like overkill. Perhaps a solution could be to open the panel, but to also disable the tool when none of the map's tilesets have terrains, to prevent misclicks? People who do have terrains are likely to have the panel enabled already anyway, or to want it enabled.

@bjorn bjorn added bug Broken behavior. usability Generally about making something more intuitive or efficient. labels Mar 29, 2021
bjorn added a commit that referenced this issue Apr 14, 2021
Enter event shouldn't change focus. Ideally, we'd find another way to
make these shortcuts work while focus is elsewhere.

Fixes part of issue #3015.
bjorn added a commit that referenced this issue Apr 14, 2021
…cted

When clicking on a terrain the expectation is that it would always
activate the Terrain Brush, but it didn't do this when the terrain was
already selected.

This used to work in the Terrains view in Tiled 1.4, so technically a
regression.

Fixes part of issue #3015.
bjorn added a commit that referenced this issue Apr 14, 2021
It's never useful to be able to collapse this view, though it can lead
to confusion.

Part of issue #3015.
@bjorn
Copy link
Member

bjorn commented Apr 14, 2021

  1. When editing a text/number property in the Tileset Editor while in Terrain edit mode, if the cursor leaves the properties panel and moves over a tile (triggering the terrain placement preview), the text/number field loses edit focus. This is inconsistent with the usual behaviour where merely moving the mouse doesn't change focus.

Hmm, this was introduced in #1638, probably to make the keyboard shortcuts work. For now I've just removed the focus change (bd60946), but it would be nice to still have those shortcuts work even if focus isn't in the tileset view. The shortcuts also turned out to be non-functional though, but I fixed that in 23da913.

  1. ... However, if that terrain was already selected, Tiled does not go back to the Terrain Brush and keeps the previous tool selected. I think clicking on a terrain should activate the Terrain Brush, even if the terrain was already selected.

Fixed in 68bd2a2.

  1. It's possible for the Terrains list in the Terrain Sets panel to be collapsed. Does it ever make sense for the terrains list to be collapsed while the Terrain Sets panel is in use?

I don't think so, made not collapsible in ab6541c.

Thanks for the feedback! I'll have a look at 3 and 5 tomorrow.

bjorn added a commit that referenced this issue Apr 15, 2021
This is more useful than always selecting the eraser.

Also avoid a potential crash after deleting the terrain that was currently
used by the Terrain Brush.

Part of issue #3015
bjorn added a commit that referenced this issue Apr 15, 2021
The empty Terrain Sets view now tries to be a little more helpful than
just being empty.

I tried automatically showing/raising the Terrain Sets view when the
Terrain Brush is selected, but it didn't feel right. Could work if it
also automatically goes back to its previous state when the tool is
deselected, but it's not obvious to me how that could be implemented.

Also fixed a crash that could happen when removing a tileset, because
that could delete a still referenced WangColorModel.

Part of issue #3015
@bjorn
Copy link
Member

bjorn commented Apr 15, 2021

I've improved upon 3 by automatically selecting the first terrain (07689a5).

Regarding 5, as discussed on Discord I didn't manage to find a nicer solution to automatically hiding/showing this view for now. In 07a38d7 I did make the Terrain Sets view a little more helpful in case there isn't any tileset with a terrain set:

image

Thanks again for your feedback! I think we can close this issue now. :-)

@bjorn bjorn closed this as completed Apr 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Broken behavior. usability Generally about making something more intuitive or efficient.
Projects
Archived in project
Development

No branches or pull requests

2 participants