-
Notifications
You must be signed in to change notification settings - Fork 74
[GEN][ZH] Fix viewport scroll speed so it is independent of FPS in LookAtTranslator::translateGameMessage() #1026
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
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.
Tested, works great!
Generals/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp
Outdated
Show resolved
Hide resolved
Generals/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp
Outdated
Show resolved
Hide resolved
Generals/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp
Outdated
Show resolved
Hide resolved
Generals/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp
Outdated
Show resolved
Hide resolved
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 change does not link to the relevant issue reports.
5cf8ff1
to
a1da7ad
Compare
Tweaked based on feedback. |
a1da7ad
to
24efba7
Compare
Updated the code to use the current / last calculated FPS. |
24efba7
to
0f9ffcb
Compare
Im still working on this, need to alter the implementation a little since right mouse button scroll doesn't work quite right. |
0f9ffcb
to
acb7301
Compare
Made some small updates to this, some tweaks improve the functionality of the right mouse button scrolling so needed a refactoring of my original implementation. I also needed to make sure to scale the scrolling based on the current logic rate and not the fixed 30 FPS as it was causing issues when people use a faster play speed in skirmish or on custom maps. This does not include any scaling changes on max scroll speed. |
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.
How does the scroll speed now compare to Original Game Scroll Speed?
I think we do need a reference point, where the changed Scroll Speed matches the Scroll Speed at a certain FPS of the Original Game.
I think 30 FPS is a good candidate, because that is the default Single Player and Replay frame rate. So basically we can setup the default Scroll Speed to be identical to the Original Game at 30 FPS.
That way, it will be also easier to reason how much something needs to be scaled to match the scrolling speed of the original game. So for example players using 60 FPS would then need to 2x their Scroll Speeds to get the same speed as they did in the Original Game.
Generals/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp
Outdated
Show resolved
Hide resolved
GeneralsMD/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp
Outdated
Show resolved
Hide resolved
Generals/Code/GameEngine/Source/GameClient/MessageStream/LookAtXlat.cpp
Outdated
Show resolved
Hide resolved
That is the whole function of the logicToFps ratio. It makes the scrolling speed match what the scroll speed should be when the FPS is capped at the vanilla value. When our the fps matches the original target and the logic rate is also locked to 30, the ratio is just 1.0. meaning the game is just scrolling at what it was originally expected. The scroll messages are generated in the game client which can run significantly faster than the game logic when the FPS is uncapped. so as FPS increases we get more scroll messages appearing before they are acted on within the game logic. So to counter this we rescale the affect each individual scroll message has. That way at higher FPS each scroll message has a smaller movement. At one point when i used a locked logic rate of 30FPS, it would cause the scroll speed to be slower than it should when a person increased the game speed in a skirmish game beyond 30. So i changed this to take the current game speed into account and that then let the scroll speed scale properly once more. The scroll option can then further scale the scrolling amount as it was originally intended to with a reasonable affect on the scrolling rate. |
acb7301
to
7280621
Compare
Tweaked the implementation, the original implementation just did not work how it was supposed to. A side effect of how i have implemented this now is that the scroll speed setting works properly with the RMB scrolling |
7280621
to
b35b98c
Compare
Need to check into this a bit more, when playing online there seems to be issues with the scroll speed still. There must be something i am missing in that instance. |
offset.x += TheGlobalData->m_horizontalScrollSpeedFactor * vec.x * sqr(TheGlobalData->m_keyboardScrollFactor); | ||
offset.y += TheGlobalData->m_verticalScrollSpeedFactor * vec.y * sqr(TheGlobalData->m_keyboardScrollFactor); | ||
offset.x = TheGlobalData->m_horizontalScrollSpeedFactor * logicToFpsRatio * vecLength * vec.x * TheGlobalData->m_keyboardScrollFactor; | ||
offset.y = TheGlobalData->m_verticalScrollSpeedFactor * logicToFpsRatio * vecLength * vec.y * TheGlobalData->m_keyboardScrollFactor; |
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 as if it does not produce the same result as before. You can verify correctness by temporarily running the original code beside the refactored code, then assert value equality (with a small epsilon for rounding error tolerance when so necessary).
Simplifying the implementation is certainly desirable.
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.
After doing a little more testing last night the original code doesn't actually do what it was intended to.
The significant factor that makes up the majority of the movement is the following block from the original code.
offset.x = TheGlobalData->m_horizontalScrollSpeedFactor * (m_currentPos.x - m_anchor.x);
offset.y = TheGlobalData->m_verticalScrollSpeedFactor * (m_currentPos.y - m_anchor.y);
The second block that gets added to the offset provides a miniscule amount to the overall movement. It's why when you change the scrolling speed it hardly affected right mouse scrolling.
If you just set the RMB scrolling to run from the second block you can see that it hardly moves the viewport, even with the scroll speed set to maximum. Making that added factor insignificant in the end.
(you also have to make sure the FPS scaling is handled when checking)
The refactored code allows the scrolling to work properly and get scaled by the scroll speed options.
When compared with the first line in the original code, it has essentially become the following
offset.x = TheGlobalData->m_horizontalScrollSpeedFactor * vec.x * vecLength;
offset.y = TheGlobalData->m_verticalScrollSpeedFactor * vec.y * vecLength;
That then provides normalised scrolling in all directions. Then you include the fps scaling to mitigate the FPS issues, Then multiply by TheGlobalData->m_keyboardScrollFactor
which adds the ability to alter the scroll speed through the options menu.
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.
The speed of the scrolling can be a little faster than expected at the moment.
But i think to work around that we just need to rescale how the keyboard scroll factor affects the RMB scrolling.
…okAtTranslator::translateGameMessage()
b35b98c
to
af763b3
Compare
Tweaked the code so it now properly works in and out of multiplayer, the original code that i had which checked if I thought the above worked in a slightly different way. |
This PR fixes the rate of viewPort scrolling to make it independent of the game clients FPS, this allows the scrolling to be consistent across the whole FPS range. This also makes the scroll speed setting work properly.
As the cameras movement is updated in the game client and not the game logic, this leaves it tied to the game clients current FPS. This can result in the camera moving faster at higher FPS as it takes the same movement step in each message update.
This helps the most when the FPS is in excess of the original target FPS of 30. And also helps within replays when fast forwarding as the FPS is completely uncapped at this point.