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

Render/Physics layer editor with up to 32 values #2770

Closed
pouleyKetchoupp opened this issue May 25, 2021 · 41 comments
Closed

Render/Physics layer editor with up to 32 values #2770

pouleyKetchoupp opened this issue May 25, 2021 · 41 comments

Comments

@pouleyKetchoupp
Copy link

Describe the project you are working on

Godot Editor

Describe the problem or limitation you are having in your project

Users have recurring issues with seeing only 20 editable layers in the editor, while up to 32 layers are used in the engine and can be set by script.
See godotengine/godot#45536

On top of that, a 3.2 bug has caused scenes to be saved with extra collision layer bits in the hidden range, which added to the confusion.
See godotengine/godot#41281

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

The proposed solution is to allow the editor to show the whole range of usable layers, as an option to avoid cluttering the inspector UI.

Instead of the current 10x2 grid, it would show a 8x2 grid by default, extendable to 8x4.

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

Mockup for layer grid in the inspector
Note: this is just a quick mockup, the layout can be improved.

Default:
image
Expanded:
image
Clicking on the ... icon for each section would show a popup with the corresponding layer names (1-16 or 17-32).

Project settings

All the sections in Layer Names would also show all the 32 available layers.

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

It can, but this change can improve editor usability.

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

Layers editor and project settings are part of the core editor.

@pouleyKetchoupp
Copy link
Author

CC @godotengine/usability

@dalexeev
Copy link
Member

I think the size of the squares can be reduced a bit. And even if we do not reduce it, it still fits more than it was (24 instead of 20). In any case, not everyone uses all layers, so it's okay that layers with large numbers are hidden. Those who need them can simply stretch the inspector.

@EricEzaM
Copy link

EricEzaM commented May 26, 2021

Can we make it dynamic in blocks of 2x2 or 2x4 so it takes up the least amount of vertical space possible?

@dalexeev
Copy link
Member

Can we make it dynamic in blocks of 2x2 or 2x4

I don’t quite understand what you mean and who the question is addressed to, but if you mean something like that, option B is better.

@EricEzaM
Copy link

EricEzaM commented May 26, 2021

Directed as an open question.

But now I think about it more, it probably isn't a very good idea. A button to expand from 16 to 32 cells would probably be best.

