SelinaDev / Godot-Roguelike-Tutorial

Yet Another Roguelike Tutorial in Godot
MIT License
78 stars 10 forks source link

Possible bug in weighted picking code #8

Closed pkillthetoy closed 7 months ago

pkillthetoy commented 7 months ago

I think _pick_weighted() in dungeon_generator.gd is bugged.

The chance being added to the cumulative_chances array is not actually cumulative. It's just the same value as in the weighted_chances dict being passed in, so you can end up with values like [80, 15] (for orc, then troll) instead of the desired [80, 95]

mappes commented 7 months ago

I noticed an issue with this too. At least for me it also seemed to favor the first item in the weighted dict. Even though other items should be spawning it only spawns the particular item, for example health potion. And if you go deep down in the dungeon it stops spawning any items at all.

I somehow managed to fix it so that it works as intended with following change to the last for loop in _pick_weighted() function:

var cumulative_sum: int = 0
for i in cumulative_chances.size():
    cumulative_sum += cumulative_chances[i]
    if cumulative_sum > random_chance:
        selection = keys[i]
        break
pkillthetoy commented 7 months ago

My fix occurs earlier. When I'm adding things to the cumulative_choices array, I have this instead:


    var sum :int = 0
    for key in choices: 
        keys.append(key)
        var chance : int = choices[key]
        sum += chance
        cumulative_chances.append(sum)```
SelinaDev commented 7 months ago

You were right, I messed up in calculating the sum but not actually using it. Thank you very much for trying the code that doesn't even have a tutorial yet, it's very helpful for me.