[PATCH] Volume control slider

[Damien PIQUET]

Use a slider to control the sound volume.

[Damien PIQUET]

Resize & revert changes fixes; corrected class description (reported by Imarok and s0600204)

[mimo]

nice work! Here are a few quick comments (without proper review of the code):

• you must add a way to display the selected value of the slider (maybe in a tooltip ?), and the min and max could be displayed too.
• it seems that your slider implementation is a ratio between 0 and 1? but that's not always wanted: the default gains were all between 0 and 1, but I'm not sure that these were supposed to be relative gain vs the maximum gain (in particular the master gain, but someone knowing the sound system should comment), and there are other places where we could use a slider not necessarily between 0 and 1. Ideally, we should have min/max values of the slider in options.json, and these values would be used to define the min/max of the slider or to make a conversion between the value between 0 and 1 returned by the slider, and the output of the slider which should be between min and max.
• futhermore, as the slider should never return a value outside its min/max range, lines 166-169 seem useless.

[elexis]

#2593 points out other desired features, but I wouldn't mind if we get basic slider support now and add more in that ticket if you can't implement all things easily.

Before comitting you should IMO at least address these points:

• Agreeing with mimo that one should be able to pass min and max to the slider, so that it's versatile and easily usable. If that computation isn't done in C++ it would have to be done in JS which would likely end in code duplication.
• Reuse the caption field for the value like other GUI object types do.
• Those lines mimo mentioned indeed some redundant.
• Displaying the min/max/current value is likely irrelevant for this ticket, if it's going to be a 0-100% thing anyway. It would become more important when one can select values from a given range.

Coding style:

• L99 first check MouseOver then !MouseOver in the else part, also that one pair of braces is not needed
• Draw a sprite at the % position comment not really needed as it states the same as the code
• // Cursor should 5% of it's parent with a max ? didn't understand that one

[elexis]

Backlogging due to lack of progress.

[Vladislav Belov]

Any progress?

[fabio]

(refs #3737)

[Vladislav Belov]

Current slider works with max/min values, uses the checkbox button image.

[Vladislav Belov]

Another style:

[Vladislav Belov]

Improved patch by dpiquet

[wraitii]

Looks good. I don't think the "another style" works for our interface, but the first is OK. However the knobs don't go all the way to the left and I get an error when I open the options screen (something about width).

[Vladislav Belov]

Added missed width, used the first design, buttons move to ends of both sides

[Vladislav Belov]

Currently the upper bound of sound gain is 10.0: ​http://trac.wildfiregames.com/wiki/Manual_Settings. But 1.0 is normal sound gain (not increased, not decreased), so probably, make the upper bound in [1.0, 2.0], because user with normal settings will have a problem to make gain in [0.5, 1.0], also the default gain value is 0.5.

[echotangoecho]

While you're at it, IMO, the "Gain" part of the sound settings names should be renamed "volume".

[Vladislav Belov]

Replying to echotangoecho:

While you're at it, IMO, the "Gain" part of the sound settings names should be renamed "volume".

I agree, but in a separated commit.

[wraitii]

In 19479:

Add sliders to the GUI. Use them for the sound/music volume controls.

Patch by vladislavbelov, based on a patch by dpiquet.

Fixes #4039.
Differential Revision: ​https://code.wildfiregames.com/D325

[elexis]

Thanks for the patch guys!

[elexis]

In 19482:

Use the slider for the number of shader graphics setting.
Thereby remove the only hardcoded reference of options.json from options.js.
Rename Gain to Volume, refs #4039.

Differential Revision: ​https://code.wildfiregames.com/D398
Reviewed By: Vladislav

[elexis]

In 23005:

Change GUI Object Setting values to be members of the IGUIObject inheriting class, rename AddSetting to RegisterSetting.

Improves performance for Draw calls by 3-5% according to a shady benchmark.
Improves memory layout, since the values are not on the heap anymore but in the using class.
Reduces complexity of the implementation and increases type safety.
Allows specifying default values at setting value construction time, refs D2242.
Inspired by Vladislav introducing members that cached GetSetting values in rP19479/D325, refs #4039, rP20075/D763, refs 4225, rP21429/D474, D406, which were formerly proposed to be removed in D2241.

Differential Revision: ​https://code.wildfiregames.com/D2313
Tested on: clang 8.0.1, gcc 9.1.0, Jenkins vs2015
Comments By: Vladislav