-
-
Notifications
You must be signed in to change notification settings - Fork 21.6k
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
Reorganize inspector layout workflow for Control
nodes
#55157
Reorganize inspector layout workflow for Control
nodes
#55157
Conversation
I'm excited to play with this new UI workflow. I still hate the philosophy behind Control to bits, but this PR looks like it could make Control nodes much more intuitive and easier to use! Thanks a lot for all the work as well as presenting it in such detail! |
1b02b92
to
c698162
Compare
It's all a part of the same property, vertical/horizontal size flag.
No, it only allows flags that the container supports. The reason for
I realized that later, but saw no point reworking the toolbar when it was already done properly with the drawers PR. I can remove "Fill & Expand" from the list, if that helps, but I've decided that until drawers are merged we can live with the same options that were previously available for containers in the Inspector.
IIRC I was hesitant to change the way presets from the toolbar are applied, because I'm not sure how people use all those options. I mostly don't have need for anchor mode or keeping aspect, or setting anchors without offsets. So I didn't want to break that, while still making the Inspector changes to the spec from the proposal. I'm fine making the toolbar behave the same as the Inspector. Keep in mind, that inspector also changes grow direction in anchor preset mode.
They are supposed to be editor-only, but I can't make them fully editor-only with the flags that we have. I guess the only side-effect here would be that setting them can potentially reset some values? |
For the rest: I'll take a look and fix the problems. |
This looks very good for the most part to me. My main gripe is that I would like to re-discuss the size flags as I am not entirely sure they can be as best as they can be. Guess when you are back from vacation we can see this. Also would appreciate a review from @groud |
@madmiraal I'm not sure what point you are trying to convey? The part that you've quoted explains that there are two systems, anchors and containers. Offsets/margins are a part of the anchors system. |
Sorry, my confusion. Let me read it all again and make sure I understand what this change is doing. |
I think its good and would be nice to merge and get feedback. We can discuss the size flags at another time. |
0b5de9d
to
e86b148
Compare
e86b148
to
b20ed45
Compare
b20ed45
to
107b6f2
Compare
All should work fine now, though UndoRedo has already worked correctly for me. Note that the layout mode changes immediately upon starting the drag, but the rest of the inspector only updates when you let go. This is because the layout mode is deduced from non-zero anchors, but the rest requires an inspector update and doing it too quickly causes a lot of lag.
Done, though should be noted that I followed the existing convention for the getter and setter of size flags. I'll update them in a separate, follow-up PR to avoid making this bigger still. Same for grow direction, I guess. |
Thanks! |
Problem, it seems it is now impossible to see the size of a control when it is under a container. Sure I dont want to modify it, but I want to see its width and height... like, for example, just to know how much to set the |
It's a bit confusing to me because I'm not seeing how I can control the margins (or offsets) anywhere in the inspector. I don't see the "offsets" and "grow direction" groups as in the video in the beginning of the pull request description. I take it that changing size and position will automatically set these "offsets" but what if I want something to be "full rect" and have a "margin" of 10 on each side?
This also just tripped me up a lot, as I can't control the grow directions through the inspector and setting the anchors in the toolbat had no effect on it, I though the grow direction had been completely removed, until I read this comment on the PR and set the anchor through the inspector. As I'm coming from Godot 3.x my first reflex it to use the toolbar I'm already used to. ---- EDIT ---- I don't know, but I feel like there should be a better way of doing this? Maybe always showing offsets and grow direction on anchor mode, and if you change their value automatically set the preset to "Custom"? Or at least setting the default preset of uncontrolled mode to custom? As what I was trying to do was just set the grow direction of some debug labels on a character to always grow up and never down like this: On 3.x in this case, I don't need to care about anchors or offsets, this is pretty much a position "mode" but I still need grow direction, and it took me a while to find this pull request and find out how I could do this. And this in fact might be a problem. If I have a control that is a child of another control, and I want to position it manually but change the grow direction, I won't be able to do it by "Position Mode", I'll have to set "Anchor Mode", then set "Custom" just to change the grow direction. |
This is 100% the intention. The related proposal and this PR aimed to streamline the UI so that users are not confused by the amount of options and when and why they are relevant. Position mode is the simplest way to position a control with anchors. It's not that useful, IMO, but it'll work for people trying things without having to burden them with anchors and stuff. The rest of your suggestions hit the same wall. Yes, you need to explicitly change the anchor mode to custom to edit those properties. That's by design, because we want to limit the options so users don't get overwhelmed and confused. If you're experienced with Godot UI, it's not a big deal to enable the custom setting. If you're not that experienced, you have less chances to break things. There is an existing complaint that we should at least show the properties, but make them readonly. We haven't decided on that yet. But making them visible and editable, or making the defaults to be on par with 3.x would go against the whole point of this change. Yes, you will have to relearn how this part of the inspector works. Regarding the toolbar, it's something that will be fixed one way or another before 4.0 stable, but we hit a roadblock in making a decision about toolbars in general, so the resolution of that is halted at the moment. But yeah, we should probably make presets set with the toolbar to affect the grow direction as well and be on par with the inspector presets. |
I understand the intention is to reduce clutter, and I agree that most of my suggestions do hit that wall, but I wouldn't count "grow direction" towards clutter. It is something that works independent of anchors, can work on position mode, can work on uncontrolled mode, and being set according to the anchors is just a "quality of life" feature. I think it should just be a normal property, or at least visible in all cases besides "container" mode. About the read-only suggestions, I think some cases are necessary, like I've seen somewhere about not being able to see current The way things are now I don't think I would ever even imagine But I still think |
Yes, I understand your concern. It boiled down to the idea that it's good to set Grow Direction when you set a preset, because a lot of times a certain grow direction makes a lot of sense by default for a certain preset. And in the new flow the fields that are forced by a preset are non-editable. We can make the grow direction fields always available, change them without user's knowledge when they apply the preset and change the preset to be "Custom" if the user overrides them in turn. But the question is whether that would lead to a good experience, or if it's going to be confusing. |
I think the question is actually if it would be better/worse experience or more/less confusing in relation to the current workflow where it is hidden under the Custom anchors layout. But I guess this would need some kind of A/B testing or focused test, as I'll obviously be biased to what I think, and I don't really see grow direction as something "attached" to Anchor Presets, since it's functionality is independent on anchors or offsets, and it doesn't need to be visible always, as they do not make sense for controls inside a container, so they shouldn't be visible in "container mode". So yeah, in my bias I do think it would be a better experience, specially if you're using the inspector to change the Anchor Presets, and the grow direction changes at the same time when you change the Anchor Presets, as you'd be able to see that happening, it's not totally masked (I mean, at least it's not if the "grow direction" group is opened, as in, you just edited it, so I understand your concern). But I wouldn't change the Presets to Custom when you change the grow directions though. If the grow directions properties are not nested inside the anchors, as I think they shouldn't be, changing it should not affect the anchor presets at all. Again, grow direction is not part of a anchor preset, as it's functionality is not related to anchors, adjusting them together with anchor presets is just a "quality of life" thing. If this "quality of life" is confusing and we need to stay consistent, I'd go for anchor presets not changing grow direction in any case at all or maybe making it optional with an editor setting like |
I personally find the new Control-Container workflow in Godot4 (as tested in Alpha9) a huuuuge improvement over the the Godot3 workflow. I find it much more intuitive and much less confusing. Thank you very much @YuriSizov! I have to agree though with @eh-jogos on that finding the Custom mode is way too difficult. Personally I would have expected to find it in the "Layout Mode". Most of the Control node and Control Container user flow should be reasonably intuitive now, even for complete beginners who have not used them before. A few things that are not intuitively clear to me now in Godot4 Alpha9:
Unfortunately there is still one glaring issue I have, which is best illustrated with one of the most basic and imho most common usecase:
Yes, I how how to do this the "Godot way", but it this is highly counter intuitive and took me years to figure out. Unfortunately it does not seem to have improved in Godot4 Alpha9 yet. In fact it got even worse. Imagine following scenario: You don't want to do some elaborate layouting, all you want to do is to add a bit of text (Label), make sure it's origin is centered, so if you add any amount of text it will stay centered on the origin. Sounds simple? Well in Godot3 and still in Godot4 it is not. Naturally the user will first check the Inspector, and find the "horizontal Alignment" there. however setting this property to "center" will not have the desired results, since this will just center the text relative to the Labels rect, not set the Label nodes origin to the rect center and keep it there, expanding the rect outward as desired. So you start to learn more about the Layout options in the toolbar and obviously try "Layout: Center", now in Godot4 called "Anchor: Center". But this will result in suddenly placing the Label in the center of the screen (if the parent is a Control root node and therefore has "Full Rect") or centered on the parent origin if it is a Node2D. It will also appear centered if the parent is a Control node with a rect size of 0 (super confusing if the rect later expands). Anyway. Even though having a parent Control with 0 rect size or Node2D first looks like the solution, it becomes obvious it is not as soon as you start typing more text and the Label rect still expands only to the right. You can click the Layout/Anchor toolbar "Center" again to again center the Label rect in relation to the parent, but you would have to do this every time you change the text! So maybe both? Go back to the Inspector "horizontal Alignment" setting and set it to center, but of course this won't help, since this still affect only how the text is aligned inside the Label rect. (a fact also easily missed by a beginner). Instead in Godot3 you had to set the "Grow Direction" to "Both". In Godot4 Apha9, this property is now impossible hard to find. Making all of this even a greater torture: You have figure out you have to click "Anchors" in the toolbar, for some reason randomly click "keep current Ratio", and then happen to again look through all the Inspector settings and notice you have now entered the "Custom" mode, which finally reveals the "Grow Direction", which you have to know that "Both" means finally what you have thought to be a singular one button solution called something like "Center Alignment" or "Growth: Center Alignment". This is really really bad. It's a UI flow made by the devil. What's worse is how this does not even work for scaling and rotation. If you now want to scale or rotate, Godot will do this from the origin which is fixed to the top left. unless you set the so called Pivot Offset. So every time the text changes length, even though you have figured out how to get to the "grow both" setting, you still need to be manually set the Pivot Offset using pixel values! There is no built in way to use percentages or set it to preset positions like the Anchors would do. Imagine what a nightmare this is if you want to animate rotation and scale of a centered expanding text (or anything else you want centered and the rect expanding) I have a feeling at the core of this convoluted mess is still the fixed origin. Overall though, I think this is a great improvement to user friendlyness, so thanks again to @YuriSizov and everyone who reviewed and contributed! |
Actually the way it works now, setting the Layout to center should fix it, as it will automatically set the grow direction to both on horizontal and vertical values. As @KoBeWi first pointed in this conversation and @YuriSizov confirmed, the toolbar not setting grow direction is a bug that is waiting for consensus on how to treat toolbar things, but if you set the Acnhors to Center through the inspector instead of the toolbar, it already works.
I gather that right now, if you use a Control with And I don't know about other game engines, but when I used flash, even though we could change the origin of stuff, sometimes the best solution was still to have a "pivot" element you can animate freely and keep your text or whatever other element properties "clean" and "default" so you could do other effects with it. Maybe this is like the grow direction thing, and improving documentation and tutorials to showcase this should help? I know your pain and a documentation section/tutorial on "animating UI" would really help people having to figure out this things on their own. I'm reading the guidelines on how to contribute to the documentation to see if I can help with some of this. |
I see. Well, I can confirm the bug. And you would also have to know you can rotate and scale the node you want to rotate and scale, but only the parent, which again, has to meet certain criteria to ever work as a pivot. This is really backwards stupid and the opposite of a intuitive beginner friendly design. I honestly doubt though there is much one can do about this without completely rethinking Control nodes at their very core. So my hope for Godot4 is to at least make these processes as transparent and automated as possible.
Yes extra pivots "in between" nodes are cool very useful and have their place. But adding extra nodes should not be a necessity for the most basic tasks like centering, rotation and scaling. Godot already has origin an pivot built into every Control. It's just the user workflow implementation is completely shit in Godot 3 and prior and has not been changed since. |
you don't need to add another node as a pivot just to get grow directions working, you can just go to the inspector, set the Anchor preset to Center or if you want to have more control, set it to Custom and change the grow direction properties I agree this is not super beginner friendly which was what I was talking about grow direction not being tied to anchors as it is useful and works independent of anchors, but it is also not as hard as you say. The Label can be a child of any other node and just to center the text this will always work, though currently it needs to be done in the inspector instead of the toolbar. To animate the text, or UI in general is another story. And a tutorial/documentation with these common tips could go a long way in easing this pains even if not the ideal solution. But I still agree that percentage based pivot offset would help on some cases. |
I did not claim you would. What I did say is you need another parent node with 0 rect size in order to center your node (to continously grow, scale and rotate from center). Like I said, I don't assume this will be fixed with Godot4, but I really hope we at least can make this process a lot more transparent and automated (one button click upfront and and center, instead of 5 clicks into cascading menus who's purpose and discoverability is totally opaque for beginners) |
Closes godotengine/godot-proposals#2841, closes godotengine/godot-proposals#299.
Following the rationale and suggestions presented in the aforementioned proposals this provides a new, focused way to set up the layout and position property for
Control
nodes in Godot. I think it's best to present these changes in a form of a minute long overview of the new flow:2021-11-20_15-59-54.mp4
The idea is to only suggest properties that are currently relevant when working with control nodes. Godot has two distinct positioning systems (with some permutations within): anchors and containers. Those two positioning modes are complimentary to each other, but cannot be used at the same time on the same control. This is a point of huge confusion for our users, as the editor normally doesn't guide you or prevent you from changing the wrong properties in a wrong context.
A recent PR to prevent editing properties controlled by the container helps with preventing, but doesn't really give a good cues as to why the properties are suddenly unavailable. To address that (and using the discussion from godotengine/godot-proposals#2841) I've introduced a new common "Layout" section to the
Control
properties that encompasses everything related to positioning. It shows some universal properties by default, but also hides most of the rest beyond the selected "Layout Mode" setting.Layout mode can have one of these 4 values:
2021-11-20_16-35-08.mp4
To help with guiding users I've added a collapsible warning to the top of the "Layout" section. In its short form it gives a quick message that can be understood and used by experienced users. If extended, it gives a short tip about what positioning properties are available.
Additionally I've introduced an "indented" rule for the inspector properties that allows to indent a group of properties with a special hint and give that indentation a little colorful side-border. I find this quite important to highlight among others those sections that become editable when the "Custom" mode is selected for anchors. I expect that this may not be a desired addition to the Inspector code, or maybe just the implementation, but I really think it brings the whole thing together on the user experience level. I've experimented a bit with different highlighting rules, or none at all, and this is really what I found to give the best visual association.
Note that every new component works well with RTL layouts as well!
I've also added quick container sizing options to the toolbar. They are a bit ugly and take space, but that's something that I will hopefully solve with godotengine/godot-proposals#3550. But something felt amiss in the toolbar without being able to quickly adjust container children as well as non-container children.
Those options are properly disabled following the same parent node rules as the layout options. With an explanation added via the tooltip (as it was already done before for the anchor layout presets). Speaking of which I've tried to explain better what "Keep Ratio" does with a new name and a tooltip:
Even with the aforementioned proposal to move to tool drawers this dropdown list is not at all practical for its own set purpose. I'm eyeing godotengine/godot-proposals#3559 as a way to address it, albeit I would want to start with a less custom solution and just rearrange the existing icons in a rectangle, corresponding to their configuration.
One technical note. The new layout mode and anchor preset inspector properties are supposed to be inspector only. I managed to remove them from the docs using
PROPERTY_USAGE_INTERNAL
, but they are still available to scripting, even though there is no purpose for them there. I don't know of a better approach to provide such inspector items, as I need an actual property to drive the rest of the properties there, and make them enabled or disabled, and I need to have it exactly where they are now. If anyone has a better suggestion for this, let me know. But this is probably not that problematic and can be left as is.