Stardew Valley: Fix a bug where locations in logic would disappear fr…

…om universal tracker as items get sent (ArchipelagoMW#4230)

Co-authored-by: Exempt-Medic <[email protected]>
(cherry picked from commit e262c8b)

worlds/stardew_valley/__init__.py

@@ -3,7 +3,7 @@
 from typing import Dict, Any, Iterable, Optional, Union, List, TextIO
 
 from BaseClasses import Region, Entrance, Location, Item, Tutorial, ItemClassification, MultiWorld, CollectionState
-from Options import PerGameCommonOptions
+from Options import PerGameCommonOptions, Accessibility
 from worlds.AutoWorld import World, WebWorld
 from . import rules
 from .bundles.bundle_room import BundleRoom
@@ -91,15 +91,14 @@ class StardewValleyWorld(World):
     web = StardewWebWorld()
     modified_bundles: List[BundleRoom]
     randomized_entrances: Dict[str, str]
-    total_progression_items: int
 
-    # all_progression_items: Dict[str, int] # If you need to debug total_progression_items, uncommenting this will help tremendously
+    total_progression_items: int
+    excluded_from_total_progression_items: List[str] = [Event.received_walnuts]
 
     def __init__(self, multiworld: MultiWorld, player: int):
         super().__init__(multiworld, player)
         self.filler_item_pool_names = []
         self.total_progression_items = 0
-        # self.all_progression_items = dict()
 
         # Taking the seed specified in slot data for UT, otherwise just generating the seed.
         self.seed = getattr(multiworld, "re_gen_passthrough", {}).get(STARDEW_VALLEY, self.random.getrandbits(64))
@@ -121,17 +120,27 @@ def force_change_options_if_incompatible(self):
         goal_is_perfection = self.options.goal == Goal.option_perfection
         goal_is_island_related = goal_is_walnut_hunter or goal_is_perfection
         exclude_ginger_island = self.options.exclude_ginger_island == ExcludeGingerIsland.option_true
+
         if goal_is_island_related and exclude_ginger_island:
             self.options.exclude_ginger_island.value = ExcludeGingerIsland.option_false
             goal_name = self.options.goal.current_key
-            player_name = self.multiworld.player_name[self.player]
             logger.warning(
-                f"Goal '{goal_name}' requires Ginger Island. Exclude Ginger Island setting forced to 'False' for player {self.player} ({player_name})")
+                f"Goal '{goal_name}' requires Ginger Island. Exclude Ginger Island setting forced to 'False' for player {self.player} ({self.player_name})")
+
         if exclude_ginger_island and self.options.walnutsanity != Walnutsanity.preset_none:
             self.options.walnutsanity.value = Walnutsanity.preset_none
-            player_name = self.multiworld.player_name[self.player]
             logger.warning(
-                f"Walnutsanity requires Ginger Island. Ginger Island was excluded from {self.player} ({player_name})'s world, so walnutsanity was force disabled")
+                f"Walnutsanity requires Ginger Island. Ginger Island was excluded from {self.player} ({self.player_name})'s world, so walnutsanity was force disabled")
+
+        if goal_is_perfection and self.options.accessibility == Accessibility.option_minimal:
+            self.options.accessibility.value = Accessibility.option_full
+            logger.warning(
+                f"Goal 'Perfection' requires full accessibility. Accessibility setting forced to 'Full' for player {self.player} ({self.player_name})")
+
+        elif self.options.goal == Goal.option_allsanity and self.options.accessibility == Accessibility.option_minimal:
+            self.options.accessibility.value = Accessibility.option_full
+            logger.warning(
+                f"Goal 'Allsanity' requires full accessibility. Accessibility setting forced to 'Full' for player {self.player} ({self.player_name})")
 
     def create_regions(self):
         def create_region(name: str, exits: Iterable[str]) -> Region:
@@ -171,15 +180,26 @@ def create_items(self):
                                for location in self.multiworld.get_locations(self.player)
                                if location.address is not None])
 
-        created_items = create_items(self.create_item, self.delete_item, locations_count, items_to_exclude, self.options, self.content,
-                                     self.random)
+        created_items = create_items(self.create_item, locations_count, items_to_exclude, self.options, self.content, self.random)
 
         self.multiworld.itempool += created_items
 
         setup_early_items(self.multiworld, self.options, self.player, self.random)
         self.setup_player_events()
         self.setup_victory()
 