Maybe if the inspector is wide enough it would render as 2x16, otherwise 4x8 (that's sort of what I mean by dynamic)

@groud
Copy link
Member

groud commented May 26, 2021

The inspector have to keep a compact width for Godot to be usable on small screen. So I wouldn't expand this horizontally.
I think the original proposal is the best one. As they are not used often, I don't think the 16+ layers need to be visible all the time. So an expand/collapse button is the best solution IMHO.

@EricEzaM
Copy link

EricEzaM commented May 26, 2021

This is my inspector on a 1080p screen. If it was able to detect when it could render as 2x16, I would prefer that as it would be less vertical space used.

Screenshot_20210526-194358_Chrome.jpg

There is more than enough space. I agree that the latter 16 layers could be toggled

Sorry for close, fat fingered it on mobile

@EricEzaM EricEzaM reopened this May 26, 2021
@groud
Copy link
Member

groud commented May 26, 2021

Well, even on an 1080p, you can still expand the editor horizontally to have a lot of space available like you have here. But what I mean is that on a default reasonable size, the inspector should not be expanded horizontally by the fact we decided to go for a fully horizontal layout.

Also, the problem with a fully-horizontal layout is that it becomes unreadable without separators between them, which would make it take even more space.

If we manage to find a way to have a readable 2x16 layout, while not having it causing a width increase of the inspector, I am ok for this solution. But otherwise, I still think an expand/collapse button is a quite reasonable compromise.

@dalexeev
Copy link
Member

@groud
Copy link
Member

groud commented May 26, 2021

@dalexeev the checkboxes are already not that big, I don't think making them smaller is a good solution.
Moving the three dots next to the label makes sense though.

@dalexeev
Copy link
Member

@groud
Copy link
Member

groud commented May 26, 2021

For reference, this is the inspector for KinematicBody2D where I set the width so that most of the properties are readable (so it's compact but still very usable):
2021-05-26-132149_275x1455_scrot
So, well, it kind of makes my point, as there's not enough room to double the width of the mask/layer properties without increasing the width of the inspector.

@dalexeev
Copy link
Member

#808080 All Godot users
#ff0000 Narrow screen users
#0000ff Users who want all 32 layers
#ff00ff Users who may be* affected by this issue

(* – If 32 pixels wide will be of fundamental importance to them.)

@groud
Copy link
Member

groud commented May 26, 2021

@dalexeev I mean, I am not strictly against this solution, but you have to see the advantages and drawback of both solutions:

  • Extending horizontal space taken limits the vertical space needed and don't require clicking a button. But unlike other properties, it would require you to extend the inspector to see it's value completely, which is, IMHO, unacceptable. While it's not a big deal to need to extend the inspector to see the full property name (because once you know the property name, it's easy to remember what might be missing), it would be a significant issue to have to do this when setting the properties value themselves. It could use an horizontal scrollbar, but this is quite an unusual thing. Horizontal scrolling should generally be avoided as it's not common and intuitive enough, and mice rarely have horizontal scrolling wheels. Remember that, when setting the properties of nodes, you might be doing other things on the main screen to select interesting node, so any space taken by the inspector is not available anymore in the main screen.
  • An expand/collapse button has no impact at all on anyone on small screen. Once opened, which is a local operation only taking more vertical space in the inspector, it does not disturb whatever you are doing in the central screen. The more vertical space taken requires you to scroll a little bit more, but it is a vertical scroll, which is quite common. That being said, to avoid having to click the button in every mask/layer field, I believe the button should probably expand/collapse all fields (the expanded/collapsed state should be global).

Whatever we choose, we wont compromise on Godot being usable on small screen. Godot is used a lot in game jams where people bring their laptop, often 13" wide. So yeah, 32 pixels is important, especially in the inspector.

@Zireael07
Copy link

Just FYI, I'm in favor of an expand/collapse button with a global state, too (I used to use Godot on a 14" laptop, so yeah)

@dalexeev
Copy link
Member

dalexeev commented May 26, 2021

Outdated

I also cannot say that I am categorically against any of the solutions, because I don't need all 32 layers. It just seems to me that we are solving some non-existent or unimportant problem. Until now, only 20 layers were available from the UI, 24 checkboxes are placed in the same place without changes, with a slight change - 28. And if you stretch the inspector a little - all 32.

The gain in space in width is rather hypothetical, since it is not clear who will receive it. For example, my laptop has a screen width of 1366 pixels (most screens are 1920 pixels wide), but I can still afford to extend the inspector a bit. At the same time, those with a wide screen (and those who need more than 16 layers, but not necessarily all 32) will have to waste vertical space, even if their inspector is already wide enough.

In addition, there is a button with 3 dots, clicking on which opens a window with checkboxes for all layers. You can simply make rarely used layers last, and then there will be no problem that they are not visible in the narrow inspector.

Continuing your thought, why not do something like this (now I seem to understand what EricEzaM meant):


And if the inspector is wide enough, remove the minimize button altogether. (It's just that the problem seems so far-fetched to me that it's not worth the code to solve it.)

EDIT. I realized what I didn’t like about the original proposal, and I think that this version will suit everyone:

@groud
Copy link
Member

groud commented May 26, 2021

At the same time, those with a wide screen (and those who need more than 16 layers, but not necessarily all 32) will have to waste vertical space, even if their inspector is already wide enough.

Well, no, since by default it would be collapsed. So neither horizontal or vertical space is lost.

Continuing your thought, why not do something like this (now I seem to understand what EricEzaM meant):

Yep, that's the idea.

And if the inspector is wide enough, remove the minimize button altogether. (It's just that the problem seems so far-fetched to me that it's not worth the code to solve it.)

It looks like a good idea in fact, and It should not be that difficult to implement. It's the best of both solution IMO.

@YuriSizov
Copy link
Contributor

This is going to be a bit of a tangent to the problem we are trying to solve here, but hear me out.

Every time I touch those cells for collision layers, I have one big problem: I can't remember which is which, at all. You can name layers, but there is no place beside the cells to see the names. You can hover, but that's cumbersome (and also doesn't update immediately after the settings are changed, but that's more of a bug). Then you can use the drop menu, but then what's the point of the shorthand cell UI? Now imagine you have 12 cells more.

So I inadvertently start to use cells first in a visual group to help myself navigate a bit, like so:

image

So looking at this I'm thinking if we could do something different while still providing access to all 32 bits. What I think we can do is allow to group and color collections of cells. It can be arranged as a part of the project settings, where you can define groups, group color and then alongside layer's name you can select which group it belongs to. Unsorted cells can then be listed separately. In the inspector groups are displayed as a collapsible section, with a title and the same set of cells, each of which has a color of the group:

groups-and-colors

We can still support long enough collections with horizontal grouping of 4/5 cells with up to 8/10 cells in a row, similar to how it is now. But with groups unneeded cells are explicitly hidden, and users can make their own arrangement that makes sense to them.

One can argue for coloring each layer, but I think groups make more sense overall. As a bonus, coloring of groups can also be used when visualizing collision shapes, somehow, to allow distinguishing them better at a glance. Not sure how though, as nodes can have many groups at the same time.

@Zireael07
Copy link

Grouping/coloring is a nice to have, but showing layer names would be a very good idea indeed.

@mrjustaguy
Copy link

#2770 (comment) might be worth splitting into a separate proposal on it's own.

@YuriSizov
Copy link
Contributor

#2770 (comment) might be worth splitting into a separate proposal on it's own.

Okay, done: #2780. However this issue is resolved, I'd like to have grouping either way. The end solution may change how #2780 is implemented, though, and vice versa.

@dalexeev
Copy link
Member

dalexeev commented May 26, 2021

EDIT. The problem with pycbouh's suggestion and this is that it doesn't look like one or two properties, but a group of properties, plus much less compact. This is fine for the popup, but not for the inspector. Although the idea of being able to assign colors to layers is good.

@mrjustaguy
Copy link

mrjustaguy commented May 26, 2021

Hm, this Name - Layer / Mask setup is interesting...

Pro: Cleaner
Con: More Space Used

@YuriSizov
Copy link
Contributor

YuriSizov commented May 26, 2021

@dalexeev

Although the idea of being able to assign colors to layers is good.

Colors without a name can still be confusing. Add a name to a color and you have a group. The way to display it may differ, of course.

plus much less compact.

I find it as compact as each user wants it to be, but this is not to be discussed here, I think, since I have my own proposal for that now.

@mrjustaguy

Con: More Space Used

We can still save on space in Danil's latest concept if we only show the named layers. It won't be as compact as it is now, but we can't have it both with names and as compact. 🙃

@pouleyKetchoupp
Copy link
Author

In a discussion on RocketChat around layers and masks, @reduz proposed this mockup which is a bit similar to @dalexeev's proposal but takes less vertical space:

Clipboard - May 26, 2021 3_05 PM

This solution does seem like a good compromise, as it would show layer names and could support up to 32 layers.

@dalexeev
Copy link
Member

Colors without a name can still be confusing.

If you use associations, then it will quickly be remembered. Red - enemies, gray - walls, gold - coins, etc. Most people are good at remembering the relative position of elements, if they are not too uniform and not too chaotic.

@YuriSizov
Copy link
Contributor

If you use associations

Association is a tricky thing when you work alone on a project for 6 months and return to configuring layers only every so often.

@pouleyKetchoupp
Copy link
Author

The grid might be important for people to understand the layer and mask concept, especially for visual-oriented users.

In order to display information about layer names, we could do something like this so users know what they are selecting:

@YuriSizov
Copy link
Contributor

YuriSizov commented May 26, 2021

@pouleyKetchoupp Conceptually I love this. Namely because if we were to add groups to this, we could still do it compactly: only color in the grid and the name in the side info panel. This also gives nicer feeling of immediateness compared to tooltips.

That said, this still runs into the problem that the dock becomes too wide. We don't want it to be too wide for the sake of small display users. I've been asked to consider this in a recent PR regarding the Inspector myself. This is way too wide. Maybe we can sacrifice one column for this side panel? According to other participants here it seems that most people won't ever need even 10 layers, so displaying one column of 10 should be more than enough.

@pouleyKetchoupp
Copy link
Author

@pycbouh In the end the text would be probably at the top or bottom instead of the side, so we can save more horizontal space. And the solution from #2770 (comment) can be used to display only the number of layers that fit the horizontal space.

@rune-scape
Copy link

rune-scape commented May 28, 2021

Which rect corresponds to which bit in #2770 (comment)?
Should it look like one of these?
Neither of them are intuitive to me. But I guess the one on the right makes more sense.
godot-2770-1

@rune-scape
Copy link

I feel like the simple one originally proposed is unambiguous in bit layout, takes up less space than the original 20-layer one, and exposes all 32 layers (and is trivial to implement).

@rune-scape
Copy link

I'm running into a "bug" while trying to get the PopupMenu button up to the EditorInspectorSection.

The bottom editor's existence has precedence over the label_reference height:
https://github.com/godotengine/godot/blob/364ea7f280a3f074795e542b16b1d0ec76cf6ce2/editor/editor_inspector.cpp#L222-L227

Leading to a very minor visual inconsistency.
godot-2770-2

@rune-scape
Copy link

rune-scape commented May 29, 2021

And I would like to point out that CustomPropertyEditor has a layer editor too.
godot-2770-3
Although not as nice looking. (And the layer names are off by 1 😞)
I could make use of the EditorPropertyLayersGrid in there if anyone cares?

@rune-scape
Copy link

rune-scape commented May 30, 2021

Maybe an editor setting like horizontal_vector_2_editing to switch between minimum vertical space and minimum horizontal space would cover @EricEzaM's case?
Possibly named horizontal_layer_editing.

@akien-mga
Copy link
Member

Implemented by godotengine/godot#51039 and godotengine/godot#51040.

@dalexeev
Copy link
Member

Why are render layers still limited to 20? light_mask is int32, isn't it? Or the limitation in something else?

@pouleyKetchoupp
Copy link
Author

Why are render layers still limited to 20? light_mask is int32, isn't it? Or the limitation in something else?

From what I remember, rendering has reserved layers for internal logic but I can't find that in the code base. Maybe someone from @godotengine/rendering can confirm? Otherwise the same change could be done in render to use up to 32 layers.

@Calinou
Copy link
Member

Calinou commented Aug 23, 2021

I think the 3D editor gizmos use layer 31 and 32 internally, but I'm not 100% sure.

@pouleyKetchoupp
Copy link
Author

I think the 3D editor gizmos use layer 31 and 32 internally, but I'm not 100% sure.

Indeed, it seems to use layers 24-27 for editor purpose:
https://github.com/godotengine/godot/blob/b86a1cc248a27158fbac631cedaa27b815faa9cd/editor/plugins/node_3d_editor_plugin.h#L156-L163

@dalexeev
Copy link
Member

I think the 3D editor gizmos use layer 31 and 32 internally, but I'm not 100% sure.

Indeed, it seems to use layers 24-27 for editor purpose

This needs to be documented as the layers are still available from user code.

Also, if layers 20-23 are available, then it makes sense to make them visible in the editor. There will be 3 groups of 8 layers.

@pouleyKetchoupp pouleyKetchoupp modified the milestones: 4.0, 3.x Aug 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants