godot
godot copied to clipboard
godotengine
Add FlowContainer wrap options for center alignment.
Fixes inability to wrap a partially filled, last row or column so that the last items preserve the grid pattern of the previous filled rows or columns.
Adds a last_wrap_alignment property to set a wrap alignment option that determine the wrapping behavior of the last unfilled row or column.
For example, a common desired behavior of a centered flow container of tiles, is for the last row to fill in from the left, rather than per the current default behavior of filling in centered.
The PR includes LAST_WRAP_ALIGNMENT_INHERIT, LAST_WRAP_ALIGNMENT_BEGIN, LAST_WRAP_ALIGNMENT_CENTER, and LAST_WRAP_ALIGNMENT_END options, and also works when the flow container has vertical enabled.
The default last_wrap_alignment is LAST_WRAP_ALIGNMENT_INHERIT which preserves backward compatiblity.
https://user-images.githubusercontent.com/33969780/215350978-4da3cad6-fc8f-4617-942b-a688f015d0ea.mov
@Calinou
Can you advise on what's triggering the format complaint in the static check?
It was an errant TAB :( - all good now...
To avoid this in the future, I recommend configuring your editor to trim trailing whitespace by default :slightly_smiling_face:
Could you amend the commit message? It currently says "Doc fix." which is not an accurate description for what this PR does (see PR workflow).
Thanks for that tip, I'll use that elsewhere! Your code was perfect, just had to add the notify_property_list_changed().
The implementation looks good. The feature has no proposal though.
I hadn't known about the proper process - is a proposal in this case required?
is a proposal in this case required?
Yes: https://github.com/godotengine/godot-proposals
There is a linux doc complaint that looks incorrect, because the default value is indeed 1 for the new property. Nothing changed in the docs - how should this be handled?
Run --doctool and just push the result. Doc strings outside description should not be edited manually.
How often do you want to align the whole grid one way, but the last row of items another way?
If you are using FlowContainer as a grid, then all the time. You would want to maintain a grid arrangement, regardless of alignment. You could think of this enhancement as adding a center-aligned grid container with auto-flow.
A common example is browsing a collection of image tiles or icons.
I think that it's a rare instance where you would ever want to break the grid and have the last row or column mis-aligned with the prior row or column. If there wasn't a backward-compatibility issue, I would opt to have the center align option default to filling in from the left per this enhancement.
Why is it limited to the central alignment only? IMO it makes more sense to allow it with any alignment and default to some
INHERIToption that aligns the last row according to the alignment option.
This could be done, but I think it would be rare to want that behavior, per my prior comment on preserving a grid arrangement regardless of alignment choice.
That said, I'm not entirely sure how useful this is as it can be implemented with an extra container.
You would have to move children in and out of an extra container or use a GridContainer to get the behavior of this enhancement. The issue is there is no script access to alignment_ofs, which a separate container or script needs to respond to to maintain the grid alignment with the prior row or column.
switch (alignment) {
case ALIGNMENT_CENTER: {
if (current_line_idx != 0 && center_wrap != CENTER_WRAP_CENTER && !line_data.is_filled) {
if (center_wrap == CENTER_WRAP_END) {
alignment_ofs = line_data.stretch_avail - (lines_data[current_line_idx - 1].stretch_avail * 0.5);
} else { // Is CENTER_WRAP_BEGIN
alignment_ofs = lines_data[current_line_idx - 1].stretch_avail * 0.5;
}
} else {
alignment_ofs = line_data.stretch_avail * 0.5;
}
} break;
case ALIGNMENT_END: {
alignment_ofs = line_data.stretch_avail;
} break;
default:
break;
}
Please use the "Edit" button instead of multiposting.
You would have to move children in and out of an extra container
No, the way you can set it up is to have a parent container and a FlowContainer with a left-side alignment. The in the FlowContainer you would use size flags to position it within the parent container however you want. You would need to give your FlowContainer a min size, but that's already the case here.
Of course, that may not work exactly as you desire if you want FlowContainer to be resizable at runtime. Then this double-alignment system makes sense as a solution. But then I don't see why it must be limited to the center alignment only. It won't change the implementation much, it would just remove an artificial limitation from the new option.
Please use the "Edit" button instead of multiposting.
Roger about multiposting...
Of course, that may not work exactly as you desire if you want FlowContainer to be resizable at runtime. Then this double-alignment system makes sense as a solution. But then I don't see why it must be limited to the center alignment only. It won't change the implementation much, it would just remove an artificial limitation from the new option.
Yes, I presume requiring resizing at runtime. I concede the point and will remove the limitation, which is sensible and simplifies the docs...
Okay, this push makes the last wrap alignment work with any AlignmentMode, and defaults to LAST_WRAP_ALIGNMENT_INHERIT, which continues to preserve backward-compatibility.
I admit I was slightly skeptical of the usefulness for the other alignment modes, but after playing with it, I found it cool and I'm certain there will be many good uses for it.
Sorry for kicking this around. Let's make sure we merge it once 4.3 dev cycle starts (I still would like to review though).