+        # This is really a best-effort to get the total progression items count. It is mostly used to spread grinds across spheres are push back locations that
+        # only become available after months or years in game. In most cases, not having the exact count will not impact the logic.
+        #
+        # The actual total can be impacted by the start_inventory_from_pool, when items are removed from the pool but not from the total. The is also a bug
+        # with plando where additional progression items can be created without being accounted for, which impact the real amount of progression items. This can
+        # ultimately create unwinnable seeds where some items (like Blueberry seeds) are locked in Shipsanity: Blueberry, but world is deemed winnable as the
+        # winning rule only check the count of collected progression items.
+        self.total_progression_items += sum(1 for i in self.multiworld.precollected_items[self.player] if i.advancement)
+        self.total_progression_items += sum(1 for i in self.multiworld.get_filled_locations(self.player) if i.advancement)
+        self.total_progression_items += sum(1 for i in created_items if i.advancement)
+        self.total_progression_items -= 1  # -1 for the victory event
+
     def precollect_starting_season(self):
         if self.options.season_randomization == SeasonRandomization.option_progressive:
             return
@@ -304,14 +324,8 @@ def create_item(self, item: Union[str, ItemData], override_classification: ItemC
         if override_classification is None:
             override_classification = item.classification
 
-        if override_classification & ItemClassification.progression:
-            self.total_progression_items += 1
         return StardewItem(item.name, override_classification, item.code, self.player)
 
-    def delete_item(self, item: Item):
-        if item.classification & ItemClassification.progression:
-            self.total_progression_items -= 1
-
     def create_starting_item(self, item: Union[str, ItemData]) -> StardewItem:
         if isinstance(item, str):
             item = item_table[item]
@@ -330,10 +344,6 @@ def create_event_location(self, location_data: LocationData, rule: StardewRule =
         region.locations.append(location)
         location.place_locked_item(StardewItem(item, ItemClassification.progression, None, self.player))
 
-        # This is not ideal, but the rule count them so...
-        if item != Event.victory:
-            self.total_progression_items += 1
-
     def set_rules(self):
         set_rules(self)
 
@@ -426,15 +436,25 @@ def fill_slot_data(self) -> Dict[str, Any]:
 
     def collect(self, state: CollectionState, item: StardewItem) -> bool:
         change = super().collect(state, item)
-        if change:
-            state.prog_items[self.player][Event.received_walnuts] += self.get_walnut_amount(item.name)
-        return change
+        if not change:
+            return False
+
+        walnut_amount = self.get_walnut_amount(item.name)
+        if walnut_amount:
+            state.prog_items[self.player][Event.received_walnuts] += walnut_amount
+
+        return True
 
     def remove(self, state: CollectionState, item: StardewItem) -> bool:
         change = super().remove(state, item)
-        if change:
-            state.prog_items[self.player][Event.received_walnuts] -= self.get_walnut_amount(item.name)
-        return change
+        if not change:
+            return False
+
+        walnut_amount = self.get_walnut_amount(item.name)
+        if walnut_amount:
+            state.prog_items[self.player][Event.received_walnuts] -= walnut_amount
+
+        return True
 
     @staticmethod
     def get_walnut_amount(item_name: str) -> int:

worlds/stardew_valley/items.py

@@ -172,14 +172,14 @@ def get_too_many_items_error_message(locations_count: int, items_count: int) ->
     return f"There should be at least as many locations [{locations_count}] as there are mandatory items [{items_count}]"
 
 
-def create_items(item_factory: StardewItemFactory, item_deleter: StardewItemDeleter, locations_count: int, items_to_exclude: List[Item],
+def create_items(item_factory: StardewItemFactory, locations_count: int, items_to_exclude: List[Item],
                  options: StardewValleyOptions, content: StardewContent, random: Random) -> List[Item]:
     items = []
     unique_items = create_unique_items(item_factory, options, content, random)
 
-    remove_items(item_deleter, items_to_exclude, unique_items)
+    remove_items(items_to_exclude, unique_items)
 
-    remove_items_if_no_room_for_them(item_deleter, unique_items, locations_count, random)
+    remove_items_if_no_room_for_them(unique_items, locations_count, random)
 
     items += unique_items
     logger.debug(f"Created {len(unique_items)} unique items")
@@ -195,14 +195,13 @@ def create_items(item_factory: StardewItemFactory, item_deleter: StardewItemDele
     return items
 
 
-def remove_items(item_deleter: StardewItemDeleter, items_to_remove, items):
+def remove_items(items_to_remove, items):
     for item in items_to_remove:
         if item in items:
             items.remove(item)
-            item_deleter(item)
 
 
-def remove_items_if_no_room_for_them(item_deleter: StardewItemDeleter, unique_items: List[Item], locations_count: int, random: Random):
+def remove_items_if_no_room_for_them(unique_items: List[Item], locations_count: int, random: Random):
     if len(unique_items) <= locations_count:
         return
 
@@ -215,7 +214,7 @@ def remove_items_if_no_room_for_them(item_deleter: StardewItemDeleter, unique_it
         logger.debug(f"Player has more items than locations, trying to remove {number_of_items_to_remove} random filler items")
     assert len(removable_items) >= number_of_items_to_remove, get_too_many_items_error_message(locations_count, len(unique_items))
     items_to_remove = random.sample(removable_items, number_of_items_to_remove)
-    remove_items(item_deleter, items_to_remove, unique_items)
+    remove_items(items_to_remove, unique_items)
 
 
 def create_unique_items(item_factory: StardewItemFactory, options: StardewValleyOptions, content: StardewContent, random: Random) -> List[Item]:

worlds/stardew_valley/stardew_rule/state.py

@@ -1,10 +1,13 @@
 from dataclasses import dataclass
-from typing import Iterable, Union, List, Tuple, Hashable
+from typing import Iterable, Union, List, Tuple, Hashable, TYPE_CHECKING
 
 from BaseClasses import CollectionState
 from .base import BaseStardewRule, CombinableStardewRule
 from .protocol import StardewRule
 
+if TYPE_CHECKING:
+    from .. import StardewValleyWorld
+
 
 class TotalReceived(BaseStardewRule):
     count: int
@@ -102,16 +105,19 @@ def value(self):
         return self.percent
 
     def __call__(self, state: CollectionState) -> bool:
-        stardew_world = state.multiworld.worlds[self.player]
+        stardew_world: "StardewValleyWorld" = state.multiworld.worlds[self.player]
         total_count = stardew_world.total_progression_items
         needed_count = (total_count * self.percent) // 100
         player_state = state.prog_items[self.player]
 
-        if needed_count <= len(player_state):
+        if needed_count <= len(player_state) - len(stardew_world.excluded_from_total_progression_items):
             return True
 
         total_count = 0
         for item, item_count in player_state.items():
+            if item in stardew_world.excluded_from_total_progression_items:
+                continue
+
             total_count += item_count
             if total_count >= needed_count:
                 return True

worlds/stardew_valley/test/TestCrops.py

@@ -11,10 +11,10 @@ def test_need_greenhouse_for_cactus(self):
         harvest_cactus = self.world.logic.region.can_reach_location("Harvest Cactus Fruit")
         self.assert_rule_false(harvest_cactus, self.multiworld.state)
 
-        self.multiworld.state.collect(self.world.create_item("Cactus Seeds"), prevent_sweep=False)
-        self.multiworld.state.collect(self.world.create_item("Shipping Bin"), prevent_sweep=False)
-        self.multiworld.state.collect(self.world.create_item("Desert Obelisk"), prevent_sweep=False)
+        self.multiworld.state.collect(self.create_item("Cactus Seeds"))
+        self.multiworld.state.collect(self.create_item("Shipping Bin"))
+        self.multiworld.state.collect(self.create_item("Desert Obelisk"))
         self.assert_rule_false(harvest_cactus, self.multiworld.state)
 
-        self.multiworld.state.collect(self.world.create_item("Greenhouse"), prevent_sweep=False)
+        self.multiworld.state.collect(self.create_item("Greenhouse"))
         self.assert_rule_true(harvest_cactus, self.multiworld.state)

worlds/stardew_valley/test/TestOptions.py

@@ -1,6 +1,6 @@
 import itertools
 
-from Options import NamedRange
+from Options import NamedRange, Accessibility
 from . import SVTestCase, allsanity_no_mods_6_x_x, allsanity_mods_6_x_x, solo_multiworld
 from .assertion import WorldAssertMixin
 from .long.option_names import all_option_choices
@@ -54,6 +54,23 @@ def test_given_goal_when_generate_then_victory_is_in_correct_location(self):
                 victory = multi_world.find_item("Victory", 1)
                 self.assertEqual(victory.name, location)
 
+    def test_given_perfection_goal_when_generate_then_accessibility_is_forced_to_full(self):
+        """There is a bug with the current victory condition of the perfection goal that can create unwinnable seeds if the accessibility is set to minimal and
+        the world gets flooded with progression items through plando. This will increase the amount of collected progression items pass the total amount
+        calculated for the world when creating the item pool. This will cause the victory condition to be met before all locations are collected, so some could
+        be left inaccessible, which in practice will make the seed unwinnable. 
+        """
+        for accessibility in Accessibility.options.keys():
+            world_options = {Goal.internal_name: Goal.option_perfection, "accessibility": accessibility}
+            with self.solo_world_sub_test(f"Accessibility: {accessibility}", world_options) as (_, world):
+                self.assertEqual(world.options.accessibility, Accessibility.option_full)
+
+    def test_given_allsanity_goal_when_generate_then_accessibility_is_forced_to_full(self):
+        for accessibility in Accessibility.options.keys():
+            world_options = {Goal.internal_name: Goal.option_allsanity, "accessibility": accessibility}
+            with self.solo_world_sub_test(f"Accessibility: {accessibility}", world_options) as (_, world):
+                self.assertEqual(world.options.accessibility, Accessibility.option_full)
+
 
 class TestSeasonRandomization(SVTestCase):
     def test_given_disabled_when_generate_then_all_seasons_are_precollected(self):

worlds/stardew_valley/test/__init__.py

@@ -6,7 +6,7 @@
 from contextlib import contextmanager
 from typing import Dict, ClassVar, Iterable, Tuple, Optional, List, Union, Any
 
-from BaseClasses import MultiWorld, CollectionState, PlandoOptions, get_seed, Location, Item, ItemClassification
+from BaseClasses import MultiWorld, CollectionState, PlandoOptions, get_seed, Location, Item
 from Options import VerifyKeys
 from test.bases import WorldTestBase
 from test.general import gen_steps, setup_solo_multiworld as setup_base_solo_multiworld
@@ -236,7 +236,6 @@ def world_setup(self, *args, **kwargs):
 
         self.original_state = self.multiworld.state.copy()
         self.original_itempool = self.multiworld.itempool.copy()
-        self.original_prog_item_count = world.total_progression_items
         self.unfilled_locations = self.multiworld.get_unfilled_locations(1)
         if self.constructed:
             self.world = world  # noqa
@@ -246,7 +245,6 @@ def tearDown(self) -> None:
         self.multiworld.itempool = self.original_itempool
         for location in self.unfilled_locations:
             location.item = None
-        self.world.total_progression_items = self.original_prog_item_count
 
         self.multiworld.lock.release()
 
@@ -257,28 +255,22 @@ def run_default_tests(self) -> bool:
         return super().run_default_tests
 
     def collect_lots_of_money(self, percent: float = 0.25):
-        self.multiworld.state.collect(self.world.create_item("Shipping Bin"), prevent_sweep=False)
-        real_total_prog_items = self.multiworld.worlds[self.player].total_progression_items
+        self.collect("Shipping Bin")
+        real_total_prog_items = self.world.total_progression_items
         required_prog_items = int(round(real_total_prog_items * percent))
-        for i in range(required_prog_items):
-            self.multiworld.state.collect(self.world.create_item("Stardrop"), prevent_sweep=False)
-        self.multiworld.worlds[self.player].total_progression_items = real_total_prog_items
+        self.collect("Stardrop", required_prog_items)
 
     def collect_all_the_money(self):
-        self.multiworld.state.collect(self.world.create_item("Shipping Bin"), prevent_sweep=False)
-        real_total_prog_items = self.multiworld.worlds[self.player].total_progression_items
-        required_prog_items = int(round(real_total_prog_items * 0.95))
-        for i in range(required_prog_items):
-            self.multiworld.state.collect(self.world.create_item("Stardrop"), prevent_sweep=False)
-        self.multiworld.worlds[self.player].total_progression_items = real_total_prog_items
+        self.collect_lots_of_money(0.95)
 
     def collect_everything(self):
         non_event_items = [item for item in self.multiworld.get_items() if item.code]
         for item in non_event_items:
             self.multiworld.state.collect(item)
 
     def collect_all_except(self, item_to_not_collect: str):
-        for item in self.multiworld.get_items():
+        non_event_items = [item for item in self.multiworld.get_items() if item.code]
+        for item in non_event_items:
             if item.name != item_to_not_collect:
                 self.multiworld.state.collect(item)
 
@@ -290,25 +282,26 @@ def get_real_location_names(self) -> List[str]:
 
     def collect(self, item: Union[str, Item, Iterable[Item]], count: int = 1) -> Union[None, Item, List[Item]]:
         assert count > 0
+
         if not isinstance(item, str):
             super().collect(item)
             return
+
         if count == 1:
             item = self.create_item(item)
             self.multiworld.state.collect(item)
             return item
+
         items = []
         for i in range(count):
             item = self.create_item(item)
             self.multiworld.state.collect(item)
             items.append(item)
+
         return items
 
     def create_item(self, item: str) -> StardewItem:
-        created_item = self.world.create_item(item)
-        if created_item.classification & ItemClassification.progression:
-            self.multiworld.worlds[self.player].total_progression_items -= 1
-        return created_item
+        return self.world.create_item(item)
 
     def remove_one_by_name(self, item: str) -> None:
         self.remove(self.create_item(item))
@@ -336,15 +329,13 @@ def solo_multiworld(world_options: Optional[Dict[Union[str, StardewValleyOption]
         original_state = multiworld.state.copy()
         original_itempool = multiworld.itempool.copy()
         unfilled_locations = multiworld.get_unfilled_locations(1)
-        original_prog_item_count = world.total_progression_items
 
         yield multiworld, world
 
         multiworld.state = original_state
         multiworld.itempool = original_itempool
         for location in unfilled_locations:
             location.item = None
-        multiworld.total_progression_items = original_prog_item_count
 
         multiworld.lock.release()