WASasquatch / was-node-suite-comfyui

An extensive node suite for ComfyUI with over 210 new nodes
MIT License
1.15k stars 170 forks source link

Add randomizer to Load Batch Images #297

Closed concarne000 closed 9 months ago

concarne000 commented 10 months ago

Seems pretty inoffensive to add and is a handy addition

WASasquatch commented 9 months ago

Good idea.

Arcitec commented 9 months ago

@WASasquatch The merged code has a bug. It's calculating an image index which can be out of range. See:

>>> import random
>>> random.randint(0, 1)
1
>>> random.randint(0, 1)
1
>>> random.randint(0, 1)
0
>>> random.randint(0, 1)
1
>>> a = ["hey"]
>>> len(a)
1
>>> a[random.randint(0, len(a))]
'hey'
>>> a[random.randint(0, len(a))]
'hey'
>>> a[random.randint(0, len(a))]
'hey'
>>> a[random.randint(0, len(a))]
'hey'
>>> a[random.randint(0, len(a))]
'hey'
>>> a[random.randint(0, len(a))]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IndexError: list index out of range
>>> a[random.randint(0, len(a))]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
IndexError: list index out of range
>>> 

This line is bugged:

newindex = random.randint(0, len(fl.image_paths))

And adding "-1" to len is a potential fix but would require checking for "len = 0" to avoid landing at "-1".

Typically, it's more common to just generate a random float and multiply it by length and then truncate it, which only generates valid offsets. Like this:

>>> a = ["hey", "there"]
>>> random.random() * len(a)
1.7677572859335193
>>> random.random() * len(a)
1.4537541125560896
>>> random.random() * len(a)
0.7901836766272128
>>> int(random.random() * len(a))
1
>>> int(random.random() * len(a))
0
>>> int(random.random() * len(a))
1

So the fixed code is:

newindex = int(random.random() * len(fl.image_paths))

I suggest that @concarne000 makes the pull request with that fix.

Arcitec commented 9 months ago

Nevermind, I take care of it. :)

https://github.com/WASasquatch/was-node-suite-comfyui/pull/307