Add a logarithmicly scaled slider to the options
Changes PlannedPublic
Actions

Authored by bb on Oct 16 2020, 11:40 PM.

Details

Reviewers: None

Summary

Implicitly suggested by elexis in D1493.

Removing a cslider function, which didn't do what was in the documentation and called only once and which isn't that useful anyhow.
Adding some debug/fallbacks for invalid value combinations.

Test Plan

Agree sliders should have this support
Add a logarithmic slider to the options

Diff Detail

Repository

rP 0 A.D. Public Repository

Branch

/ps/trunk

Lint

Lint OK

Severity	Location	Code	Message
Warning	binaries/data/mods/public/gui/options/options.js:122	ESLintBear (indent)	ESLintBear (indent)
Warning	binaries/data/mods/public/gui/options/options.js:123	ESLintBear (indent)	ESLintBear (indent)
Warning	binaries/data/mods/public/gui/options/options.js:124	ESLintBear (indent)	ESLintBear (indent)
Warning	binaries/data/mods/public/gui/options/options.js:125	ESLintBear (indent)	ESLintBear (indent)
Warning	binaries/data/mods/public/gui/options/options.js:126	ESLintBear (indent)	ESLintBear (indent)
Warning	binaries/data/mods/public/gui/options/options.js:241	ESLintBear (no-shadow)	ESLintBear (no-shadow)

Unit

No Unit Test Coverage

Build Status

Buildable 13352
Build 26788: Vulcan Build	Jenkins
Build 26787: Vulcan Build (macOS)	Jenkins
Build 26786: Vulcan Build (Windows)	Jenkins
Build 26785: arc lint + arc unit

Event Timeline

bb created this revision.Oct 16 2020, 11:40 PM

bb added inline comments.

binaries/data/mods/public/gui/options/options.js
158	One can nuke the default when setting it to all options
source/gui/ObjectTypes/CSlider.cpp
54	Maybe ENSURE is better
source/gui/ObjectTypes/CSlider.h
60	Maybe an own datatype (like EAlign) is better

Successful build - Chance fights ever on the side of the prudent.

builderr-release-macos.txt
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgraphics.a(precompiled.o) has no symbols
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/ranlib: file: ../../../binaries/system/libgui.a(precompiled.o) has no symbols

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/1657/display/redirect

Harbormaster completed remote builds in B13352: Diff 13641.Oct 16 2020, 11:51 PM

bb requested review of this revision.Oct 16 2020, 11:51 PM

GetSliderRatio did exactly what the comment said, you might think that the value is the misleading name though.

What's a usage of the feature?

It seems it makes adding steps a bit harder.

In D3039#133487, @vladislavbelov wrote:

GetSliderRatio did exactly what the comment said, you might think that the value is the misleading name though.

@return ratio between the value of the slider and its actual size in the GUI value is clearly wrong, whether you call it misleading or whatelse, misleading comments are just as much telling lies about the code.

What's a usage of the feature?

Wonders in D1493

It seems it makes adding steps a bit harder.

Not harder than adding the logarithmical steps

In D3039#133500, @bb wrote:

@return ratio between the value of the slider and its actual size in the GUI value is clearly wrong, whether you call it misleading or whatelse, misleading comments are just as much telling lies about the code.

I suppose it means m_MaxValue - m_MinValue, but I don't mind anyway.

What's a usage of the feature?

Wonders in D1493

Does the std::log work good for all cases? It seems it needs adjustments (how the curve behaves).

Not harder than adding the logarithmical steps

Actually logarithmical steps are even messier because of precision, you can't represent some integer values in floats (in case we convert them to JS Number)

Maybe we should allow arbitrary scales from the JS somehow

Needs rethinking.