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

step is broken. #74

Closed
wayfu opened this issue Jul 4, 2015 · 3 comments
Closed

step is broken. #74

wayfu opened this issue Jul 4, 2015 · 3 comments

Comments

@wayfu
Copy link

wayfu commented Jul 4, 2015

I add two GUIs like so:

var obj1 = { x: 1 };
var gui1 = new dat.GUI();
var controller1 = gui1.add(obj1, 'x', 0, 2, 0.01);
controller1.onChange(function(value) {
  console.log(value);
});

var obj2 = { y: 1 };
var gui2 = new dat.GUI();
var controller2 = gui2.add(obj2, 'y').min(0).max(2).step(0.01);
controller2.onChange(function(value) {
  console.log(value);
});

With notation 1 ("x"), dat.GUI will ignore step completely (as evidenced in the console output). With notation 2 ("y"), step will be respected for value changes, but precision in the NumberControllerBox is wrong.

I followed the path into the dat.GUI code, and I believe it's a problem with the factory which only supplies max and min values to NumberControllerSlider and NumberControllerBox and throws away step. In notation 1, step will never be seen by any controller. In notation 2, the step setter function will adjust the value for the Slider, but the change does not propagate to the Box.

I've crudely added step to the factory like so (argument[4]):

          if (common.isNumber(arguments[2]) && common.isNumber(arguments[3])) {

            // Has min and max.
            return new NumberControllerSlider(object, property, arguments[2], arguments[3], arguments[4]);

          } else {

            return new NumberControllerBox(object, property, { min: arguments[2], max: arguments[3], step: arguments[4] });

          }

This fixes the problem for notation 1 in my example (notation 2 would still need to update the Slider's display NumberControllerBox from the step setter), but I'm sure there need to be checks built around it, and I'm simply not confident enough in my ability to catch everything to submit a PR.

Perhaps you can help?

@makc
Copy link

makc commented Oct 18, 2015

step will be respected for value changes, but precision in the NumberControllerBox is wrong

simple workaround in #77

@wassname
Copy link

Fixed by pull request #47 (if you use step). Not sure when if it will be merged

@customlogic
Copy link
Contributor

Has been fixed in develop branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants