ArchipelagoMW / Archipelago

Archipelago Multi-Game Randomizer and Server
https://archipelago.gg
Other
521 stars 678 forks source link

OSRS: Fixes to Logic errors related to Max Skill Level determining when Regions are accessible #4188

Closed digiholic closed 4 days ago

digiholic commented 1 week ago

What is this fixing or adding?

Addresses issues found in RC testing - Regions that required a skill level to access (ex. Crafting Guild, Cooking Guild, and Canoe tasks) were not properly checking to make sure their requirements were within the Max Skill Level set in the options. This would mean that a player might need to reach 32 Cooking to access an Apple from the Cooking Guild for a Level 30 Cooking Task, even if 30 were the chosen max Cooking Level required.

Additionally, some general cleanup to make rules logic easier to work with, and to fix a few edge cases:

How was this tested?

The easiest situation to force is the Canoe logic. By setting max woodcutting level to 1, we essentially remove the ability to use Canoes. 10 seeds were rolled, and all spoiler logs were checked for any logical paths that took any Canoe route, and none were found. 10 more seeds with a Woodcutting Level of 99 were rolled, and all of them contained at least one Canoe route.

Things like Crafting Guild and Cooking Guild access are more complicated to test since they require more than just a skill level to access, so those will need to be tested with actual runs.

Automated tests were also run multiple times to ensure the Indirect Condition removal did not cause any failures to generate. Indirect Condition related failures were quite rare though, so I cannot be 100% sure they're gone.

NewSoupVi commented 1 week ago

Getting loads of errors on a fully random yaml - Idk if this was always the case because random settings don't work well with this game because they end up being unsolvable / restrictive? I don't feel like it was ever this much though, like every 3rd or 4th fully random seed fails. Well, here are some seeds

Ftr, these hit Fill errors, not one of your designated Exceptions (I hit that too a couple of times, but that's fine)

Old School Runescape.json 59826788302066781595 77746130863581927747 89898869177983739745 68734120406112450527

Exempt-Medic commented 1 week ago

Mentioned it in DM's but just putting it here as well: Cook Some Stew has accessibility problems from Fishing

NewSoupVi commented 6 days ago

95140043061228385161 12545918943402021278 players.zip

Fill.FillError: Not enough locations for progression items. There are 1 more progression items than there are available locations.

Very rare, but this seems very fixable

digiholic commented 4 days ago

95140043061228385161 12545918943402021278 players.zip

Fill.FillError: Not enough locations for progression items. There are 1 more progression items than there are available locations.

Very rare, but this seems very fixable

Still trying to work this out. As far as I can tell, the seed it builds is legitimately unbeatable. I just mapped out the items and locations available and it legitimately cannot place that last item anywhere it can actually be reached at. I think I'll need to find a way to check for this during Task generation and swap out tasks if it fails.

Exempt-Medic commented 4 days ago

Still trying to work this out. As far as I can tell, the seed it builds is legitimately unbeatable. I just mapped out the items and locations available and it legitimately cannot place that last item anywhere it can actually be reached at. I think I'll need to find a way to check for this during Task generation and swap out tasks if it fails.

I'm confused/concerned that it seems to only happen with multiple OSRS players. I did three thousand solo gens and didn't hit this at all, but hit it several times in just 100 2-player gens

NewSoupVi commented 4 days ago

There's something strange going on here. I need to do some investigation.

NewSoupVi commented 4 days ago

This is not your fault. It's a bug with Fill. Even if there are no locations left to place an item, Fill enters a mode called "swap" to try to swap items around to make a valid placement. It actually succeeds at this, but then chokes on the last meters for no reason.

NewSoupVi commented 4 days ago

There is still an unaddressed comment, otherwise I am good to merge this