add & update options#3045
Conversation
There was a problem hiding this comment.
Pull request overview
This PR extends the options system by adding UI/config toggles for resizing certain dialogs/menus and swapping Yes/No button order, while also simplifying volume handling and introducing a C++14-compatible clamp helper.
Changes:
- Added new options: resize popup command menu (level 0–5), resize select dialogs, and swap Yes/No button positions.
- Switched
sound_volume/music_volumeconfig values fromdouble(0–1) toint(0–100) and updated audio backends accordingly. - Introduced
myclampand replaced several manual min/max bounds checks withmyclamp/std::max.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| strings.conf | Adds new system strings for the added options. |
| gframe/sound_manager.h | Updates volume setter APIs to accept int (0–100). |
| gframe/sound_manager.cpp | Converts int volume (0–100) to backend float (0–1). |
| gframe/image_manager.cpp | Applies the new “resize select window” option to button texture scaling. |
| gframe/game.h | Adds new config fields and helpers for popup menu button sizing; declares new UI elements. |
| gframe/game.cpp | Adds option UI controls, config load/save entries, button swapping, and command menu resizing. |
| gframe/event_handler.cpp | Handles new checkbox/scrollbar events; updates popup menu layout logic. |
| gframe/deck_con.cpp | Uses myclamp for big-card zoom bounds. |
| gframe/config.h | Adds myclamp helper template. |
| gframe/client_field.cpp | Uses myclamp/std::max to simplify bounds/max computations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Please don't. |
BTW, while moving to C++17 doesn't seem to have obvious obstacles, it doesn't appear to bring clear benefits for this project at the moment either. |
|
It is 2026 now. The question is not "Why should ygopro move to c++17?" now. |
|
By the way, clamp sometimes hide the real problem. |
I still hold the same view. Upgrading the language standard to C++17 lacks sufficient justification. A small amount of syntactic sugar and a few additions to the standard library have limited significance. As for the fervor with "the standard", I'll pass. Upgrading the language standard to C++17 is also unnecessary. It appears that mainstream compilers will not abandon C++14 for at least the next 10 years.
No reason to change it.
Stay until there are some reasons. Such as compliers dropping support, great new features in new standard. If you can prove C++17 or any newer standard have enough benefits, I'll be happy to upgrade. |
It’s a bit ironic that we are calling C++17 "non-essential" while simultaneously adding manual implementations of C++17 features (like std::clamp) to the project. If these features are useful enough to be implemented by hand, they are essential enough to be used from the standard library. I think it is time to stop polyfilling the 2016s and embrace the baseline of 2026.
In 2026, C++17 is the "common language" of C++ developers. If you would like to know, here are some examples
No, I won't. |
One can always abstract common code snippets into functions. The mere fact that a feature exists in a certain standard doesn't mean we must adopt the entire standard if we only need that one piece.
These are two completely different concepts and shouldn't be lumped together.
Following that logic, I'd argue we should accelerate our move to C++20 instead. Actually, using fewer language features often makes code easier to understand. As for decreasing writing complexity, AI-assisted completion largely mitigates that. Using
If necessary, I'd actually prefer
Pure syntactic sugar. As far as I remember, there are still many similar refactors unfinished in ocgcore. Using syntactic sugar only in some places can actually make the codebase more inconsistent.
It's basically handled automatically by compiler optimizations anyway.
It shouldn't be used at all when dealing with small-scale data.
Fluorohydride/ygopro shouldn't move to C++17 only because you want. |
|
Your arguments conflate "concept" with "implementation" and ignore the fundamental language guarantees introduced after C++14. 1. Guaranteed Copy Elision is NOT an optimization 2. #ifdef cannot replace if constexpr 3. "Syntactic Sugar" vs. Safety 4. The "Small Scale Data" Straw Man 5. Engineering Reality in 2026 Claiming that "AI completion" justifies keeping writing complexity high is a poor engineering standard. We write code for humans to maintain, not for AIs to generate. If the project's goal is to remain a time capsule of 2016, then say so. But don't dismiss standardized, safer, and more efficient language features as "limited significance" when you are already manually polyfilling them. |
add
myclamputility template toconfig.hstd::clampis avail since C++17, but we are using C++14; also replaced a few manual max expressions withstd::maxchange
sound_volume/music_volumefromdoubletointreduce the previous ×100 / ÷100 conversions
add resize popup menu option
replaces the old boolean
resize_popup_menuwith an integer level (0-5); adds a scrollbar in the options UI;ResizeCmdMenu()now reads the level to scale button heightadd resize select window option
when enabled, card/position select dialog buttons scale with the screen (
yScale); when disabled they use a fixed scale (1.2, the previous default for card)add swap yes/no button option
when enabled, place the "No" button on the left, align with some Japan games