resolution_fixed type_patch (an actual patch, not a request for one) | by Forgon

This patch series introduces 7 new configuration options among minor changes that are related to them. Overall there are 15 commits, which should be tested in the order in which they are attached, since some of them depend on each other. A few words need translation into foreign languages, to be done by people other than me.

Videos demonstrating each new option except cameraAccel are compiled in this YouTube playlist. Unfortunately, it shows the string "Audio and Zoom Options", which was too long for the minimum screen resolution of 640x480 pixels (see attached image [raw-attachment:audio_and_zoom_options_old.png]) and thus replaced with "Audio / Zoom Options".

For reviewers, all commits of this ticket are joint in one file (view [raw-attachment:ticket_4629_all_15_commits.patch]).

I carefully split my contributions into many parts because

• squashing commits is trivial, splitting a commit is often not
• it eases debugging
• it helps reviewers to understand the code

Fix variable setting in "Options" menus

Setting non-boolean options, such as "Language" in "Game Options", was broken:

To distinguish between left and right mouse button, the program checked whether the right mouse button had been released with the function 'mouseReleased()', which returns the mouse button state found within the last frame by the function 'inputNewFrame()' after mouse events have already been processed in function 'inputHandleMouseButtonEvent()', so that events can be missed (see './lib/sdl/main_sdl.cpp'). If the right mouse button had not been released, it was assumed the left button had been pressed. This meant that right-clicks following each other within a short interval were confused with left-clicks, so that a user intending to quickly cycle backwards through option values could accidentally cycle forwards.

Using the function 'widgGetButtonKey_DEPRECATED()' defined in './lib/widget/widget.cpp' and shared by the header file './lib/widget/widget.h' instead of 'mouseReleased()' solved the problem.
There may well be many other opportunities in the source code to replace frame-dependent input recognition with proper event handling, but I was too wary of risking new bugs to investigate this.

In "Video Options", vertical synchronisation could not be reenabled unless it was functional since commit 195159406ddbd0c1cd70d13aba54938903a39eee. This has caused problems for some Linux users but had to be changed anyway, because vsync

• is enabled by default, so that the user must be confused by not being allowed to reenable it
• takes its initial displayed value in "Video Options" (default: "On") from its configuration file setting, which is another source of confusion
• is not set to false in the configuration file if dysfunctional, which is yet another source of confusion
• can always be reset in the configuration file
• should rather issue a non-intrusive warning recognisable to a normal user

Besides, I removed an empty function 'closeConfig()'. I caused a linker error by removing code (commit 88c1e9c9f3beb10f1e747f7854d384f076b8cf76), so I should not claim it is useless. If it has a purpose, you might add a helpful comment above it.

=== Configuration options mapZoom, mapZoomRate and radarZoom

mapZoom changes the starting height of the camera

• values range from 0 to 10000 (previously 500 to 5000)
• default value STARTDISTANCE (previously called START_DISTANCE) increased from 2000 to 2500
• savegames <= 10 keep starting at 1500 height, defined in OLD_START_HEIGHT (previously called START_HEIGHT)
• set in steps of mapZoomRate

mapZoomRate changes zoom speed

• default value 250 (previously 1000/3)
• values range from 50 to 500

With the scrollwheel, one turn of the scrollwheel will change the zoom by exactly the value of 'mapZoomRate'. The key bindings for 'Zoom in' and 'Zoom out' are less precise.

radarZoom changes the size of the minimap

• values continue to range from 8 to 64 in steps of 4

Most good players zoom into the minimap and out of the map once a game starts. Expect that almost everyone will use these options once they are released.

Minor changes related to this:

• removed a line in './src/game.cpp' that set radar zoom in function 'gameLoadV()' because it is also set in function 'loadGame()'. Please refactor this further, it looks as though identical code is run twice!
• display an info message atop of the screen when radar/map zoom is changed ingame through mouse/keyboard (see attached image [raw-attachment:zoom_info_message.png]). It is important that the messages are only shown in this situation, otherwise they would also appear at the beginning of every (save-)game

In the main menu, "Audio Options" were renamed to "Audio / Zoom Options", where all three options can now set interactively (see attached image [raw-attachment:audio_and_zoom_options_new.png]). To assure no invalid values can be displayed or set there, I altered the template 'stepCycle()' which is now called 'seqCycle()' and takes its argumenst similar to seq(1) in coreutils.

=== Configuration options cameraSpeed (previously called scroll), cameraAccel, radarJump and scrollEvent

cameraSpeed sets the speed of horizontal movement on the map, e.g. when using arrow keys

• replaces scroll (labelled "Scroll Speed" in game options), the name of which was misleading
• values range from 100 to 5000 (default 2500), set in steps of 100
• values denote the maximum speed and no longer the mere acceleration rate
• during first 2 seconds of movement, the camera accelerates to maximum speed
• for 1 second after movement or less if camera direction is reversed, the camera decelerates to 0
• no longer set via slider, because slider values can only be guessed

Camera behaviour has changed significantly. Previously, the variable scroll had a default value of 800 and took values ranging from 100 to 1600, that denoted acceleration per second, to which 800 were added to calculate the maximum speed. The attached image [raw-attachment:old_camera_speed.png] should help to understand this.

This formula did not scale well (see [raw-attachment:old_scroll_table.csv]): With scroll set to 100, it took 9 seconds for the camera to achieve maximum speed, which, after the user had ceased movement, was followed by 4.5 additional seconds of deceleration. This made it difficult to aim at a precise location, and often the camera would arrive at a further distance than intended.

Now scroll is obsolete and has been replaced with cameraSpeed, the name of which I find less misleading. Its value denotes maximum speed only, and its range of values has been greatly extended so that any bad mouse/touchpad configuration can be compensated for. The default maximum speed has been increased from 1600 to 2500.

Acceleration and deceleration should neither be inconsistently nor overly long. Assuming that arrow keys will usually be pressed for 3-5 seconds, I introduced an acceleration time of (up to) 2 seconds followed by (up to) 1 second of deceleration regardless of camera speed. These times are identical to those used by the old scroll setting with its default value of 800.

View [raw-attachment:new_camera_speed_with_accel_true.svg] for an overview of all new possible settings. A PNG version can be found here, which Trac mysteriously rejects to display properly. To contrast the old scroll variable with the new cameraSpeed variable, look at [raw-attachment:camera_speed_settings.png].

A new button in "Game Options" allows setting this option interactively (see attached image [raw-attachment:game_options.new.png]). In its place, a slider was previously used to set the variable scroll. It had the disadvantage that the user could not see what value he picked and was moreover set to 100 in both its lowest and second-lowest setting.

It could be mentioned during the next release that the config file variable scroll is now obsolete, although I suspect most players never knew what it did.

To measure and record in-game camera speed, I created:

• a patch that logs it to the file camera_speed.csv inside the configuration directory. Two versions exist: For recordings before this patch series (use [raw-attachment:old_scroll_speed_logger.patch]) and after this patch series (use [raw-attachment:new_camera_speed_logger.patch]). Physfs >= 2.1 is required for either of them.
• a Gnuplot script to generate a graph (PNG or SVG) from the file camera_speed.csv (see [raw-attachment:gnuplot_speed_record])
• a bash script to reproducibly test camera speed by pressing arrow keys for specific time intervals (see [raw-attachment:record_camera_speed]). Requires xdotool.

The charts attached to this ticket were created by

• applying the newer patch file to enable logging
• a bash script wrapping ./record_camera_speed (see [raw-attachment:record_camera_speeds]) to generate data, which can be found inside [raw-attachment:camera_speeds.zip]
• a Gnuplot script (see [raw-attachment:gnuplot_speed_records]) to create various charts with the data

When recording, mind that the camera keeps its own time. This has consequences:

• recordings within the first two seconds of a game are broken (see [raw-attachment:camera_speed_glitch.png])
• all recordings start with an offset of 0.5 seconds (see './src/display.cpp')

Besides, I abolished the useless variables xDiffFrac and yDiffFrac introduced in commit 2249996f1c7779bb9f6b3be82d6ae207c5aa69f5 that played a minor role in calculating camera speeds.

cameraAccel can disable camera acceleration

• camera acceleration remains enabled by default
• if disabled, camera speed is halved without altering the cameraSpeed variable
• if disabled, the string " / 2" is appended to cameraSpeed values in "Game Options"

To increase the accuracy of the camera, its acceleration can be disabled. Besides starting and stopping the camera without delay, disabling this option halves its speed. The effect of this should be obvious after looking at [raw-attachment:camera_acceleration.png].

The full range of camera speed values then remains available, albeit twice as slow (view [raw-attachment:new_camera_speed_with_accel_false.png]). To indicate that, in "Game Options" the string " / 2" is printed after cameraSpeed (view [raw-attachment:game_options_new_without_camera_acceleration.png]).

radarJump determines camera behaviour when right clicking on the minimap. Possible values:

• Instant (true): Instantly jump to the targeted location. Experienced players navigate via minimap and they should like this feature.
• Tracked (false): Slowly pan to the target while shaking the camera (default)

A new button in "Graphics Options" allows setting this option interactively (see attached image [raw-attachment:graphics_options.new.png]).

scrollEvent changes the function of the scroll wheel. Possible values:

• 0: set map/radar zoom (default)
• 1: set game speed
• 2: set camera pitch (rotation around x-axis)

A new button in "Mouse Options" allows setting this option interactively (see attached image [raw-attachment:mouse_options.new.png]).

Minor changes related to this:

• abolish obscure key mapping 'Left Alt + Left Ctrl + Scroll' to change game speed (obsolete through scrollEvent)
• abolish pressing 'Left Alt + Middle Mouse' as an obscure alternative to right-clicking on the minimap

=== Prettier "Options" menus

The "Game Options" string "English (United Kingdom)" is too long for the layout (see attached image [raw-attachment:english_uk_layout_error.png]), so I changed it to "British English".

All game options are re-aligned along the same lines with a bit more space for text as a consequence. The attached image [raw-attachment:options_overview.png] shows all modified menus before this patch above their improved counterparts below with red lines indicating where text is to be aligned.

Issue migrated from trac:4629 at 2022-04-16 12:46:23 -0700