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

[NOSQUASH] Use our GUIElement classes everywhere #13732

Merged
merged 10 commits into from Aug 14, 2023

Conversation

Desour
Copy link
Member

@Desour Desour commented Aug 12, 2023

  • Fixes Prefer internal GUIElement/formspec classes whenever possible  #12366.
    • Note though that we can't yet delete the gui classes in irrlicht that we don't use, because irrlicht itself still uses them for other gui elements.
    • Also note that CGUIEditBox is not replaced by GUIEditBox, as latter is an abstract class, and I also don't know what it does different.
  • Additionally, the globals guienv and guiroot are removed.
    • guiroot: We don't actually need to create a root gui element (was a big static text). The guienvironment already provides a root element.
    • guienv : The guienv is always reachable through the renderingengine, so it's redundant.
  • Least and first, touchscreengui is made to compile (had a missing header and a const in vector elem type).
  • The color (BgColor) of buttons is now initialized.

To do

This PR is a Ready for Review.

(I recommend reviewing commits separately.)

How to test

  • Compare before/after of:
    • Any scrollbar.
      Note how the arrow buttons are now brightened on hover.
      Styles, however, do not affect the buttons. (As expected.) See below for test formspec.
    • The volume change menu.
    • The key change menu.
    • The touchscreengui. (Compile with -DENABLE_TOUCH=1.) (Haven't tested on an actual phone.)

Formspec for testing possible style issues with scrollbar (set via devtest node meta editor):

formspec_version[6]
size[10,10]
style[*;bgcolor=red]
button[1,1;2,1;btn;Button]
scrollbar[1,3;0.25,3;vertical;scrbar;0]

@Desour Desour added Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements Formspec labels Aug 12, 2023
@rubenwardy rubenwardy changed the title [NOSQUASH] Use our guielement classes everywhere [NOSQUASH] Use our GUIElement classes everywhere Aug 12, 2023
(Same as what CGUIButton uses (via colors=0).)
Note that GUIScrollBar needs an ISimpleTextureSource now due to button styling.
Just use the root element, like GUIButton:add().
The guienvironment already provides a root gui element, we don't need to add another one.
(For CGUIEnvironment, the env itself is the root element.)
(It can already be accessed via the renderingengine.)
@Desour Desour marked this pull request as ready for review August 12, 2023 16:36
Copy link
Member

@sfan5 sfan5 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did some quick tests, seems good

@sfan5 sfan5 merged commit 16da954 into minetest:master Aug 14, 2023
13 checks passed
@Desour Desour deleted the gui_our_elems branch August 14, 2023 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Formspec Maintenance Tasks to keep the codebase and related parts in order, including architectural improvements One approval ✅ ◻️
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Prefer internal GUIElement/formspec classes whenever possible
2 participants