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

Remove/rearrange some settings and improve tiny button/LED handling #494

Open
wants to merge 36 commits into
base: master
Choose a base branch
from

Conversation

NQNStudios
Copy link
Collaborator

@NQNStudios NQNStudios commented Nov 24, 2024

This removes the 3 settings that appeared to be performance related.

Resetting the help messages is now a button, which triggers a confirmation dialog. Makes more sense than being an LED.

It's no longer possible to set your UI so scale that you can't change it back. The preferences window is also more fitted to the main window now. Fix #470

Screenshot 2024-11-24 at 5 02 04 PM

@CelticMinstrel
Copy link
Member

Some thoughts before I look at the changes in detail:

  • I kinda aesthetically like it best with the display alignment options at the top. I think putting scale beneath those would still satisfy it being reversible even if you set it too high? Other than if you choose an option which wouldn't physically fit on your screen at all even for the main window, but the correct answer there is to not show those options. I'm not going to say I'm absolutely against leaving it in its current position though.
  • Let's move the Never Show Help button down to just above the Reset Help button. Makes more sense to keep the two help options together.
  • Is it my imagination or does Reset Help have a smaller font size? It shouldn't.

@CelticMinstrel
Copy link
Member

One other thing I notice in the screenshot: the "Display Alignment" header is not left-aligned with the other headers.

@NQNStudios
Copy link
Collaborator Author

Is it my imagination or does Reset Help have a smaller font size? It shouldn't.

It's not your imagination. Looks like tiny buttons use point size 9

if(type == BTN_TINY) style.pointSize = 9;

and LEDs use point size 10.

textSize(10) {

This is the only tiny button in the game itself. All others are in the scenario editor. Do we bump the text size up a point for tiny buttons across the board and not worry what might happen in the editor?

Or do we add a code path that allows configuring the tiny button text size on a case by case basis?

@NQNStudios
Copy link
Collaborator Author

NQNStudios commented Nov 25, 2024

I kinda aesthetically like it best with the display alignment options at the top.

I tried doing it that way first, and thought it looked really bad. But I forgot to take a screenshot or save a copy of the XML to make it happen. It's kinda tricky doing things back and forth--if I just edit the image to give you an idea of how the other way looks, would you be able to decide from that if you do want it that way?

image

@NQNStudios
Copy link
Collaborator Author

I discovered how leds and tiny buttons were all using a hard-coded width for their hitboxes. That was when this PR became more of a sweeping overhaul haha.

throw xMissingAttr(elem.Value(), "width", elem.Row(), elem.Column(), fname);
}
}

location cButton::getPreferredSize() const {
return {btnRects[type][0].width(), btnRects[type][0].height()};
int width = btnRects[type][0].width();
if(type == BTN_TINY && !getText().empty()){
Copy link
Member

Choose a reason for hiding this comment

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

Hmm… this could probably also be done for BTN_PUSH later (the label should probably be centered beneath the button in that case)… though I think it's unused right now so I guess it's not a priority.

@CelticMinstrel
Copy link
Member

if I just edit the image to give you an idea of how the other way looks, would you be able to decide from that if you do want it that way?

I think that screenshot looks good. What do you dislike about it?

I also had another thought about how it could be done:

Display Alignment                          Scale
+------------------------------------+  UI      Map
| [ ] Top Left      [ ] Top Right    |  [ ] 1   [ ] 1
|                                    |  [ ] 1.5 [ ] 1.5
|             [ ] Center             |  [ ] 2   [ ] 2
|                                    |  [ ] 3   [ ] 3
| [ ] Bottom Left   [ ] Bottom Right |  [ ] 4   [ ] 4
+------------------------------------+
  [ ] Window

@NQNStudios
Copy link
Collaborator Author

Ok I'll flip the scale LEDs and the display alignment widget tomorrow.

@NQNStudios NQNStudios changed the title Remove/rearrange some settings. Remove/rearrange some settings and improve tiny button/LED handling Nov 26, 2024
@NQNStudios
Copy link
Collaborator Author

Screenshot 2024-11-26 at 12 15 28 PM

I think this is pretty dang good. And you can see it fits inside the main window.

@NQNStudios
Copy link
Collaborator Author

@CelticMinstrel two commits came along for the ride here to fix some things that bothered me while testing the rest of it with a replay. They could split into another PR, but if there's any more back-and-forth to do with the rest, it'd be nice to keep at least the first one in this branch until we lock in the rest. Also, I think both commits are good changes with low risk and might as well not be split out.

aff33dd disables the Spiderweb Software splash screen when running a replay with the No Splash Screen setting (the setting usually only disables the 2nd splash screen.) This saves a lot of time when running replays repeatedly.

166d71a Since I implemented replaying the startup menu buttons, they've had that visual bug where the rest of the screen is rendered black. I fixed that.

If you like the layout I shared just above, this is ready to merge.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Preferences window taller than the main window (and possibly other dialogs are too?)
2 participants