Bug: Having a progressive item in starting inventory remove all other instance from item pool

neocerber commented 3 months ago

What happened?

While doing other stuff, I discovered that, in the current main (25/03/2024, 1d4512590e0b78355e5c10174a9c6749e1098a72), adding a Progressive item in the starting inventory remove all its instance from the pool for SC2.

What I did:

checkout main
Generate a game with default template Starcraft 2.yaml.txt, except starting with
- Progressive Stimpack (Marine): 1
- Progressive Stimpack (Marauder): 1
- Progressive Protoss Air Armor: 2
Upload the game generated.
Use /collect

Results?

Everything is available, except
- No Marine stimpack
- No Marauder stimpack
- No Protoss Air Armor

What were the expected results?

Having everything including the two Marine stimpack, the two Marauder stimpack and the three Protoss Air Armor.

Note: If I correct the Webtracker to show starting inventory, than I see one Marine stimpack, one Marauder stimpack and two Protoss Air Armor. So, it is really that the other instances are removed.

The problem seems to be in allowed_quantity(), where if name in excluded_items or (...) does not take into account that some items have quantity. So, if a Progressive item is in starting inventory, pew pew, all instance of that item is not in the pool!

(Might have similar problem with other cases) I do not have a clean solution atm since allowed_quantity() is used elsewhere and I am not confident enough that I won't break things.

Software

Local generation

MatthewMarinets commented 2 weeks ago

This is likely fixed by #192, but probably better to check.

neocerber commented 2 weeks ago

@MatthewMarinets I re-runned the test once and the faulty behavior was not present. Thus, it seems to be fixed. Shall I close the Issue now or should we wait for sc2-next to be in AP main?

MatthewMarinets commented 2 weeks ago

Shall I close the Issue now or should we wait for sc2-next to be in AP main?

Probably more a question for @Ziktofel. It likely comes down to how likely it is someone else bumps into this problem on main, wants to report it on gh, but checks open issues but not closed ones. I think that's unlikely (someone will likely post in discord first and get told it's fixed on beta), so I'd personally say to close it.

Ziktofel commented 2 weeks ago

Here, the bugs shall be closed when solved in sc2-next as we've already dealt with the problem and it will be solved by a next feature release.

Ziktofel / Archipelago