Core: Add new ItemClassification "deprioritized" which will not be placed on priority locations (if possible) (#4610)

* Add new deprioritized item flag

* 4 retries

* indent

* .

* style

* I think this is nicer

* Nicer

* remove two lines again that I added unnecessarily

* I think this test makes a bit more sense like this

* Idk how to word this lol

* Add progression_deprioritized_skip_balancing bc why not ig

* More text

* Update Fill.py

* Update Fill.py

* I am the big stupid

* Actually collect the other half of progression items into state when filling without them

* More clarity on the descriptions (hopefully)

* visually separate technical description and use cases

* Actually make the call do what the comments say it does

BaseClasses.py

@@ -1436,27 +1436,43 @@ class Location:
 
 
 class ItemClassification(IntFlag):
-    filler = 0b0000
+    filler = 0b00000
     """ aka trash, as in filler items like ammo, currency etc """
 
-    progression = 0b0001
+    progression = 0b00001
     """ Item that is logically relevant.
     Protects this item from being placed on excluded or unreachable locations. """
 
-    useful = 0b0010
+    useful = 0b00010
     """ Item that is especially useful.
     Protects this item from being placed on excluded or unreachable locations.
     When combined with another flag like "progression", it means "an especially useful progression item". """
 
-    trap = 0b0100
+    trap = 0b00100
     """ Item that is detrimental in some way. """
 
-    skip_balancing = 0b1000
+    skip_balancing = 0b01000
     """ should technically never occur on its own
     Item that is logically relevant, but progression balancing should not touch.
-    Typically currency or other counted items. """
+    
+    Possible reasons for why an item should not be pulled ahead by progression balancing:
+    1. This item is quite insignificant, so pulling it earlier doesn't help (currency/etc.)
+    2. It is important for the player experience that this item is evenly distributed in the seed (e.g. goal items) """
 
-    progression_skip_balancing = 0b1001  # only progression gets balanced
+    deprioritized = 0b10000
+    """ Should technically never occur on its own.
+    Will not be considered for priority locations,
+    unless Priority Locations Fill runs out of regular progression items before filling all priority locations. 
+    
+    Should be used for items that would feel bad for the player to find on a priority location.
+    Usually, these are items that are plentiful or insignificant. """
+
+    progression_deprioritized_skip_balancing = 0b11001
+    """ Since a common case of both skip_balancing and deprioritized is "insignificant progression", 
+    these items often want both flags. """
+
+    progression_skip_balancing = 0b01001  # only progression gets balanced
+    progression_deprioritized = 0b10001  # only progression can be placed during priority fill
 
     def as_flag(self) -> int:
         """As Network API flag int."""
@@ -1504,6 +1520,10 @@ class Item:
     def trap(self) -> bool:
         return ItemClassification.trap in self.classification
 
+    @property
+    def deprioritized(self) -> bool:
+        return ItemClassification.deprioritized in self.classification
+
     @property
     def filler(self) -> bool:
         return not (self.advancement or self.useful or self.trap)

Fill.py

@@ -526,18 +526,48 @@ def distribute_items_restrictive(multiworld: MultiWorld,
     single_player = multiworld.players == 1 and not multiworld.groups
 
     if prioritylocations:
+        regular_progression = []
+        deprioritized_progression = []
+        for item in progitempool:
+            if item.deprioritized:
+                deprioritized_progression.append(item)
+            else:
+                regular_progression.append(item)
+
         # "priority fill"
-        maximum_exploration_state = sweep_from_pool(multiworld.state)
+        # try without deprioritized items in the mix at all. This means they need to be collected into state first.
-        fill_restrictive(multiworld, maximum_exploration_state, prioritylocations, progitempool,
+        priority_fill_state = sweep_from_pool(multiworld.state, deprioritized_progression)
+        fill_restrictive(multiworld, priority_fill_state, prioritylocations, regular_progression,
                          single_player_placement=single_player, swap=False, on_place=mark_for_locking,
                          name="Priority", one_item_per_player=True, allow_partial=True)
 
-        if prioritylocations:
+        if prioritylocations and regular_progression:
             # retry with one_item_per_player off because some priority fills can fail to fill with that optimization
-            maximum_exploration_state = sweep_from_pool(multiworld.state)
+            # deprioritized items are still not in the mix, so they need to be collected into state first.
-            fill_restrictive(multiworld, maximum_exploration_state, prioritylocations, progitempool,
+            priority_retry_state = sweep_from_pool(multiworld.state, deprioritized_progression)
-                            single_player_placement=single_player, swap=False, on_place=mark_for_locking,
+            fill_restrictive(multiworld, priority_retry_state, prioritylocations, regular_progression,
-                            name="Priority Retry", one_item_per_player=False)
+                             single_player_placement=single_player, swap=False, on_place=mark_for_locking,
+                             name="Priority Retry", one_item_per_player=False, allow_partial=True)
+
+        if prioritylocations and deprioritized_progression:
+            # There are no more regular progression items that can be placed on any priority locations.
+            # We'd still prefer to place deprioritized progression items on priority locations over filler items.
+            # Since we're leaving out the remaining regular progression now, we need to collect it into state first.
+            priority_retry_2_state = sweep_from_pool(multiworld.state, regular_progression)
+            fill_restrictive(multiworld, priority_retry_2_state, prioritylocations, deprioritized_progression,
+                             single_player_placement=single_player, swap=False, on_place=mark_for_locking,
+                             name="Priority Retry 2", one_item_per_player=True, allow_partial=True)
+
+        if prioritylocations and deprioritized_progression:
+            # retry with deprioritized items AND without one_item_per_player optimisation
+            # Since we're leaving out the remaining regular progression now, we need to collect it into state first.
+            priority_retry_3_state = sweep_from_pool(multiworld.state, regular_progression)
+            fill_restrictive(multiworld, priority_retry_3_state, prioritylocations, deprioritized_progression,
+                             single_player_placement=single_player, swap=False, on_place=mark_for_locking,
+                             name="Priority Retry 3", one_item_per_player=False)
+
+        # restore original order of progitempool
+        progitempool[:] = [item for item in progitempool if not item.location]
         accessibility_corrections(multiworld, multiworld.state, prioritylocations, progitempool)
         defaultlocations = prioritylocations + defaultlocations
 

test/general/test_fill.py

@@ -603,6 +603,28 @@ class TestDistributeItemsRestrictive(unittest.TestCase):
         self.assertTrue(player3.locations[2].item.advancement)
         self.assertTrue(player3.locations[3].item.advancement)
 
+    def test_deprioritized_does_not_land_on_priority(self):
+        multiworld = generate_test_multiworld(1)
+        player1 = generate_player_data(multiworld, 1, 2, prog_item_count=2)
+
+        player1.prog_items[0].classification |= ItemClassification.deprioritized
+        player1.locations[0].progress_type = LocationProgressType.PRIORITY
+
+        distribute_items_restrictive(multiworld)
+
+        self.assertFalse(player1.locations[0].item.deprioritized)
+
+    def test_deprioritized_still_goes_on_priority_ahead_of_filler(self):
+        multiworld = generate_test_multiworld(1)
+        player1 = generate_player_data(multiworld, 1, 2, prog_item_count=1, basic_item_count=1)
+
+        player1.prog_items[0].classification |= ItemClassification.deprioritized
+        player1.locations[0].progress_type = LocationProgressType.PRIORITY
+
+        distribute_items_restrictive(multiworld)
+
+        self.assertTrue(player1.locations[0].item.advancement)
+
     def test_can_remove_locations_in_fill_hook(self):
         """Test that distribute_items_restrictive calls the fill hook and allows for item and location removal"""
         multiworld = generate_test_multiworld()