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

Optionally align the top toolbar buttons horizontally centred #21989

Merged
merged 11 commits into from
May 10, 2024

Conversation

AaronVanGeffen
Copy link
Member

@AaronVanGeffen AaronVanGeffen commented May 8, 2024

Implements #622 / #15643 as an optional alternative toolbar mode.

  • It seems clang-tidy, macOS, and Ubuntu Jammy don't support std::ranges::reverse_view yet?
  • Fix potential overlap with FPS counter
  • Test toggling the setting a bit.
  • Add string ids for the new option strings.
  • What should be the default layout? (on or off?) -> off

For a follow-up PR, we could look into moving the bottom panels to the top as well, or indeed moving the buttons to the bottom. I'd prefer not to move the goal posts for this PR too much.

Setting on:
centre

Setting off:
left_right

@AaronVanGeffen AaronVanGeffen changed the title TopToolbar: refactor OnPrepareDraw into separate functions Optionally align the top toolbar buttons horizontally centred May 8, 2024
@AaronVanGeffen AaronVanGeffen added the changelog This issue/PR deserves a changelog entry. label May 8, 2024
@AaronVanGeffen AaronVanGeffen force-pushed the toolbar-alignment branch 2 times, most recently from 776fa4c to 96ce7f7 Compare May 10, 2024 20:23
@AaronVanGeffen AaronVanGeffen merged commit b0a3888 into OpenRCT2:develop May 10, 2024
24 checks passed
@AaronVanGeffen AaronVanGeffen deleted the toolbar-alignment branch May 10, 2024 20:42
@Gymnasiast Gymnasiast added this to the v0.4.12 milestone May 10, 2024
{
screenCoords.y = kTopToolbarHeight + 3;
}

DrawText(dpi, screenCoords, { COLOUR_WHITE }, buffer);

// Make area dirty so the text doesn't get drawn over the last
Copy link
Contributor

Choose a reason for hiding this comment

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

L143 needs to be updated to take into account the new variable y location of the fps counter. Causes #22066

Copy link
Member Author

Choose a reason for hiding this comment

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

Oops. This is a bit surprising though, as it uses the same screenCoords variable we modify up ahead. Will look into it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog This issue/PR deserves a changelog entry.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants