-
Notifications
You must be signed in to change notification settings - Fork 42
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
base: master
Are you sure you want to change the base?
Conversation
Some thoughts before I look at the changes in detail:
|
One other thing I notice in the screenshot: the "Display Alignment" header is not left-aligned with the other headers. |
It's not your imagination. Looks like tiny buttons use point size 9 cboe/src/dialogxml/widgets/button.cpp Line 58 in df0c46d
and LEDs use point size 10. cboe/src/dialogxml/widgets/led.cpp Line 32 in df0c46d
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? |
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? |
Somewhere along the line, I broke the map scale so it was using UI scale.
I discovered how |
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()){ |
There was a problem hiding this comment.
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.
I think that screenshot looks good. What do you dislike about it? I also had another thought about how it could be done:
|
Ok I'll flip the scale LEDs and the display alignment widget tomorrow. |
This reverts commit 489f753.
@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. |
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