search

henricj / dunelegacy

GNU General Public License v2.0
27 stars 5 forks source link

Fix ingame viewport scrolling #20

Closed juj closed 2 years ago

juj commented 2 years ago

Previously ingame map scrolling was processed inside the doInput() function, which meant that each received SDL event (input or otherwise) would trigger an accumulation of a viewport scroll step.

This caused viewport scrolling to behave erratically and often jump around very large quantas.

Fix the issue by moving scroll handling into the main game loop, where a 60msec timer is used to pace scrolling to occur at a regular speed. Holding down shift key will result in 3x fast speed scrolling.

juj commented 2 years ago

One way to reproduce the previous viewport scrolling bug is to ingame:

  1. move mouse cursor somewhere around the center of the screen, and the map viewport e.g. at top of the game scene.
  2. be careful not to accidentally move the mouse at all, then press down the keydown button,
  3. observe the screen begins to scroll down at a ~fixed pace,
  4. while the screen is scrolling towards the bottom of the screen, move the mouse a little around the center of the screen
  5. Observe that the viewport immediately jumps downwards by a large amount (even though mouse was nowhere near the viewport edges that should make it contribute to scrolling). This is caused by the influx of mouse events to SDL loop, which resulted in the keyboard initiated scrolling getting processed multiple times.
henricj commented 2 years ago

I haven't documented it anywhere, but I try to use clang-format through the "clangformat" target before checking in anything:

cmake --build . --target clangformat

I don't always remember and different versions of clang-format behave differently (e.g., VS2022 currently ships with LLVM 13 but I've got LLVM 14 on the command line through chocolatey). The PR is fine as is, but a bunch of the lines will get changed the next time I check something in and remember to run that clangformat target (which will format the entire tree based on the LLVM 14 clang-format's interpretation of the tree's .clang-format file).

juj commented 2 years ago

a bunch of the lines will get changed the next time I check something in

That sounds fine. Would you like me to run it on this PR before landing, or is it ok for that to occur the next time you do a commit?

henricj commented 2 years ago

That sounds fine. Would you like me to run it on this PR before landing, or is it ok for that to occur the next time you do a commit?

This seemed like a perfect opportunity for me to try out GitHub's "Allow edits from maintainers" for the first time.

I'll merge as soon as the checks complete (because, of course, they will all complete successfully).