Stardew Valley: Fix potential soft lock with vanilla tools and entrance randomizer + Performance improvement for vanilla tool/skills (ArchipelagoMW#3002)

* fix vanilla tool fishing rod requiring metal bars
fix vanilla skill requiring previous level (it's always the same rule or more restrictive)

* add test to ensure fishing rod need fish shop

* fishing rod should be indexed from 0 like a mentally sane person would do.

* fishing rod 0 isn't real, but it definitely can hurt you.

* reeeeeeeee

Author: Jouramie

worlds/stardew_valley/logic/fishing_logic.py

@@ -73,7 +73,8 @@ def can_catch_quality_fish(self, fish_quality: str) -> StardewRule:
             return rod_rule & self.logic.skill.has_level(Skill.fishing, 4)
         if fish_quality == FishQuality.iridium:
             return rod_rule & self.logic.skill.has_level(Skill.fishing, 10)
-        return False_()
+
+        raise ValueError(f"Quality {fish_quality} is unknown.")
 
     def can_catch_every_fish(self) -> StardewRule:
         rules = [self.has_max_fishing()]

worlds/stardew_valley/logic/skill_logic.py

@@ -44,10 +44,14 @@ def can_earn_level(self, skill: str, level: int) -> StardewRule:
         tool_material = ToolMaterial.tiers[tool_level]
         months = max(1, level - 1)
         months_rule = self.logic.time.has_lived_months(months)
-        previous_level_rule = self.logic.skill.has_level(skill, level - 1)
+
+        if self.options.skill_progression != options.SkillProgression.option_vanilla:
+            previous_level_rule = self.logic.skill.has_level(skill, level - 1)
+        else:
+            previous_level_rule = True_()
 
         if skill == Skill.fishing:
-            xp_rule = self.logic.tool.has_tool(Tool.fishing_rod, ToolMaterial.tiers[max(tool_level, 3)])
+            xp_rule = self.logic.tool.has_fishing_rod(max(tool_level, 1))
         elif skill == Skill.farming:
             xp_rule = self.logic.tool.has_tool(Tool.hoe, tool_material) & self.logic.tool.can_water(tool_level)
         elif skill == Skill.foraging:
@@ -137,13 +141,17 @@ def can_get_fishing_xp(self) -> StardewRule:
     def can_fish(self, regions: Union[str, Tuple[str, ...]] = None, difficulty: int = 0) -> StardewRule:
         if isinstance(regions, str):
             regions = regions,
+
         if regions is None or len(regions) == 0:
             regions = fishing_regions
+
         skill_required = min(10, max(0, int((difficulty / 10) - 1)))
         if difficulty <= 40:
             skill_required = 0
+
         skill_rule = self.logic.skill.has_level(Skill.fishing, skill_required)
         region_rule = self.logic.region.can_reach_any(regions)
+        # Training rod only works with fish < 50. Fiberglass does not help you to catch higher difficulty fish, so it's skipped in logic.
         number_fishing_rod_required = 1 if difficulty < 50 else (2 if difficulty < 80 else 4)
         return self.logic.tool.has_fishing_rod(number_fishing_rod_required) & skill_rule & region_rule
 

worlds/stardew_valley/logic/tool_logic.py

@@ -12,10 +12,14 @@
 from ..stardew_rule import StardewRule, True_, False_
 from ..strings.ap_names.skill_level_names import ModSkillLevel
 from ..strings.region_names import Region
-from ..strings.skill_names import ModSkill
 from ..strings.spells import MagicSpell
 from ..strings.tool_names import ToolMaterial, Tool
 
+fishing_rod_prices = {
+    3: 1800,
+    4: 7500,
+}
+
 tool_materials = {
     ToolMaterial.copper: 1,
     ToolMaterial.iron: 2,
@@ -40,27 +44,31 @@ def __init__(self, *args, **kwargs):
 class ToolLogic(BaseLogic[Union[ToolLogicMixin, HasLogicMixin, ReceivedLogicMixin, RegionLogicMixin, SeasonLogicMixin, MoneyLogicMixin, MagicLogicMixin]]):
     # Should be cached
     def has_tool(self, tool: str, material: str = ToolMaterial.basic) -> StardewRule:
+        assert tool != Tool.fishing_rod, "Use `has_fishing_rod` instead of `has_tool`."
+
         if material == ToolMaterial.basic or tool == Tool.scythe:
             return True_()
 
         if self.options.tool_progression & ToolProgression.option_progressive:
             return self.logic.received(f"Progressive {tool}", tool_materials[material])
 
-        return self.logic.has(f"{material} Bar") & self.logic.money.can_spend(tool_upgrade_prices[material])
+        return self.logic.has(f"{material} Bar") & self.logic.money.can_spend_at(Region.blacksmith, tool_upgrade_prices[material])
 
     def can_use_tool_at(self, tool: str, material: str, region: str) -> StardewRule:
         return self.has_tool(tool, material) & self.logic.region.can_reach(region)
 
     @cache_self1
     def has_fishing_rod(self, level: int) -> StardewRule:
+        assert 1 <= level <= 4, "Fishing rod 0 isn't real, it can't hurt you. Training is 1, Bamboo is 2, Fiberglass is 3 and Iridium is 4."
+
         if self.options.tool_progression & ToolProgression.option_progressive:
             return self.logic.received(f"Progressive {Tool.fishing_rod}", level)
 
-        if level <= 1:
+        if level <= 2:
+            # We assume you always have access to the Bamboo pole, because mod side there is a builtin way to get it back.
             return self.logic.region.can_reach(Region.beach)
-        prices = {2: 500, 3: 1800, 4: 7500}
-        level = min(level, 4)
-        return self.logic.money.can_spend_at(Region.fish_shop, prices[level])
+
+        return self.logic.money.can_spend_at(Region.fish_shop, fishing_rod_prices[level])
 
     # Should be cached
     def can_forage(self, season: Union[str, Iterable[str]], region: str = Region.forest, need_hoe: bool = False) -> StardewRule:

worlds/stardew_valley/test/TestRules.py

@@ -8,6 +8,7 @@
     FriendsanityHeartSize, BundleRandomization, SkillProgression
 from ..strings.entrance_names import Entrance
 from ..strings.region_names import Region
+from ..strings.tool_names import Tool, ToolMaterial
 
 
 class TestProgressiveToolsLogic(SVTestBase):
@@ -596,6 +597,54 @@ def swap_museum_and_bathhouse(multiworld, player):
     bathhouse_entrance.connect(museum_region)
 
 
+class TestToolVanillaRequiresBlacksmith(SVTestBase):
+    options = {
+        options.EntranceRandomization: options.EntranceRandomization.option_buildings,
+        options.ToolProgression: options.ToolProgression.option_vanilla,
+    }
+    seed = 4111845104987680262
+
+    # Seed is hardcoded to make sure the ER is a valid roll that actually lock the blacksmith behind the Railroad Boulder Removed.
+
+    def test_cannot_get_any_tool_without_blacksmith_access(self):
+        railroad_item = "Railroad Boulder Removed"
+        place_region_at_entrance(self.multiworld, self.player, Region.blacksmith, Entrance.enter_bathhouse_entrance)
+        collect_all_except(self.multiworld, railroad_item)
+
+        for tool in [Tool.pickaxe, Tool.axe, Tool.hoe, Tool.trash_can, Tool.watering_can]:
+            for material in [ToolMaterial.copper, ToolMaterial.iron, ToolMaterial.gold, ToolMaterial.iridium]:
+                self.assert_rule_false(self.world.logic.tool.has_tool(tool, material), self.multiworld.state)
+
+        self.multiworld.state.collect(self.world.create_item(railroad_item), event=False)
+
+        for tool in [Tool.pickaxe, Tool.axe, Tool.hoe, Tool.trash_can, Tool.watering_can]:
+            for material in [ToolMaterial.copper, ToolMaterial.iron, ToolMaterial.gold, ToolMaterial.iridium]:
+                self.assert_rule_true(self.world.logic.tool.has_tool(tool, material), self.multiworld.state)
+
+    def test_cannot_get_fishing_rod_without_willy_access(self):
+        railroad_item = "Railroad Boulder Removed"
+        place_region_at_entrance(self.multiworld, self.player, Region.fish_shop, Entrance.enter_bathhouse_entrance)
+        collect_all_except(self.multiworld, railroad_item)
+
+        for fishing_rod_level in [3, 4]:
+            self.assert_rule_false(self.world.logic.tool.has_fishing_rod(fishing_rod_level), self.multiworld.state)
+
+        self.multiworld.state.collect(self.world.create_item(railroad_item), event=False)
+
+        for fishing_rod_level in [3, 4]:
+            self.assert_rule_true(self.world.logic.tool.has_fishing_rod(fishing_rod_level), self.multiworld.state)
+
+
+def place_region_at_entrance(multiworld, player, region, entrance):
+    region_to_place = multiworld.get_region(region, player)
+    entrance_to_place_region = multiworld.get_entrance(entrance, player)
+
+    entrance_to_switch = region_to_place.entrances[0]
+    region_to_switch = entrance_to_place_region.connected_region
+    entrance_to_switch.connect(region_to_switch)
+    entrance_to_place_region.connect(region_to_place)
+
+
 def collect_all_except(multiworld, item_to_not_collect: str):
     for item in multiworld.get_items():
         if item.name != item_to_not_collect: