Page MenuHomeWildfire Games

responsive hotkey scrollspeed
ClosedPublic

Authored by nwtour on Feb 24 2021, 11:59 PM.

Details

Summary

Changing the value "scroll speed" is not intuitive. Nothing is displayed on the screen, indicating that hotkey is worked
Patch types the value to js console

Test Plan

.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

nwtour requested review of this revision.Feb 24 2021, 11:59 PM
nwtour created this revision.
nwtour updated this revision to Diff 16073.Feb 25 2021, 12:16 AM

Full context + screenshot

Good idea πŸ‘

If you can, I would suggest using "Translate" around the string so that it gets translated.
I think "increased/decreased" would be more natural, but I'm not 100% sure.

nwtour updated this revision to Diff 16076.Feb 25 2021, 9:55 PM
  • Update year
  • Value now natural digit
  • String can be translated
nwtour updated this revision to Diff 16402.Mar 11 2021, 9:43 PM

sync with master

Why not all camera speed changes are printed?

build/premake/premake5.lua
782 β†—(On Diff #16402)

Is it required?

source/graphics/CameraController.cpp
685 β†—(On Diff #16402)

It'd be good to print at least one digit after the point. It's not a fixed point number, so x + y - y might be not equal to x. And with rounding it's unclear.

nwtour updated this revision to Diff 16410.Mar 12 2021, 8:26 AM

added zoom and rotate speed
translate smoke test passed

nwtour updated this revision to Diff 16411.Mar 12 2021, 8:31 AM

to %0.1f

build/premake/premake5.lua
782 β†—(On Diff #16402)

CameraController.cpp
In file included from ../../../source/i18n/L10n.h:28,

from ../../../source/graphics/CameraController.cpp:24:

../../../source/lib/external_libraries/tinygettext.h:36:10: fatal error: tinygettext/tinygettext.hpp: No such file
#include <tinygettext/tinygettext.hpp>

^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
vladislavbelov added inline comments.Mar 12 2021, 9:58 AM
build/premake/premake5.lua
782 β†—(On Diff #16402)

I removed tinygettext from the public scope in rP25041. Try with the latest svn please.

nwtour updated this revision to Diff 16416.Mar 12 2021, 10:42 AM

Remove premake5.lua change
Build successfull and smoke test passed

This revision was not accepted when it landed; it landed in state Needs Review.Mar 12 2021, 9:57 PM
This revision was automatically updated to reflect the committed changes.
Owners added subscribers: Restricted Owners Package, Restricted Owners Package.Mar 12 2021, 9:57 PM

Few notes after testing:

  • Unfortunately L10n still depends on icu, so I added it to premake to not break --without-pch
  • Missed include of ps/CLogger.h
  • m_ViewRotate*Speed should have been printed for both components: X and Y

The rest looks good.

  • m_ViewRotate*Speed should have been printed for both components: X and Y

I have never seen AAA-games where when changing a parameter it said "Rotate speed decreased to X = 1.025, Y = 1.328".
The author of the patch should decide for himself whether he creates a debug function or improves the game experience.
Please give a response time next time.

The rest looks good

Thanks for your time

I have never seen AAA-games where when changing a parameter it said "Rotate speed decreased to X = 1.025, Y = 1.328".

The code changes both parameters, so it should print both. Else it's misleading a player. I'm not sure that there're AAA games with detailed logs - they're too technical for a player. Usually all such adjustments are done by sliders in settings.

The author of the patch should decide for himself whether he creates a debug function or improves the game experience.

I'm not sure that I get what do you mean.

Thanks for your time

Thanks for the patch!