Skip to content
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

Range: Fix cases where max was set to or below min value #33908

Merged
merged 1 commit into from
Nov 26, 2019

Conversation

akien-mga
Copy link
Member

It will now raise an error whenever this happens so that we can fix
these situations. max == min is not allowed as it could lead to
divisions by zero in ratios, and max < min doesn't make much sense.

Fixes #33907.

It will now raise an error whenever this happens so that we can fix
these situations. `max == min` is not allowed as it could lead to
divisions by zero in ratios, and `max < min` doesn't make much sense.

Fixes godotengine#33907.
@akien-mga akien-mga added this to the 3.2 milestone Nov 26, 2019
@akien-mga akien-mga merged commit a698fd1 into godotengine:master Nov 26, 2019
@akien-mga akien-mga deleted the range-fix-max-errors branch November 26, 2019 09:51
@@ -100,6 +100,7 @@ void Range::set_value(double p_val) {
shared->emit_value_changed();
}
void Range::set_min(double p_min) {
ERR_FAIL_COND_MSG(p_min >= shared->max, "Range cannot have min value higher or equal to its max value.");
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This wasn't a very good idea as it puts too strict requirements on how to set min and max values. For example, since the default min and max are 0 and 100, this triggers an error:

set_min(256)
set_max(16384)

Since min will be higher than max temporarily. It can be worked around by setting max first, but it's not really intuitive. I'll relax the requirement as it's only a problem in get_as_ratio, which already has a check.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed with 966c68b.

akien-mga added a commit that referenced this pull request Nov 26, 2019
This wasn't a very good idea as it puts too strict requirements on how
to set `min` and `max` values. For example, since the default min and
max are 0 and 100, this triggers an error:

```
set_min(256)
set_max(16384)
```

Since `min` will be higher than `max` temporarily. It can be worked
around by setting max first, but it's not really intuitive. I'll relax
the requirement as it's only a problem in `get_as_ratio`, which already
has a check.

Fix another min == max occurrence.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Cannot get ratio when minimum and maximum value are equal" error at run time
1 participant