andrewgu / ModConfigMenu

XCOM 2 Mod Config Menu: a project to build a shared settings menu for Xcom 2 mods.
11 stars 13 forks source link

Sliders Can't Reach Defined Minimum Value #20

Closed Patrick-Seymour closed 8 years ago

Patrick-Seymour commented 8 years ago

Creating a slider with min/max/step of 0/100/1. Accessing value via MCM_API_Slider.GetValue(). Log shows that the range is between exactly 1.0000 and 100.0000, along with integers between these two points. Presumably the "min/max" values are inclusive, so it would be nice if "0" was a value that could be reached with the slider, otherwise I'd have to make the range (-1)-100 to get a usable range of 0-100.

As an aside, might also be nice to have the value displayed next to the slider on its left (would even make viewing this issue easier). There's a big gap between the horizontally-scrolling label text on the left and the slider's left-hand side arrow on the right, with apparently nothing displayed between these two things.

andrewgu commented 8 years ago

Oops! Yeah, will fix for 1.0

andrewgu commented 8 years ago

Fixed, also added a numeric display with an accompanying API change to give you the ability to format the slider's values to your preference. This is explicitly provided because the slider bar has limited resolution and does not evenly divide all intervals, so the value in the slider might not be an integer (or any other pattern you require). Default behavior is to show the value rounded to the nearest integer.

There's a weird problem with UISlider.SetStepSize that is mostly mitigated by a magical incantation, but I'm going to file a separate issue for it.

Patrick-Seymour commented 8 years ago

Not sure whether to put this here or in a new issue, but this seems appropriate.

Not sure if this is intentional (not supporting negatives in sliders) but trying to use negative values doesn't seem to work well at all. Here's some examples:

https://www.dropbox.com/s/svkatfh0v2d7wx8/mcm-wrong-negatives-and-last-all.png?dl=0

Basically this was two tests, top two images are one test and bottom two images are another test. The text on the right is the output of GetValue() on the corresponding slider elements.

It's super weird! It looks like the default positioning of the slider "nub" within the slider instance is actually correct, taking in to account the negative range. Eg, (-100)-0 range with a value of (-9) puts the nub about 90% of the way to the right. But as soon as you touch the nub, the perceived range of the slider changes to 0-100. A range of (-100)-100 looks correct when the slider instance is first instantiated, but then as soon as you touch the nub, it changes to a perceived range of 0-200.

The negative values are also not displaying correctly. They should be "-10" when in fact are displaying as "-9". But the GetValue() function is returning the correct value.

It looks like "step" is ignored. In all cases, the step size was exactly 1, but the values being returned by the slider all have floating points. Which raises an interesting question in itself: Are you designing these sliders to support floating ranges (eg 0.01-0.02 with a step of 0.001)? Or negatives?

Also of note, the last slider in a group doesn't return anything but zero. Must be related to the SetEditable(false) not working on the last slider in a group.

andrewgu commented 8 years ago

The slider has a weird problem of having an actual width of 99, not 100. This is a side effect of the underlying control going from 1 to 100 instead of 0 to 100. The net result is that your "steps" end up being off by floating point errors. Why it was implemented that way, beats me.

andrewgu commented 8 years ago

Just pushed a new update, should fix your slider range problem. It doesn't fix the precision problem, which is still coming from the fact that the slider's "range" is 99 units wide.

The problem is that the built-in slider looks like it operates like an integer, whereas the slider in MCM_API_Slider simulates a floating point slider where you round the numbers yourself. More flexibility that way.

In the future, it's probably worth reimplementing the slider by hand if only to get around this dumb 99.0 wide issue.

BlueRaja commented 8 years ago

The range of [1,100] is 100, not 99 ([0,100] would have a width of 101)

andrewgu commented 8 years ago

[1-100] has 100 distinct integer values and the interval is 99 units wide, which means that if you're trying to map [1,100] to [m,n], the formula to convert the slider position is f(p) = m + (n-m)*((p-1)/99)

If it were [0,100] it would have 101 distinct values and the interval would be 100 units wide, making the formula f(p) = m + (n-m)*(p/100)

That's the source of the floating point issues that @Patrick-Seymour was running into trying to get the slider to do [0,100].

Patrick-Seymour commented 8 years ago

Slider ranges work much better now! I can still see one minor issue though: Looks like when the sliders are first initialized, if one is using the value "0" it displays incorrectly until the slider is touched.

https://www.dropbox.com/s/simqlkj5109zop2/mcm-sliders-almost-starting-values.png?dl=0

Top two 0-100 ranges; bottom two (-100)-0 ranges. Rows 1 and 3 are initialized to value 0; rows 2 and 4 are initialized to values 15 and -15. The third row should show the nub all the way to the right, as that is where the zero position on that slider is.

It's a pretty dang small issue in the grand scheme of things.

Might be that somewhere if the value is literally "0", it just defaults to the left side instead of calculating where the position should be.

andrewgu commented 8 years ago

I really hope that's an error in my code, because I'm sick of UISlider's quirks already.

andrewgu commented 8 years ago

Yup, turns out it's the same mysterious problem as Issue #22. What's actually happening is that it's punting you to one slider step above the leftmost value. When I change step size to a bigger number like 20, it'll default to slightly away from the left edge. Sometimes.

I'm going to close this issue since the remaining issue is now a duplicate of Issue #22.