OoTRandomizer / OoT-Randomizer

A randomizer for Ocarina of Time.
Other
404 stars 233 forks source link

Empty Adult Trade Items causes program crash #2301

Closed StarlitScarf closed 2 months ago

StarlitScarf commented 2 months ago

Removing all items in the adult trade items selection causes both the web app and the program to fail randomizer generation. Below is the provided error:

Cannot choose from an empty sequence Traceback (most recent call last): File "C:\Users\bayle\AppData\Local\Programs\ootr-electron-gui\resources\app\python\OoTRandomizer.py", line 57, in start main(settings) File "C:\Users\bayle\AppData\Local\Programs\ootr-electron-gui\resources\app\python\Main.py", line 46, in main spoiler = generate(settings) File "C:\Users\bayle\AppData\Local\Programs\ootr-electron-gui\resources\app\python\Main.py", line 114, in generate worlds = build_world_graphs(settings) File "C:\Users\bayle\AppData\Local\Programs\ootr-electron-gui\resources\app\python\Main.py", line 156, in build_world_graphs generate_itempool(world) File "C:\Users\bayle\AppData\Local\Programs\ootr-electron-gui\resources\app\python\ItemPool.py", line 414, in generate_itempool (pool, placed_items) = get_pool_core(world) File "C:\Users\bayle\AppData\Local\Programs\ootr-electron-gui\resources\app\python\ItemPool.py", line 468, in get_pool_core item = random.choice(world.settings.adult_trade_start) File "C:\Python38\lib\random.py", line 290, in choice raise IndexError('Cannot choose from an empty sequence') from None IndexError: Cannot choose from an empty sequence

aofengen commented 2 months ago

This is not a crash. It is intended behavior. The randomizer has to place at least one trade item in the pool - you cannot remove them all.

flagrama commented 2 months ago

It could probably have a friendly error message indicating the issue though.

aofengen commented 2 months ago

I thought that was changed at one point to have a clearer error message - did that get lost?

Maplesstar commented 2 months ago

I believe you're referring to this one, aofengen? https://github.com/OoTRandomizer/OoT-Randomizer/pull/2198 Based on that error message, StarlitScarf is on the exe and this was merged after 8.1 stable.

aofengen commented 2 months ago

Ah makes sense. Thought that had been resolved

StarlitScarf commented 2 months ago

it seems like in certain scenarios it should resolve the issue, but the error message particularly in this settings seed BSA4BGBSL62A6LTDDSAQFQNNKGCBCA8EBAAAASAD6VFQSL39JCFFLAAESBWB5AHLD5AAAAAC6VDASA4APZAJNJNABALEAA provides the seen error and the message provided is generic

Edit: while this is repoduceable in the web config, should i avoid using 8.1 stable as the generator option?

aofengen commented 2 months ago

No, just leave one of the trade quest items in the pool. The randomizer always places at least one - just set the claim check if you don't want do any other part of the trade quest.

This is not an error in the randomizer, just a poor response that has been made better in dev. You can access the dev version via the website if you want - the next stable release will fix this on stable versions, which should be coming soon.

fenhl commented 2 months ago

As mentioned, this has been fixed in #2198. If you can reproduce this error on versions 8.1.16 or above, please let us know.