-
-
Notifications
You must be signed in to change notification settings - Fork 95
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
Fix various issues with SplitContainer that make scripting it difficult. #6230
Comments
These two issues aren't related to the subject of this proposal. This is about more styling or theming options, not scripting SplitContainer. Should be made into a dedicated proposal (and a dedicated PR, if accepted).
This is not unique to split containers. Any container resizes their children, so if existing tools don't give users enough control/information over the state of the control, then a shared solution must be proposed and implemented, on the Either way, this needs to be split into a dedicated, all-Container proposal too, with a proper solution.
This could be nice to have, feel free to open a PR for that.
Well, that's pretty normal for debug drawing. But isn't your other proposal about exposing a way to access the grab area control? And as a small QoL I think you can make a PR to expose that control already, without it being linked to any other enhancement.
No, but for the sake of argument, a custom SplitContainer can be implemented as an addon, as Godot exposes the necessary logic to create custom containers. |
Acknowledged. Please see my remarks at the bottom regarding breaking out the PR features.
What's unique about split containers, is that the split offset reference of
Acknowledged.
You are right that should the grab area
The issue for me is that I am using these improvements in my project and need them integrated in the engine. They surely can remain custom to my project, but I thought they would benefit others. I am still learning the godot PR culture, and would like to see if this approach for cherry picking this split container PR is possible: It would be far easier for me to strip out the features deemed a bell or whistle, than to create multiple PRs of each bit and piece, all of which were developed as a group and rely on the same code improvements and simplifications I made, the entirety of which runs to 108 lines of code in the cpp file. I also made sure it is a backward-compatible drop-in replacement. What if I were to submit a new PR with all the "core" modifications, minus the minor enhancements that can easily be added back once the core PR were merged? How do you prefer to handle extensive enhancements to a class that are simply impractical to break out into small bits? |
Other containers also include children sizes into their sizing decisions. Like box or flow containers. I don't follow the rest of your argument, but I'm not saying that the signal would be totally useless, just that the split container is in no way unique and it doesn't make sense to handle it specifically for the split container, ignoring the rest. And I still stand by that there is nothing special about the first resize compared to the possible future resizes, when the split factor or the children sizes change. If you make a dedicated proposal, we can discuss it properly, with examples and necessary details. We can't really continue a coherent discussion, and hope others to follow, with these large multifaceted comments with large quotes, can we?
There is no gray area here. The same argument can be made about a lot of things, but we typically don't provide such debug rendering for controls. You can easily create your own drawing logic for things that you need, we just need to provide the necessary references. There is also a proposal for generic debug draw tools for 2D and 3D where something like that may become a reality at some point, but for now I don't think there is a good argument to add this specifically to the split container. RE: PR Culture. PRs should solve specific issues. If some issues are connected, they can be solved together, some general code enhancement in the areas touched by a PR can also be performed together with the main changes. However, you present us here with a set of multiple unrelated features, where only relation between them is that you need them in your project. That's not something that makes them a good bundle inclusion to the engine though. You've identified several points of enhancement or of pain. I can see that some of them can be clearly addressed with standalone, small, easy to verify, review, and accept PRs. Some of them don't need much discussion, can be included in the next release, and be done with. Others are totally arbitrary, and may or may not benefit most users. How are we supposed to gather feedback here? Are users supporting your idea because they like some of the features, or because they love all of them? Or the other way around, do they avoid upvoting the proposal because while they love some, they don't need the others? And again, would this discussion be easy to follow here? I mean, sure, in the end, if several of your ideas are accepted a single PR can be made to close them all. But that will take some time anyway, as this is a request for enhancement that hasn't even started to be discussed. While other changes (like signals for dragging, a getter for the grabber control) can be made now, tested and included now, and don't need a very long discussion. There is simply no point in putting everything in one basket and making everything depend on each other. We don't take enhancements wholesome, each needs to be accepted. So for the sake of discussion, there needs to be dedicated proposals. For the sake of providing useful features to our users, it makes sense to implement the accepted parts when they are accepted, instead of waiting for your entire grand idea to be accepted. It may never be fully agreed upon. |
I appreciate the considered arguments and we can move along when I follow up with a compacted PR. This is a good discussion for me and will improve my future PR's. I would never deliberately suggest that anyone needs a feature because I do, just to be clear about that. I'm still struggling with the assertion that these features are unrelated - they are related because together they fix all the issues known to me in the split container that in my judgement only, would also help others. Nevertheless, I do of course understand how code reasoning is easier in small bits. Furthermore, I'm a new contributor with no track record that you can rely on. |
Describe the project you are working on
GUI elements for handling large collections of items, and GUI elements for organizing related containers of text, lists and collections - common GUI utility used frequently.
Describe the problem or limitation you are having in your project
_ready()
is called earlier than the child containers are resized for the split offset.dragged
signal stops.Control
, which is not exposed for scripting.Describe the feature / enhancement and how it helps to overcome the problem or limitation
split_bar_background
StyleBox
allows solid color, gradients or textures to fill in the split bar behind or in place of the grabber icon.drag_area_scrollbar_offset
is required if one of the child controls has a scrollbar or other selectable control up against the split bar, in order to prevent the drag area from blocking mouse selection of the scroll bar or control.drag_started
anddrag_released
make it easier to script saving and restoring thesplit_offset
dragging_enabled
separates dragging fromdragger_visibility
so the appearance of the drag bar is independent from whether dragging is enabled or not.drag_area_show_drag_area
property allows you to see the drag area during development._splits_ready()
with a corresponding signal so a script knows when the split container children have been resized after the initial sort. This was a big problem because formerly,SplitContainer._ready()
was called before the children had been fit to the container, and there was no easy way to get that information.Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams
The code is complete per PR #72680
The PR combines this proposal #6231 for two enhancements that add begin and end margins to the drag area, and exposes the drag area
Control
to scripting.Screen.Recording.2023-02-03.at.12.36.25.PM.mov
Screen.Recording.2023-02-03.at.12.58.21.PM.mov
If this enhancement will not be used often, can it be worked around with a few lines of script?
No. The script required to work around these difficiencies runs to hundreds of lines of unreliable script. The
_ready
notification issue can't be worked around at all, yields and awaits are required to try and guess when the children will be ready.The new SplitContainer per the PR does the same thing better with just a few lines of script that anyone can understand.
Is there a reason why this should be core and not an add-on in the asset library?
The improvements can't be added via an add-on.
The text was updated successfully, but these errors were encountered: