godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.12k stars 69 forks source link

Improve tileset editor of godot 4.x #7177

Open jordanlis opened 1 year ago

jordanlis commented 1 year ago

Describe the project you are working on

2D plateformer

Describe the problem or limitation you are having in your project

Difficulty to use the new tileset/tilemap features in general in terms of UX/UI

Describe the feature / enhancement and how it helps to overcome the problem or limitation

Here are some UX improvement for the tileset management

1. Moving one tileset applies to all the others, but it shouldn't

tileset_pan_bug

❌Issue : you don't see the other tileset when you switch, depending on the size of the other tileset that you moved. So, it takes time to find again the other tileset

✔️Solution proposal : apply the coordinate after the move only to one tileset, and reset the view of the other when you switch

2. The action / tab bar should be reworked in order to paint quickly tiles on the tileset

Currently, here's the tab bar :

And you have to click on "configuration" to create tiles ?? image

❌Issue :

✔️Solution proposal : rework the toolbar and the display of the panels

Current : image

Proposal :

3. Improve the tilesets import

When I create a new tilemap --> New tileset --> Click on the "+" icon, I cannot add a resource to the tilsets. It create a weird thing, don't understand what it is. adding_resource

❌Issue : Not clear how to import a resource to a tileset, unless someone told you that it is drag'n drop.... You want to click on "+" but then don't understand how to add a texture

✔️Solution proposal :

4. Improve the atlas management

The UX for the atlas is not great

Current creation of an atlas atlas_management

Positioning the center positioning_center_atlas

❌Issues :

✔️Solution proposal :

5. Managing the ID of the tileset creates bug that can erase hours of work in just a finger snap

Currently, the ID of the tilesets are showned and it's not great to have that displayed. Plus, you cannot sort the tilesets however you want and if you have the bad idea to modify the ID... you lose your work

id_management

❌Issues :

✔️Solution proposal :

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

see above

If this enhancement will not be used often, can it be worked around with a few lines of script?

nope

Is there a reason why this should be core and not an add-on in the asset library?

you know why ;)

Zireael07 commented 1 year ago

Why is the ID even editable if modifying it loses the work?

KoBeWi commented 1 year ago

Solution proposal : apply the coordinate after the move only to one tileset, and reset the view of the other when you switch

You mean the view should always reset to origin or it should be remembered per source (tileset)?

When I create a new tilemap --> New tileset --> Click on the "+" icon, I cannot add a resource to the tilsets.

You can assign a texture in Configuration tab. Also https://github.com/godotengine/godot/pull/77962 improved the labels, so I think this problem might be solved.

Not possible to sort how you want your tilesets

You can find sorting options here: image You can sort by source name and rename your sources.

EDIT: I checked and changing ID does not erase any work. The tiles in TileMap store source IDs, so it makes your tiles invalid, but you can change the source back and everything is fine.

jordanlis commented 1 year ago

Solution proposal : apply the coordinate after the move only to one tileset, and reset the view of the other when you switch

You mean the view should always reset to origin or it should be remembered per source (tileset)?

That's how you prefer. Maybe it's better to remember the position for each tileset.

When I create a new tilemap --> New tileset --> Click on the "+" icon, I cannot add a resource to the tilsets.

You can assign a texture in Configuration tab. Also godotengine/godot#77962 improved the labels, so I think this problem might be solved.

I like the new wording in this proposal https://github.com/godotengine/godot/pull/77962 but the thing is that the feature is not correct if you don't drag n drop and use the + button --> You don't see how to link a texture once you've clicked on the +. We should just select a file on the hardrive, and then the thing you create should be created with this texture. Does it makes sense to you ? This is how I see this feature, because when I click on + I'm expected to import a tileset, not anything else. Or another proposal could be to click on +, a menu pops-up, and you select "import a texture". So you will still have these weird possibilites that maybe you want to keep when you click on the + button currently.

Not possible to sort how you want your tilesets

You can find sorting options here: image You can sort by source name and rename your sources.

EDIT: I checked and changing ID does not erase any work. The tiles in TileMap store source IDs, so it makes your tiles invalid, but you can change the source back and everything is fine.

Well, I didn't write correctly what I meant. What's happening here is that you can sort tileset with names or ID. So, when you see that intuitively you can change ID to sort your tileset, you playing with it. That's what happened for me. And THEN, you realize that all your tiles are missing. Good luck to find all the initial ID of all your tilesets when you have 10 for example. I tried to go back with ctrl + Z, but it wasn't enough to recover 2 hours of work. This is a really bad pattern. You shouldn't give the possibility to edit the ID, and the sorting should be independant of this technical ID. Maybe you can put 2 ID : an ID for sorting and an ID for the tiles reference ? I don't know what you prefer, from my side I'm just saying that we should look for a solution for this issue, and my proposal was to stop sorting tilesets with ID, but instead sort them with drag n drop. You move them how you want, more user friendly and you don't have the idea to tweak with these ID of the evil :)

master172 commented 1 year ago

also don't forget how adding collisions work currently making colliders which are quite big is verry hard due to how small the interface is (unless you have a big monitor)

and also how does the one way collision in tiles work, I just can't figure it out which direction does it collides in is that direction editable

also when trying to create collisions for a tile with some parts invisible while trying to make a grid based is extremely hard there is only 1 pixel snapping how do i know if i have went over the tile size in the collider

NathanLovato commented 1 year ago

This is an excellent UX proposal. I generally support all the suggestions. Thanks for taking the time to write a detailed proposal like this, with suggestions that can be added in incremental changes! If you have more problems and suggestions for the tilemap side, a similar proposal would be much appreciated.

Zireael07 commented 1 year ago

The tiles in TileMap store source IDs, so it makes your tiles invalid, but you can change the source back and everything is fine.

If you mean you have to set back the original ID, then again, why it's editable in first place?

If you mean you can change tiles' source, then it should be auto-done

KoBeWi commented 1 year ago

We should just select a file on the hardrive, and then the thing you create should be created with this texture. Does it makes sense to you ?

It can't be done directly with the +, because there are other source types than texture (for now only scenes). Two options I can think of is to automatically switching to Configuration tab (so the texture field is visible) or showing File Dialog right after you select Atlas Source.

Good luck to find all the initial ID of all your tilesets when you have 10 for example.

You could try looking into the tscn file, but yeah, it's rather tedious. I think the ID field can be hidden. It's an advanced feature and not really useful.

stop sorting tilesets with ID, but instead sort them with drag n drop.

The sources would need an additional, internal value just for the purpose of sorting. But it's doable I guess. There was a related proposal recently https://github.com/godotengine/godot-proposals/issues/7070, but the user found source naming sufficient.

I just can't figure it out which direction does it collides

It's fixed on up direction.

trying to make a grid based is extremely hard there is only 1 pixel snapping

This also has changed in 4.1.

I'd suggest making feedback based on the 4.1 version, as the tile editor has improved a bit. There are more improvements coming in 4.2.

groud commented 1 year ago

For me: 1) Makes sense. I guess we could store some editor metadata on the TileSetAtlasSource itself, that could work. :thinking:

2) Why not, but it does not explain where to put the Atlas configuration exactly. Also, I am not sure it improve things that much. Right now it's clear that you have a "main tool" and "configure that tool" organization. The "Eraser" would collide with an eraser in the "Property painting" things. Also, you are not configuring the TileSet here, you are configuring a TileSetAtlasSource which would be confusing. The idea could be refined though, if creating tiles in "configuration" is confusing, I agree it might make sense to move that to another tool. I would probably allow deleting tiles in the same tool though, instead of needing another one.

3) Creating a new atlas should switch to the atlas configuration right away. But as Kobewi mentionned, your solution is not possible as you need to be able to create scene-based sources.

4) You don't have to drag and drop the boundaries to draw a big tile, ctrl+shift is a shortcut for that. being able to define the center on the atlas itself would be great but it is difficult and time-consuming to implement. But I agree it should be done at some point. Contributions welcome I guess!

5) Those IDs need to be accessible for people who know what they are doing, because it is an acceptable use case to want to be able to completely switch an atlas source for another. But yeah, IDs are internal and should not be modified unless you know what you are doing. Regarding the sorting, I just did not implement sorting at first, and, when we did, we did not change the default sorting. A custom order could be implemented I believe, but it's not so simple to do and was not a priority to me (contribution welcome though!). Regarding why those IDs are visible in the list, it's was to help users know a tile's identifier more easily, mainly for procedural stuff. But I guess that, if it bothers people, it could go away.

jordanlis commented 1 year ago

Hi,

