SC2: Fix bugs and issues around excluded/unexcluded (#5644)

Author: MatthewMarinets

worlds/sc2/__init__.py

@@ -374,22 +374,32 @@ def create_and_flag_explicit_item_locks_and_excludes(world: SC2World) -> List[Fi
     Handles `excluded_items`, `locked_items`, and `start_inventory`
     Returns a list of all possible non-filler items that can be added, with an accompanying flags bitfield.
     """
-    excluded_items = world.options.excluded_items
-    unexcluded_items = world.options.unexcluded_items
-    locked_items = world.options.locked_items
-    start_inventory = world.options.start_inventory
+    excluded_items: dict[str, int] = world.options.excluded_items.value
+    unexcluded_items: dict[str, int] = world.options.unexcluded_items.value
+    locked_items: dict[str, int] = world.options.locked_items.value
+    start_inventory: dict[str, int] = world.options.start_inventory.value
     key_items = world.custom_mission_order.get_items_to_lock()
 
-    def resolve_count(count: Optional[int], max_count: int) -> int:
-        if count == 0:
+    def resolve_exclude(count: int, max_count: int) -> int:
+        if count < 0:
             return max_count
-        if count is None:
-            return 0
-        if max_count == 0:
-            return count
-        return min(count, max_count)
+        return count
+
+    def resolve_count(count: int, max_count: int, negative_value: int | None = None) -> int:
+        """
+        Handles `count` being out of range.
+        * If `count > max_count`, returns `max_count`.
+        * If `count < 0`, returns `negative_value` (returns `max_count` if `negative_value` is unspecified)
+        """
+        if count < 0:
+            if negative_value is None:
+                return max_count
+            return negative_value
+        if max_count and count > max_count:
+            return max_count
+        return count
 
-    auto_excludes = {item_name: 1 for item_name in item_groups.legacy_items}
+    auto_excludes = Counter({item_name: 1 for item_name in item_groups.legacy_items})
     if world.options.exclude_overpowered_items.value == ExcludeOverpoweredItems.option_true:
         for item_name in item_groups.overpowered_items:
             auto_excludes[item_name] = 1
@@ -402,28 +412,29 @@ def resolve_count(count: Optional[int], max_count: int) -> int:
             elif item_name in item_groups.nova_equipment:
                 continue
             else:
-                auto_excludes[item_name] = 0
+                auto_excludes[item_name] = item_data.quantity
 
 
     result: List[FilterItem] = []
     for item_name, item_data in item_tables.item_table.items():
         max_count = item_data.quantity
-        auto_excluded_count = auto_excludes.get(item_name)
+        auto_excluded_count = auto_excludes.get(item_name, 0)
         excluded_count = excluded_items.get(item_name, auto_excluded_count)
-        unexcluded_count = unexcluded_items.get(item_name)
-        locked_count = locked_items.get(item_name)
-        start_count: Optional[int] = start_inventory.get(item_name)
+        unexcluded_count = unexcluded_items.get(item_name, 0)
+        locked_count = locked_items.get(item_name, 0)
+        start_count = start_inventory.get(item_name, 0)
         key_count = key_items.get(item_name, 0)
-        # specifying 0 in the yaml means exclude / lock all
-        # start_inventory doesn't allow specifying 0
-        # not specifying means don't exclude/lock/start
-        excluded_count = resolve_count(excluded_count, max_count)
-        unexcluded_count = resolve_count(unexcluded_count, max_count)
+        # Specifying a negative number in the yaml means exclude / lock / start all.
+        # In the case of excluded/unexcluded, resolve negatives to max_count before subtracting them,
+        # and after subtraction resolve negatives to just 0 (when unexcluded > excluded).
+        excluded_count = resolve_count(
+            resolve_exclude(excluded_count, max_count) - resolve_exclude(unexcluded_count, max_count),
+            max_count,
+            negative_value=0
+        )
         locked_count = resolve_count(locked_count, max_count)
         start_count = resolve_count(start_count, max_count)
 
-        excluded_count = max(0, excluded_count - unexcluded_count)
-
         # Priority: start_inventory >> locked_items >> excluded_items >> unspecified
         if max_count == 0:
             if excluded_count:
@@ -486,8 +497,9 @@ def flag_excludes_by_faction_presence(world: SC2World, item_list: List[FilterIte
                 item.flags |= ItemFilterFlags.FilterExcluded
                 continue
         if not zerg_missions and item.data.race == SC2Race.ZERG:
-            if item.data.type != item_tables.ZergItemType.Ability \
-                    and item.data.type != ZergItemType.Level:
+            if (item.data.type != item_tables.ZergItemType.Ability
+                and item.data.type != ZergItemType.Level
+            ):
                 item.flags |= ItemFilterFlags.FilterExcluded
                 continue
         if not protoss_missions and item.data.race == SC2Race.PROTOSS:
@@ -641,7 +653,7 @@ def flag_mission_based_item_excludes(world: SC2World, item_list: List[FilterItem
             item.flags |= ItemFilterFlags.FilterExcluded
 
         # Remove Spear of Adun passives
-        if item.name in item_tables.spear_of_adun_castable_passives and not soa_passive_presence:
+        if item.name in item_groups.spear_of_adun_passives and not soa_passive_presence:
             item.flags |= ItemFilterFlags.FilterExcluded
 
         # Remove matchup-specific items if you don't play that matchup

worlds/sc2/client.py

@@ -40,6 +40,7 @@
     SpearOfAdunPassivesPresentInNoBuild, EnableVoidTrade, VoidTradeAgeLimit, void_trade_age_limits_ms, VoidTradeWorkers,
     DifficultyDamageModifier, MissionOrderScouting, GenericUpgradeResearchSpeedup, MercenaryHighlanders, WarCouncilNerfs,
     is_mission_in_soa_presence,
+    upgrade_included_names,
 )
 from .mission_order.slot_data import CampaignSlotData, LayoutSlotData, MissionSlotData, MissionOrderObjectSlotData
 from .mission_order.entry_rules import SubRuleRuleData, CountMissionsRuleData, MissionEntryRules
@@ -71,10 +72,12 @@
 )
 
 import colorama
-from .options import Option, upgrade_included_names
 from NetUtils import ClientStatus, NetworkItem, JSONtoTextParser, JSONMessagePart, add_json_item, add_json_location, add_json_text, JSONTypes
 from MultiServer import mark_raw
 
+if typing.TYPE_CHECKING:
+    from Options import Option
+
 pool = concurrent.futures.ThreadPoolExecutor(1)
 loop = asyncio.get_event_loop_policy().new_event_loop()
 nest_asyncio.apply(loop)

worlds/sc2/item/item_groups.py

@@ -167,6 +167,7 @@ class ItemGroupNames:
     LOTV_UNITS = "LotV Units"
     LOTV_ITEMS = "LotV Items"
     LOTV_GLOBAL_UPGRADES = "LotV Global Upgrades"
+    SOA_PASSIVES = "SOA Passive Abilities"
     SOA_ITEMS = "SOA"
     PROTOSS_GLOBAL_UPGRADES = "Protoss Global Upgrades"
     PROTOSS_BUILDINGS = "Protoss Buildings"
@@ -777,11 +778,21 @@ def get_all_group_names(cls) -> typing.Set[str]:
     item_names.MIRAGE, item_names.DAWNBRINGER, item_names.TRIREME, item_names.TEMPEST,
     item_names.CALADRIUS,
 ]
-item_name_groups[ItemGroupNames.SOA_ITEMS] = soa_items = [
+item_name_groups[ItemGroupNames.SOA_PASSIVES] = spear_of_adun_passives = [
+    item_names.RECONSTRUCTION_BEAM,
+    item_names.OVERWATCH,
+    item_names.GUARDIAN_SHELL,
+]
+spear_of_adun_actives = [
     *[item_name for item_name, item_data in item_tables.item_table.items() if item_data.type == item_tables.ProtossItemType.Spear_Of_Adun],
     item_names.SOA_PROGRESSIVE_PROXY_PYLON,
 ]
-lotv_soa_items = [item_name for item_name in soa_items if item_name != item_names.SOA_PYLON_OVERCHARGE]
+item_name_groups[ItemGroupNames.SOA_ITEMS] = soa_items = spear_of_adun_actives + spear_of_adun_passives
+lotv_soa_items = [
+    item_name
+    for item_name in soa_items
+    if item_name not in (item_names.SOA_PYLON_OVERCHARGE, item_names.OVERWATCH)
+]
 item_name_groups[ItemGroupNames.PROTOSS_GLOBAL_UPGRADES] = [
     item_name for item_name, item_data in item_tables.item_table.items() if item_data.type == item_tables.ProtossItemType.Solarite_Core
 ]

worlds/sc2/item/item_tables.py

@@ -2293,12 +2293,6 @@ def get_item_table():
     item_names.SOA_SOLAR_BOMBARDMENT
 }
 
-spear_of_adun_castable_passives = {
-    item_names.RECONSTRUCTION_BEAM,
-    item_names.OVERWATCH,
-    item_names.GUARDIAN_SHELL,
-}
-
 nova_equipment = {
     *[item_name for item_name, item_data in get_full_item_list().items()
       if item_data.type == TerranItemType.Nova_Gear],

worlds/sc2/options.py

@@ -5,14 +5,13 @@
 
 from Options import (
     Choice, Toggle, DefaultOnToggle, OptionSet, Range,
-    PerGameCommonOptions, Option, VerifyKeys, StartInventory,
+    PerGameCommonOptions, VerifyKeys, StartInventory,
     is_iterable_except_str, OptionGroup, Visibility, ItemDict,
-    Accessibility, ProgressionBalancing
+    OptionCounter,
 )
 from Utils import get_fuzzy_results
 from BaseClasses import PlandoOptions
-from .item import item_names, item_tables
-from .item.item_groups import kerrigan_active_abilities, kerrigan_passives, nova_weapons, nova_gadgets
+from .item import item_names, item_tables, item_groups
 from .mission_tables import (
     SC2Campaign, SC2Mission, lookup_name_to_mission, MissionPools, get_missions_with_any_flags_in_list,
     campaign_mission_table, SC2Race, MissionFlag
@@ -700,7 +699,7 @@ class KerriganMaxActiveAbilities(Range):
     """
     display_name = "Kerrigan Maximum Active Abilities"
     range_start = 0
-    range_end = len(kerrigan_active_abilities)
+    range_end = len(item_groups.kerrigan_active_abilities)
     default = range_end
 
 
@@ -711,7 +710,7 @@ class KerriganMaxPassiveAbilities(Range):
     """
     display_name = "Kerrigan Maximum Passive Abilities"
     range_start = 0
-    range_end = len(kerrigan_passives)
+    range_end = len(item_groups.kerrigan_passives)
     default = range_end
 
 
@@ -829,7 +828,7 @@ class SpearOfAdunMaxAutocastAbilities(Range):
     """
     display_name = "Spear of Adun Maximum Passive Abilities"
     range_start = 0
-    range_end = sum(item.quantity for item_name, item in item_tables.get_full_item_list().items() if item_name in item_tables.spear_of_adun_castable_passives)
+    range_end = sum(item_tables.item_table[item_name].quantity for item_name in item_groups.spear_of_adun_passives)
     default = range_end
 
 
@@ -883,7 +882,7 @@ class NovaMaxWeapons(Range):
     """
     display_name = "Nova Maximum Weapons"
     range_start = 0
-    range_end = len(nova_weapons)
+    range_end = len(item_groups.nova_weapons)
     default = range_end
 
 
@@ -897,7 +896,7 @@ class NovaMaxGadgets(Range):
     """
     display_name = "Nova Maximum Gadgets"
     range_start = 0
-    range_end = len(nova_gadgets)
+    range_end = len(item_groups.nova_gadgets)
     default = range_end
 
 
@@ -932,33 +931,48 @@ class TakeOverAIAllies(Toggle):
     display_name = "Take Over AI Allies"
 
 
-class Sc2ItemDict(Option[Dict[str, int]], VerifyKeys, Mapping[str, int]):
-    """A branch of ItemDict that supports item counts of 0"""
+class Sc2ItemDict(OptionCounter, VerifyKeys, Mapping[str, int]):
+    """A branch of ItemDict that supports negative item counts"""
     default = {}
     supports_weighting = False
     verify_item_name = True
     # convert_name_groups = True
     display_name = 'Unnamed dictionary'
-    minimum_value: int = 0
+    # Note(phaneros): Limiting minimum to -1 means that if two triggers add -1 to the same item,
+    # the validation fails. So give trigger people space to stack a bunch of triggers.
+    min: int = -1000
+    max: int = 1000
+    valid_keys = set(item_tables.item_table) | set(item_groups.item_name_groups)
 
-    def __init__(self, value: Dict[str, int]):
+    def __init__(self, value: dict[str, int]):
         self.value = {key: val for key, val in value.items()}
 
     @classmethod
-    def from_any(cls, data: Union[List[str], Dict[str, int]]) -> 'Sc2ItemDict':
+    def from_any(cls, data: list[str] | dict[str, int]) -> 'Sc2ItemDict':
         if isinstance(data, list):
-            # This is a little default that gets us backwards compatibility with lists.
-            # It doesn't play nice with trigger merging dicts and lists together, though, so best not to advertise it overmuch.
-            data = {item: 0 for item in data}
+            raise ValueError(
+                f"{cls.display_name}: Cannot convert from list. "
+                f"Use dict syntax (no dashes, 'value: number' synax)."
+            )
         if isinstance(data, dict):
             for key, value in data.items():
                 if not isinstance(value, int):
-                    raise ValueError(f"Invalid type in '{cls.display_name}': element '{key}' maps to '{value}', expected an integer")
-                if value < cls.minimum_value:
-                    raise ValueError(f"Invalid value for '{cls.display_name}': element '{key}' maps to {value}, which is less than the minimum ({cls.minimum_value})")
+                    raise ValueError(
+                        f"Invalid type in '{cls.display_name}': "
+                        f"element '{key}' maps to '{value}', expected an integer"
+                    )
+                if value < cls.min:
+                    raise ValueError(
+                        f"Invalid value for '{cls.display_name}': "
+                        f"element '{key}' maps to {value}, which is less than the minimum ({cls.min})"
+                    )
+                if value > cls.max:
+                    raise ValueError(f"Invalid value for '{cls.display_name}': "
+                    f"element '{key}' maps to {value}, which is greater than the maximum ({cls.max})"
+                )
             return cls(data)
         else:
-            raise NotImplementedError(f"Cannot Convert from non-dictionary, got {type(data)}")
+            raise NotImplementedError(f"{cls.display_name}: Cannot convert from non-dictionary, got {type(data)}")
 
     def verify(self, world: Type['World'], player_name: str, plando_options: PlandoOptions) -> None:
         """Overridden version of function from Options.VerifyKeys for a better error message"""
@@ -974,15 +988,16 @@ def verify(self, world: Type['World'], player_name: str, plando_options: PlandoO
         self.value = new_value
         for item_name in self.value:
             if item_name not in world.item_names:
-                from .item import item_groups
                 picks = get_fuzzy_results(
                     item_name,
                     list(world.item_names) + list(item_groups.ItemGroupNames.get_all_group_names()),
                     limit=1,
                 )
-                raise Exception(f"Item {item_name} from option {self} "
-                                f"is not a valid item name from {world.game}. "
-                                f"Did you mean '{picks[0][0]}' ({picks[0][1]}% sure)")
+                raise Exception(
+                    f"Item {item_name} from option {self} "
+                    f"is not a valid item name from {world.game}. "
+                    f"Did you mean '{picks[0][0]}' ({picks[0][1]}% sure)"
+                )
 
     def get_option_name(self, value):
         return ", ".join(f"{key}: {v}" for key, v in value.items())
@@ -998,25 +1013,25 @@ def __len__(self) -> int:
 
 
 class Sc2StartInventory(Sc2ItemDict):
-    """Start with these items."""
+    """Start with these items. Use an amount of -1 to start with all copies of an item."""
     display_name = StartInventory.display_name
 
 
 class LockedItems(Sc2ItemDict):
     """Guarantees that these items will be unlockable, in the amount specified.
-    Specify an amount of 0 to lock all copies of an item."""
+    Specify an amount of -1 to lock all copies of an item."""
     display_name = "Locked Items"
 
 
 class ExcludedItems(Sc2ItemDict):
     """Guarantees that these items will not be unlockable, in the amount specified.
-    Specify an amount of 0 to exclude all copies of an item."""
+    Specify an amount of -1 to exclude all copies of an item."""
     display_name = "Excluded Items"
 
 
 class UnexcludedItems(Sc2ItemDict):
     """Undoes an item exclusion; useful for whitelisting or fine-tuning a category.
-    Specify an amount of 0 to unexclude all copies of an item."""
+    Specify an amount of -1 to unexclude all copies of an item."""
     display_name = "Unexcluded Items"
 
 

worlds/sc2/pool_filter.py

@@ -3,8 +3,7 @@
 
 from BaseClasses import Location, ItemClassification
 from .item import StarcraftItem, ItemFilterFlags, item_names, item_parents, item_groups
-from .item.item_tables import item_table, TerranItemType, ZergItemType, spear_of_adun_calldowns, \
-    spear_of_adun_castable_passives
+from .item.item_tables import item_table, TerranItemType, ZergItemType, spear_of_adun_calldowns
 from .options import RequiredTactics
 
 if TYPE_CHECKING:
@@ -272,7 +271,7 @@ def request_minimum_items(group: List[StarcraftItem], requested_minimum) -> None
         self.world.random.shuffle(spear_of_adun_actives)
         cull_items_over_maximum(spear_of_adun_actives, self.world.options.spear_of_adun_max_active_abilities.value)
 
-        spear_of_adun_autocasts = [item for item in inventory if item.name in spear_of_adun_castable_passives]
+        spear_of_adun_autocasts = [item for item in inventory if item.name in item_groups.spear_of_adun_passives]
         self.world.random.shuffle(spear_of_adun_autocasts)
         cull_items_over_maximum(spear_of_adun_autocasts, self.world.options.spear_of_adun_max_passive_abilities.value)