godotengine
Fix incorrect notice about XDG paths working on all platforms in Data paths
They are only effective on Linux/*BSD. (For reference, in most other apps, it's rare for XDG environment variables to have an effect on other platforms.)
- See https://github.com/godotengine/godot-docs-user-notes/discussions/106#discussioncomment-10757552.
@ator-dev, here is the re-implementation of your PR on master
If you have some spare cycles I would really appreciate if you can test it and verify that I capture properly all your intentions.
I like it, but it may be more useful if we can scroll the view with Ctrl + Arrow keys when the focus style is drawn ie. the ScrollContainer or one of it's children has focus, also you will need to accept the events to avoid focusing another child when the ctrl is pressed.
I like it, but it may be more useful if we can scroll the view with Ctrl + Arrow keys when the focus style is drawn ie. the ScrollContainer or one of it's children has focus, also you will need to accept the events to avoid focusing another child when the ctrl is pressed.
Should I add this in this PR or in another? To be honest, I tried to do it to see if the change is small and decide if I should create a new PR, but my knowledge about Godot GUI is pretty limited and the result that I got was not satisfactory.
Should I add this in this PR or in another? To be honest, I tried to do it to see if the change is small and decide if I should create a new PR, but my knowledge about Godot GUI is pretty limited and the result that I got was not satisfactory.
It's ok, I think i can do it later since we can't rely on gui_input() when a child is focused, this will require a new variable focused and a scroll_page to define how much we should scroll when a key is pressed and to use shortcut_input, so i don't think it will be merged to 4.x if we apply all those changes at once.
Great to see this discussion and thanks for the reimplementation PR!
If you have some spare cycles I would really appreciate if you can test it and verify that I capture properly all your intentions.
I'm sorry, although I mean to, I'm really busy at the moment. What's most important is that the result is accessible and doesn't affect anything weirdly.
I'm sorry, although I mean to, I'm really busy at the moment. What's most important is that the result is accessible and doesn't affect anything weirdly.
No problem!
Note: The focus StyleBox is clipped for the default theme, this should be documented that it will require a focus StyleBox to be drawn without expanding and adding some content margin for the ScrollContainer panel style to avoid drawing the focus StyleBox behind the children.
Edit: I see you're not using the default focus style for the ScrollContainer, so in both cases, you will need to document this since the users will not be able to see the focus style box because it's empty when they enable this feature.
Note: The focus
StyleBoxis clipped for the default theme, this should be documented that it will require a focusStyleBoxto be drawn without expanding and adding some content margin for theScrollContainerpanel style to avoid drawing the focusStyleBoxbehind the children.
Could you please elaborate this? I'm still too new to Godot GUI.
Edit: I see you're not using the default focus style for the
ScrollContainer, so in both cases, you will need to document this since the users will not be able to see the focus style box because it's empty when they enable this feature.
When you say that I should add/update documentation, Are you talking about this file, right?
I'm wondering where I could document the use set_draw_focus_border because that method is public but is not bind/exposed to Godot Engine users. It's not clear to me if this method will be available through GDExtension just because the method is public. If that is the case, Where I should document that GDExtension new method? If not, Will be enough if I add a comment in the C++ code?
One more change is to check if (Engine::get_singleton()->is_editor_hint()) inside ScrollContainer cpp file to avoid running the code when we run a scene in editor builds.
One more change is to check
if (Engine::get_singleton()->is_editor_hint())insideScrollContainercpp file to avoid running the code when we run a scene in editor builds.
Done
@AThousandShips sorry if I tag you wrongly, but this CI job is being running for more than 5 hours. I know I can "stop" it submitting a new commit into the PR, and I'll do it soon, but I would like to let you know because you could set a configuration on GitHub or something to kill stuck CI actions.
@WhalesState I addressed all your comments, please let me know if I capture properly all your requests.
Why is this editor-only?
This makes the newly added focus property useless at runtime.
EDIT:
No, it's entirely useless, because set_draw_focus_border() is not exposed.
If there is a good reason to be like that, it should be mentioned in the docs. But I think it's fine to allow this at runtime.
It's editor only because that was the initial intent on https://github.com/godotengine/godot/pull/59362 and I was hesitant to try to make it available at runtime because I didn't know if that required an special process to follow that. I'll try to see if I can make it working on runtime.
If you want it editor-only, it should be a new type that isn't exposed. Adding things to ScrollContainer that can't be used is not the right solution.
Tested and it seems to work correctly. ScrollContainer does not need to be focused to display the border, so I think the focus change to the inspector is not needed. Could be nice to have some default focus style in default theme, you could copy some existing one.
@KoBeWi sorry but I'm not getting this:
ScrollContainer does not need to be focused to display the border, so I think the focus change to the inspector is not needed.