thanks to everyone for your answers (I won't answer to everything for the while, since my points have been made)

But here are some comments :

  1. At least for the import of an atlas, you should open directly the file importer window, as kowi mentioned. Is it an acceptable solution ?

  2. I don't understand what you said @groud about positioning the center. You said "You don't have to drag and drop the boundaries to draw big tile" but yes I must do that in order to draw big tiles no ? I'll test it tonight with your shortcut. But if it's possible to do it in a single shortcut, why not tell the user for example ? i'll edit my message once the test done. EDIT : Tested, still I don't see what you mean. This center principle should be refined. EDIT 2 : SHIT ! I see what you meant ! This is amazing, and I didn't know at all that it was possible ! We need to make it easier to discover for this feature, no ? For example, like the rect square in the tilemap part --> When you select the pen tool and press ctrl + maj, it activates the rect tool, and when you release it reactivates only the pen tool

@KoBeWi for the record, I'm now using 4.1, and about the boundaries, it is worse now : we don't have the arrows anymore to place the center. I think what should be done is that when you edit the boundaries for a big tile, it should be placed not on the center, but on the upper left (or any other corner) --> with that, you're ensuring that the simplest use case is filled by default, and if anyone wants to move this "center", they could. (base use case = placing a tile on the grid like any other tile, whereas now if the "center" is in the real center, your big tile is going outside or on half tiles depending on the size of the big tile).

Another thing that should be mentioned is that editing the boundaries is a bit weird, since you have to move the boundaries to the second next tile to update the boundaries, whereas I think you should make it like this : boundaries are updated once the cursor has been moved to the adjacent tile, and not the one after. I should share a gif maybe to explain more clearly ?

EDIT : Here's a video of delimiting the boundaries boundaries It's clearly weird that when you drag, you don't see the boundaries move first. Then you realize that it's needed to go further on the second next tile.

Another weird stuff is that

  1. Resizing the boundaries doesn't erase the other tiles. So you must ensure that all the other tiles have been deleted
  2. Resizing boundaries doesn't work correctly when the mouse cursor is hovering others tiles. So, most of the times your brain doesn't really understand why it doesn't work. Since this operation is supposed to be done a lot, it needs some refinement or game devs will become crazy :) boundaries_blocked
KoBeWi commented 1 year ago

we don't have the arrows anymore to place the center.

Oh, this is a bug 👀

EDIT: https://github.com/godotengine/godot/pull/79021

jordanlis commented 1 year ago

This is an excellent UX proposal. I generally support all the suggestions. Thanks for taking the time to write a detailed proposal like this, with suggestions that can be added in incremental changes! If you have more problems and suggestions for the tilemap side, a similar proposal would be much appreciated.

I'll do another proposal for the tilemap, but I think I have less feedback (but still some :) )

groud commented 1 year ago

At least for the import of an atlas, you should open directly the file importer window, as kowi mentioned. Is it an acceptable solution ?

Ah, like, open a file selector to set the texture right away ? I guess that's a good idea!

EDIT 2 : SHIT ! I see what you meant ! This is amazing, and I didn't know at all that it was possible ! We need to make it easier to discover for this feature, no ? For example, like the rect square in the tilemap part --> When you select the pen tool and press ctrl + maj, it activates the rect tool, and when you release it reactivates only the pen tool

Yeah, making it more discoverable would be great, but as always, it is difficult to do. Usually, you would have a dedicated tool to do the same thing and you would use the tool's button tooltip to indicate the shortcut exists, but that's some work and require yet another mode or "subtool". But any idea welcome I guess.

Another thing that should be mentioned is that editing the boundaries is a bit weird, since you have to move the boundaries to the second next tile to update the boundaries, whereas I think you should make it like this : boundaries are updated once the cursor has been moved to the adjacent tile, and not the one after. I should share a gif maybe to explain more clearly ?

Maybe the boundaries could snap to the closest boundary, if that's more expected. But theoretically, the cursor should be updated to a a "resize" shape at least, that should be a good starting point.

Resizing the boundaries doesn't erase the other tiles. So you must ensure that all the other tiles have been deleted

This is expected, deleting a tile could lead to loss of data so I prefer that users purposefully delete the tiles manually.

Resizing boundaries doesn't work correctly when the mouse cursor is hovering others tiles. So, most of the times your brain doesn't really understand why it doesn't work. Since this operation is supposed to be done a lot, it needs some refinement or game devs will become crazy :)

Yeah, that could be improved so that the tile takes as much space as possible according the dragged rect, not try to fit it exactly. That's some work but it should be doable.

jordanlis commented 1 year ago

At least for the import of an atlas, you should open directly the file importer window, as kowi mentioned. Is it an acceptable solution ?

Ah, like, open a file selector to set the texture right away ? I guess that's a good idea!

Yep

EDIT 2 : SHIT ! I see what you meant ! This is amazing, and I didn't know at all that it was possible ! We need to make it easier to discover for this feature, no ? For example, like the rect square in the tilemap part --> When you select the pen tool and press ctrl + maj, it activates the rect tool, and when you release it reactivates only the pen tool

Yeah, making it more discoverable would be great, but as always, it is difficult to do. Usually, you would have a dedicated tool to do the same thing and you would use the tool's button tooltip to indicate the shortcut exists, but that's some work and require yet another mode or "subtool". But any idea welcome I guess.

Yep, exactly. As I told you, using ctrl + maj activates the square tool when the pen is selected, and when you release the keys, it deselects it again. I think you're right : we should add another tool for "big tiles" to make it discoverable without knowing first the shortcut.

Another thing that should be mentioned is that editing the boundaries is a bit weird, since you have to move the boundaries to the second next tile to update the boundaries, whereas I think you should make it like this : boundaries are updated once the cursor has been moved to the adjacent tile, and not the one after. I should share a gif maybe to explain more clearly ?

Maybe the boundaries could snap to the closest boundary, if that's more expected. But theoretically, the cursor should be updated to a a "resize" shape at least, that should be a good starting point.

I think it's more expected like you mentionned : the closest border. Maybe it will be smoother.

Resizing the boundaries doesn't erase the other tiles. So you must ensure that all the other tiles have been deleted

This is expected, deleting a tile could lead to loss of data so I prefer that users purposefully delete the tiles manually.

I understand that. This is why I was telling that you can resize the big tile on top of existing tiles, but it will be deleted only if you release the mouse button. It doesn't erase it once you've expanded the boundaries on top of the other tiles. Instead, you just visualize in some way that if you release the mouse button, it will be deleted. But I think this is not the most important part of the proposal, since you can just load the tileset without creating automatically the tiles.

Resizing boundaries doesn't work correctly when the mouse cursor is hovering others tiles. So, most of the times your brain doesn't really understand why it doesn't work. Since this operation is supposed to be done a lot, it needs some refinement or game devs will become crazy :)

Yeah, that could be improved so that the tile takes as much space as possible according the dragged rect, not try to fit it exactly. That's some work but it should be doable.

In the end, the most important part about my initial proposal number 4 "Atlas management" would be to make the feature ctrl + maj more discoverable. Even if you don't modify all the other proposal about this point 4, it will remove the main painpoint about creating big tiles according to me

KoBeWi commented 1 year ago

Current status:

  1. I tried to implement remembering zoom and pan per atlas source. It didn't work reliably. Also it can't be stored as metadata, because it makes the TileSet modified (which is undesirable when using TileMap tab). I think it's not worth implementing tbh.
  2. Requires changing the whole structure of the editor, too much work. Maybe one day.
  3. Resolved by https://github.com/godotengine/godot/pull/79285
  4. Directly resolved by https://github.com/godotengine/godot/pull/79899, but see also https://github.com/godotengine/godot/pull/79837 and https://github.com/godotengine/godot/pull/79904
  5. Resolved by https://github.com/godotengine/godot/pull/79419

I'm not going to touch 1. anymore, but here's my unfinished attempt if anyone is interested: https://github.com/KoBeWi/godot/commit/5e8ce9621f608f97ec4ef40f18c48c22ca1b4d68 As for 2, I don't know.

Other points are resolved (after the remaining PRs get merged at least).

jordanlis commented 1 year ago

I think you did a great job! Thanks for the improvement. I must say that not everything is exactly as I was imagining it but the solutions you came up with are great. Especially the help label for creating big tiles

For me, 1 and 2 are really important, but I understand that they are harder to do so it's a good first step.

Lunarexxy commented 1 year ago

If I may suggest one more minor improvement, since it's fairly related to improving the UX of the tileset editor? Currently, all of the properties shown in the Atlas editor have "No description".

image

However, all of these are described quite excellently in the guide on using tilesets.

image

Perhaps those could be added in as property descriptions? I tried to do so myself, but I wasn't sure how to go about it, since I'm not used to C++ or the Godot codebase at all...

(Also, sorry if this counts as hijacking the orginal proposal. I just figured it was a fairly minor addition and fit in well here.)

KoBeWi commented 1 year ago

This is tricky, because property descriptions are fetched from the documentation. We don't have a system for arbitrary property descriptions. But it's a good idea. Maybe it can be implemented somehow.

Calinou commented 1 year ago

This is tricky, because property descriptions are fetched from the documentation. We don't have a system for arbitrary property descriptions. But it's a good idea. Maybe it can be implemented somehow.

This likely needs similar hacks to https://github.com/godotengine/godot/pull/49524 or https://github.com/godotengine/godot/pull/48548.