search

chrisgoringe / cg-controller

MIT License
30 stars 3 forks source link

[Bug] Value differences between Controller and node #250

Closed LukeG89 closed 2 weeks ago

LukeG89 commented 2 weeks ago

It's happening the same issue reported here: https://github.com/chrisgoringe/cg-controller/discussions/194#discussioncomment-11026738

ksampler latent

JorgeR81 commented 2 weeks ago

What ranges and step sizes did you use ?

LukeG89 commented 2 weeks ago

Same range in both nodes and 5 step in KSampler and 64 step in Latent (but I think you just need to put a number >1)

JorgeR81 commented 2 weeks ago

Ah yes, for this to work, the min./max. values must be multiples of the step size. The min. value can't be smaller than the step size ( for positve ranges ).

Like this:

50 rg2

rg1

Maybe it could be easier and more reliable to have this restriction in the slider settings, instead of trying to adjust widget values.

Because these adjustments are a bit weird anyways. For instance, with a step size of 5 we need to go from 1 to 5. But in then, we only have a step size of 4.

So, in the slider edit menu, the input boxes could turn red indicating invalid values. Like they do now, for other values. And a pop up could show up indicating this rule. It would be a litter harder for the user, but we would also be helping the user to create better and more prediable sliders.

red

LukeG89 commented 2 weeks ago

Fixed!

JorgeR81 commented 2 weeks ago

If we are going to do these automatic adjustments, wouldn't it make more sense to do it in the first step, instead of the last.

Because most of these will be caused by the need to use 1 instead of 0, for the min value ( e.g. KSampler steps ).

Now if I set min(1), max(50), step(5) then I get: 1, 6, 11, ... 46, 50

But it would be more useful to have: 1, 5, 10, ... 45, 50

chrisgoringe commented 2 weeks ago

The rounding is done by the widget. While we might override, it's more likely to cause the value mismatches that we have seen than accepting it.

Suggest you raise a new issue to look into it, but won't be changed again soon.

On Thu, 31 Oct 2024 at 10:13, JorgeR81 @.***> wrote:

If we are going to do these automatic adjustments, wouldn't it make more sense to do it in the first step, instead of the last.

Because most of these will be caused by the need to use 1 instead of 0, for the min value ( e.g. KSampler steps ).

Now if I set min(1), max(50), step(5) then I get: 1, 6, 11, ... 46, 50

But it would be more useful to have: 1, 5, 10, ... 45, 50

— Reply to this email directly, view it on GitHub https://github.com/chrisgoringe/cg-controller/issues/250#issuecomment-2448644227, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABBLMAYB3LOSJBQWIGRPGYDZ6FR3DAVCNFSM6AAAAABQ4TKXKSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDINBYGY2DIMRSG4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

JorgeR81 commented 2 weeks ago

While we might override, it's more likely to cause the value mismatches that we have seen than accepting it.

If it can cause more issues, then it's probably better to leave it like this.

I personally don't mind it, because I always use the "rule" I mentioned above: ( the min./max. values must be multiples of the step size ).