-
Notifications
You must be signed in to change notification settings - Fork 115
bugfix(gui): fix resolution and zoom scaling of unit information #1573
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
base: main
Are you sure you want to change the base?
Conversation
|
Going to look back at the "Bombed" icons again and decouple them from the health bar size so they have a consistent size. |
6a466c8 to
b3e7544
Compare
|
Pushed the tweaked Timed and remote bomb scaling to be uniform in size now. |
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.
I think this would be easier to review if refactors and scaling changes were separated in more separated commits.
I only tested this change in 800 x 600, side by side with the Original Game and just a few of the icons.
Here are my thoughts.
- Zoom scaling is nice and ideally is an option in InGameUI.ini, enabled by default.
- Icon scaling depending on object size ideally is an option in InGameUI.ini, turned off by default.
- Thicker health bar outline should be configurable in InGameUI.ini, set to original size by default.
- Default icon size needs more consideration. It would be good to have an option to use exactly the original icon scaling, and then build all the new scaling on top of that.
Perhaps split this change into more changes. First do the refactor, then do the zoom scaling. And then do the resizing of icons with options.
Ammo icon - Original left, Patch right
Ammo icons are too narrow and small in Patch.
Cargo icon - Original left, Patch right
Cargo icons are too narrow and small in Patch.
Disabled icon - Original left, Patch right
Disabled icons are noticably smaller than original.
Propaganda icon - Original left, Patch right
Propaganda icons now have a per object scale.
Repair icon - Original left, Patch right
Repair icons are significantly smaller than original.
| } | ||
|
|
||
| // Display::getWidthScale ========================================================= | ||
| /** Return the ratio of the current display width relative to the default resolution */ |
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.
Please do not use /* for comments. It screws with syntax highlighting in diff tools (when spanning multiple lines).
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.
replicating the style of the original code that is currently in place.
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.
There are // just below the new function.
GeneralsMD/Code/GameEngineDevice/Source/W3DDevice/GameClient/W3DView.cpp
Show resolved
Hide resolved
| //============================================================================= | ||
| Real Display::getHeightScale(void) | ||
| { | ||
| return INT_TO_REAL(m_height) / DEFAULT_DISPLAY_HEIGHT; |
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.
Can just use (Real)m_height?
| Int frameWidth = getIconInfo()->m_icon[ ICON_CARBOMB ]->getCurrentFrameWidth(); | ||
| Int frameHeight = getIconInfo()->m_icon[ ICON_CARBOMB ]->getCurrentFrameHeight(); | ||
| Int barWidth = healthBarRegion->width();; | ||
| Int barHeight = healthBarRegion->height();; |
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.
;;
| else | ||
| { | ||
| killIcon(ICON_CARBOMB); | ||
| } |
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.
Prefer not deleting { } if the other side of the branch does have { }
| Real healthBoxOutlineSize = 1.0f; | ||
| Real healthBoxWidth = healthBarRegion->width(); | ||
| Real healthBoxHeight = max(3, healthBarRegion->height()); | ||
| Real healthBoxOutlineSize = 2.0f; |
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.
This looks like a style change. Why is this done?
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.
It makes it clearer to see the reduction in health relative to the outline of the health bar.
|
I did not scale many of these 1:1 with their original 800x600 scaling in the end because when i originally did that they become obnoxiously large relative to units or buildings at higher resolutions. in regards to some of them, with the contained pip, yes they are larger when zoomed out, but the moment you zoom in they significantly shrink in size instead of remaining at a consistent size. The ammo pips do the same thing, they considerably shrink in size compared to being over sized when zoomed out. The enthusiastic / propaganda icons were over sized for a lot of units with their original scaling, they were nearly the size of an overlord in that particular instance but considerably shrink when you zoom since they don't maintain scaling when zooming. The repair icon might look smaller at 800x600 when zoomed out, but when zoomed in it becomes too small in the original code etc. Many of the things don't maintain consistent scaling in the original code so when zoomed out at 800x600 they are often significantly larger than they should be. And when that size is maintained with their scaling fixed while zooming, they become obnoxiously large at higher resolutions. |
|
I will look at splitting out the changes into separate commits later. |
|
Also these changes are part of Drawable and have nothing to do with InGameUi. |
|
I am not opposed to changing some defaults, but I think overall defaults should be close to the original, and then make new tweaks opt-in, to give designers the freedom to fine tune it. It should not be the programmers job to fine tune the appearance. |
|
I will break this out then into changes that first fix the resolution scaling for everything, then add the zoom scaling for things that did not originally have it. |











This PR fixes the unit information scaling within the game.
It fixes the scaling and placement of the following:
The majority of the scaling now takes the zoom and difference in resolution, relative to the default resolution that the items were scaled for, into account.
Some of the scaling and placement of info is slightly altered relative to the original 800x600 implementation.
This was done as the original sizes of the information was considered excessive.
I have currently kept the bombed icon set to scale to half the size of a health bar, it makes it more obvious which buildings have been rigged. Prior to this the bombed icon was using the infantries health bar to scale itself and it was then placed at the feet of where the infantry unit used the ability instead of on the building.