New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow negative maximum slider values #1022

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@rikardfalkeborn
Contributor

rikardfalkeborn commented May 3, 2017

Previously, the initialization of minimum_value to 0 prevented
setting negative maximum values, since the negative value would
always be less than 0, triggering an assert. Initializing it to the
least int possible avoids this.

This fixes https://gna.org/bugs/?25618.

If someone only calls slider::set_maximum_value() and not slider::set_minimum_value(), this would obviously lead to unexpected results (previous value of 0 was probably a saner default in that aspect), but there are currently no such place in the code base. I pondered whether I should add a function for setting both the min and the max in the same call (it's somewhat awkward to be unable to set minimum unless maximum is already set), but I went for the simpler solution.

Allow negative maximum slider values
Previously, the initialization of minimum_value to 0 prevented
setting negative maximum values, since the negative value would
always be less than 0, triggering an assert. Initializing it to the
least int possible avoids this.

This fixes https://gna.org/bugs/?25618.
@Vultraz

This comment has been minimized.

Show comment
Hide comment
@Vultraz

Vultraz May 10, 2017

Member

Hmmmm... this could be a fix, but I also know @gfgtdf was working on some slider refactoring, so we might not merge this.

Member

Vultraz commented May 10, 2017

Hmmmm... this could be a fix, but I also know @gfgtdf was working on some slider refactoring, so we might not merge this.

@CelticMinstrel

This comment has been minimized.

Show comment
Hide comment
@CelticMinstrel

CelticMinstrel May 10, 2017

Member

In other words, ask @gfgtdf what he thinks, right?

Member

CelticMinstrel commented May 10, 2017

In other words, ask @gfgtdf what he thinks, right?

@gfgtdf

This comment has been minimized.

Show comment
Hide comment
@gfgtdf

gfgtdf May 10, 2017

Contributor

well it's just one line, so i can eaily revert it if it clashes with my work. also my visual studio has suddenly stopped working so it might take a while before progress on my work.

my original plan to fix this issue was to replace the set_mimimun/maximum_value functions with a single set_value_range function that takes 2 parameters.

Contributor

gfgtdf commented May 10, 2017

well it's just one line, so i can eaily revert it if it clashes with my work. also my visual studio has suddenly stopped working so it might take a while before progress on my work.

my original plan to fix this issue was to replace the set_mimimun/maximum_value functions with a single set_value_range function that takes 2 parameters.

@rikardfalkeborn

This comment has been minimized.

Show comment
Hide comment
@rikardfalkeborn

rikardfalkeborn May 10, 2017

Contributor

I agree that a set_value_range function would be a better solution, so this can be closed (with out merging it).

Contributor

rikardfalkeborn commented May 10, 2017

I agree that a set_value_range function would be a better solution, so this can be closed (with out merging it).

@CelticMinstrel

This comment has been minimized.

Show comment
Hide comment
@CelticMinstrel

CelticMinstrel May 12, 2017

Member

New issue link is #1641, though I'm not sure if it matters if this is just going to be closed.

Member

CelticMinstrel commented May 12, 2017

New issue link is #1641, though I'm not sure if it matters if this is just going to be closed.

@rikardfalkeborn

This comment has been minimized.

Show comment
Hide comment
@rikardfalkeborn

rikardfalkeborn Oct 30, 2017

Contributor

e9c6dff fixed this problem (and some more).

Contributor

rikardfalkeborn commented Oct 30, 2017

e9c6dff fixed this problem (and some more).

@rikardfalkeborn rikardfalkeborn deleted the rikardfalkeborn:allow-negative-slider-maximum-values branch Oct 30, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment