Page MenuHomeWildfire Games

[CSlider] On click move the slider-button to the mouse position
Needs ReviewPublic

Authored by Imarok on Sep 1 2018, 11:13 AM.

Details

Reviewers
vladislavbelov
Trac Tickets
#2593
Summary

If the user clicks somewhere on the slider, the slider-button immediately moves to the mouse position. This allows changing slider values faster.

Test Plan

Open Settings and click on some slider.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
Lint OK
Unit
No Unit Test Coverage
Build Status
Buildable 6884
Build 11293: Vulcan BuildJenkins
Build 11292: arc lint + arc unit

Event Timeline

Imarok created this revision.Sep 1 2018, 11:13 AM
Vulcan added a subscriber: Vulcan.Sep 1 2018, 11:14 AM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/differential/725/display/redirect

Vulcan added a comment.Sep 2 2018, 8:44 PM

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

Link to build: https://jenkins.wildfiregames.com/job/differential/729/display/redirect

vladislavbelov requested changes to this revision.Dec 28 2018, 2:19 AM
vladislavbelov added inline comments.
source/gui/CSlider.cpp
92

It'd be good to avoid the code duplication. Especially if we can change the logic: D406.

This revision now requires changes to proceed.Dec 28 2018, 2:19 AM
Imarok marked an inline comment as done.Jan 3 2019, 2:01 AM
Imarok added inline comments.
source/gui/CSlider.cpp
92

About which lines are you talking? 83 and 84?
If so, what about creating a new function like void changeValue(float difference) for those two lines??

vladislavbelov added inline comments.Jan 13 2019, 8:03 PM
source/gui/CSlider.cpp
1

Bump year.

92

81-84 and 99-103 (probably something else too). I'm thinking about like following:

// Private and probably more descriptive name.
float GetRatio() const
{
	 return (m_MaxValue - m_MinValue) / (m_CachedActualSize.GetWidth() - m_ButtonSide);
}

void AddValue(float difference)
{
	m_Value = Clamp(m_Value + difference, m_MinValue, m_MaxValue);
	UpdateValue();
}

// Example:
m_Mouse = GetMousePos();
m_IsPressed = true;
float difference = float(m_Mouse.x - GetButtonRect().CenterPoint().x) * GetRatio();
AddValue(difference);
93

(GetButtonRect().right + GetButtonRect().left) * 0.5 should be replaced with GetButtonRect().CenterPoint().x.

Imarok updated this revision to Diff 7353.Jan 16 2019, 12:15 AM
Imarok marked an inline comment as done.

Deduplicate code by adding two functions

Imarok updated this revision to Diff 7354.Jan 16 2019, 12:19 AM

Use CenterPoint()

Imarok marked 4 inline comments as done.Jan 16 2019, 12:20 AM
Imarok added inline comments.
source/gui/CSlider.cpp
93

Nice, didn't knew that exists :)

Imarok marked an inline comment as done.Jan 16 2019, 12:20 AM

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

Linter detected issues:
Executing section Source...

source/gui/CSlider.h
|   1| /*·Copyright·(C)·2018·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2018"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/974/

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

Linter detected issues:
Executing section Source...

source/gui/CSlider.h
|   1| /*·Copyright·(C)·2018·Wildfire·Games.
|    | [NORMAL] LicenseYearBear:
|    | License should have "2019" year instead of "2018"
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/differential/975/

Imarok updated this revision to Diff 7447.Mon, Feb 4, 8:30 PM

Fix license header year

Vulcan added a comment.Mon, Feb 4, 8:33 PM

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

Link to build: https://jenkins.wildfiregames.com/job/differential/1036/

vladislavbelov added inline comments.Sat, Feb 16, 5:53 PM
source/gui/CSlider.cpp
75

Let's replace these lines and similar below by IncrementallyChangeValue.

source/gui/CSlider.h
59

I suggest to move these functions under the protect section. As we already have similar functions there.