Options: Rework locked_items and excluded_items

[MatthewMarinets]

What feature would you like to see?

Rework item exclusions and locks to better deal with progressive items and better interact with triggers.

Changes

More expressive options

locked_items and excluded_items should change from an ItemSet to an OptionDict or OptionList class with good validation. OptionList has a .verify() method which may allow for complicated validation; if it is insufficient, an OptionDict is a good fallback option as it allows specifying a schema.

Ideally, the syntax may look something like this in yaml (with lists):

excluded_items:
  - Combat Shield (Marine)  # same as `Combat Shield (Marine): 0
  - Progressive Stimpack (Marine): 1  # excludes one level of that name
  - Progressive Stimpack (Marauder): 0  # excludes all levels
    Progressive Stimpack (Hellion): 2  # dict elements are appended to the parent list, so people can forget the dash safely
  - Battlecruiser  # list items can appear after dict items
  - Mercenaries: 1 # item groups should distribute the amount to all group members

This would be the most accepting syntax. The type annotation in Python would be:

type AcceptingCollection = list[str] | list[dict[str, int]] | dict[str, int]

This has a number of ways it could go wrong depending on existing core infrastructure:

• Forgetting the first - would turn the whole structure into a dict; there may be an earlier type check that would choke on that and prevent us from fixing it in our own validation methods
• The OptionSet / OptionList verify methods may not be expressive enough to allow for all item names and item groups

If things aren't working out, we can switch to an OptionDict as a backup and go with this syntax:

excluded_items:
  Combat Shield (Marine): 0
  Progressive Stimpack (Marauder): 1
  Battlecruiser: 1
  Mercenaries: 0

Which should still be plenty fine and expressive for our yaml writers, though it would be a breaking change.

Open question: if an item is specified multiple times (through groups, e.g. Stimpacks and Progressive Stimpack (Marine)), should the numbers add or override? Within a dict, specifying the same key multiple times overrides, I think.

Dealing with both locked and excluded

Currently, if an item is both locked and excluded, that's an error. This has to be reworked for this progressive change, as we may want to exclude 1 level of biosteel, lock 1 level, and leave 1 level to RNG. This is also irritating for yaml writers, who may want to make constrained unit sets additively, essentially supplying a unit whitelist rather than a blacklist.

To meet these requirements:

0. Convert from options; i.e. item.exclude_amount := item.quantity if option.exclude_amount[item] == 0 else option.exclude_amount[item]. If the item is not in the options, set the amount to 0. Do a similar operation for locked items.
1. For an item where item.exclude_amount + item.lock_amount <= item.quantity:

• Lock item.lock_amount copies of the item
• Exclude item.exclude_amount copies of the item
• In other words, the world should have a number of copies of the item in the range item.lock_amount <= item.amount <= item.quantity - item.exclude amount

2. For an item where item.exclude_amount + item.lock_amount > item.quantity:

• Issue a warning to stderr / stdout / the log, but continue
• item.lock_amount should "win". In other words, set item.exclude_amount := item.quantity - item.lock_amount
• Continue through section 1. with these updated values.

3. If item.lock_amount > item.quantity:

• Set item.lock_amount := item.quantity and item.exclude_amount := 0 and continue to section 1.

Consequences

The docs say OptionList, OptionSet, and OptionDict are not supported in the generated settings page. However, OptionSet seems to appear in the weighted settings page as it is right now (at least it does for excluded_missions). I'm not sure if this only works because we supply a complete list of valid keys, but we could possibly do that with our item lists (and exclude the non-user-facing item groups while we're at it).

OptionDict options don't seem to appear in weighted settings at all (at least they don't for Factorio).

The UX on the existing item and mission exclusions is already awful, as item groups go to the top, things are ordered by definition order (which may appear arbitrary to users), and the only search is page-wide, meaning you get results from all lists. I'm in favour of just having it be removed and telling people they must go through yaml to use mission / item exclusions and locks in the future.

[Ziktofel]

Well, for UX you don't want to look over all the 600 items to find one in the weights page

And surely, the Item count is the max, then locked should win against excluded (so if I both exclude and lock 2 levels of regen bio-steel, there'll be 2 levels in the world)

[MadiMadsen]

i think having it be a yaml only thing you will need to edit should be fine because most people making their first yaml for sc2 shouldn't be worrying about excluding/locking items.

[MatthewMarinets]

i think having it be a yaml only thing you will need to edit should be fine because most people making their first yaml for sc2 shouldn't be worrying about excluding/locking items.

We'll probably want to re-use this infrastructure for excluding / locking missions as well, though.

[Ziktofel]

For hiding options see ArchipelagoMW#3125

[MatthewMarinets]

Update: Tinkered around a little, and found that while the mixed syntax (lists containing dicts) is expressible with yaml and works fine, it doesn't work with triggers if you try to append a list to a dict or vice versa. As such, I'm falling back to just requiring these options to always be a dict.

To put it in terms of Python types, the ones on the table are:

• Mixed: Dict[str, int] | Iterable[str | Dict[str, int]]
• List of dicts: Iterable[str | Dict[str, int]]
• Dict: Dict[str, int]

List of dicts is still possible, but it would be finnicky and would probably behave even more oddly with triggers.

For posterity, this is the code I used to convert mixed syntax to a dict:

class ItemCountSet(OptionDict):
    """
    A helper class for options that take lists of items or dicts of items to integer amounts, or mixes of the two.
    List items get converted to dict items with value 1.
    """
    @classmethod
    def from_any(cls, value: Union[Dict[str, int], Iterable[Union[str, Dict[str, int]]]]):
        if isinstance(value, dict):
            formatted_value = value
        else:
            formatted_value: Dict[str, int] = {}
            for element in value:
                if isinstance(element, str):
                    formatted_value[element] = 0
                    continue
                for element_name, element_value in element.items():
                    formatted_value[element_name] = formatted_value.get(element_name, 0) + element_value
        return cls(formatted_value)

[MatthewMarinets]

Open question: how should excluded items on progressive items interact with exclude_nco_items? NCO exclude currently removes level 2 stimpack for all stimmable units. If someone specified to exclude NCO items and they excluded 1 level of stim, should that leave 1 level of stim, or remove both levels?

My personal preference is to just remove the various exclude_nco|wol|ext_items options in favour of one unified "exclude non-vanilla campaign items" option, and make it behave additively. Just so there's one easy switch for people that want the classic AP WoL experience, and anyone who wants something more specific can learn item groups (it's not like "ext" is very well-defined). It also cuts down on options bloat.

[Ziktofel]

I think that it should be reworked into some item groups as they overlap

If you don't want NCO group, but you pick WoL or BW, L1 stim should be available for appropriate units (Marine in WoL, Marine and Firebat in BW). The same is for Banshee's cross-spectrum dampeners

There's also one thing with excluding non vanilla items - you might fail logic for some locations on standard tactics (Evil Awoken fails without ext items currently on standard due to Disintegrating Particles or Particle Reflection due to lack of saves). However, more mastery locations could have this for Standard

However, "exclude non-vanilla campaign items" won't create option for playing NCO without NCO items (as they become vanilla campaign items)

[Ziktofel]

Maybe add an option to whitelist items/item groups, however, there might be technical difficulties with different level of upgrade belonging to different groups (Like WoL groups provides only L1 Marine Stimpack, while NCO provides both levels)

If empty - treat as everything is whitelisted, if filled, the player is given a union of those groups. While excluded_items is a blacklist, anything listed won't appear in game.

Thus:
A whitelisted item CAN appear in the game
A blacklisted item MUST NOT appear in the game
A locked item MUST appear in the game

Thinking about filtering whitelists and blacklists for logic requirements: Before culling, do similar for whitelists and blacklists - if an item is to be removed due to whitelist or blacklists, do a logic check, if it fails, don't remove - thus the generator will go against whitelists/blacklists only if it'd fail to generate instead.
This case should throw a warning

[MatthewMarinets]

Getting the yaml to accept a whitelist is as simple as specifying excluded_items: [AllUnits] and locked_items: <my_whitelist>.

With my current progress, I've only gotten as far as the explicit user options start_inventory, locked_items, excluded_items, and I'm treating their priority in that order. With PR #189 I should be able to get mission/campaign-based and other-option based excludes working nicely with this output, and I'll evaluate if I can just pass this list down the line for the other excludes or if I need to refactor those as well.

[Ziktofel]

With whitelists you can do:

Example 1:

whitelist_items:
  - WoL

This allows Marine but not Magrail Munitions (Marine)

Example 2:

whitelist_items:
  - NCO

This allows Marine but not Viking

Example 3:

whitelist_items:
  - WoL
excluded_items:
  - NCO

This allows Viking but not Marine (any item from WoL can spawn, while any item from NCO mustn't spawn). Marine won't appear even without the whitelist_items declaration

Example 4:

whitelist_items:
  - NCO
excluded_items:
  - WoL

This allows Liberator but not Marine. However, Marine can be shuffled back if logic requirements are failed in order to fulfill logic.

Example 5:

excluded_items:
  - Everything

This won't fail the generation outright, you'd get the bare minimum items in order to beat the game, chosen at random

[MatthewMarinets]

With whitelists you can do:

I'm beginning to see the appeal, especially with triggers in the mix. It does introduce some complexity and I'm a little on the fence as to whether it's worth it.

Note the default whitelist would have to be Everything, which would be our first list/dict option with a non-empty default value.

I see two use cases that would want this feature, and I don't think whitelists are the way to go for one of them:

1. Users who want to have a positive syntax for specifying the item pool without locking all those items in. "Positive" here means you're specifying what's in the pool rather than what's not in the pool, which plays nicer with trigger appending.
2. Users essentially expressing a complex / compound item group to be excluded (e.g. "exclude all WoL items except those that also appear in NCO"). I think this use-case is better served with an expression parser, so you could just exclude WoL-NCO or something. If done well, this can exclude very complex groups, like (Terran & Units) - (NCO & Starport) to include only NCO Starport units for terran. It's complicated, but I think users searching for this complexity would be better served with enumerations anyways.

I'll leave this out of my first draft as things are complicated enough already (start_inventory, excluded_items, locked_items all need to be resolve against each other just to start, then mission dependencies / item parentage dependencies, etc). I will, however, keep an eye on it so it shouldn't be too difficult to add in a later commit, either in the same PR or near future.

[Ziktofel]

The whitelist would be one of the easy ways to specify that the user wants to play NCO with WoL inventory

[Ziktofel]

Implementation-wise, you'd first exclude any item that doesn't follow the whitelist, then exclude any item that matches the blacklist, current excluded_items

[MatthewMarinets]

Implementation-wise, you'd first exclude any item that doesn't follow the whitelist, then exclude any item that matches the blacklist, current excluded_items

Remember locked items also exists, which with my proposed changes functions as a whitelist that overrides the excluded items. In my current branch, I have the priorities set up as start_inventory >> excluded_items >> locked_items. Having a whitelist adds another layer to that, which can get confusing but wouldn't be that harder to implement on top of things.

So you can functionally use the locked_items list as a whitelist for WoL-only NCO:

excluded_items:
  - Terran Units
  - NCO Upgrades
locked_items:
  - WoL Units

The only real advantage to having a whitelist is being able to specify the positive list without locking it. I'm not sure how common that's going to be, so I'd rather leave the whitelist off of the first PR and add the whitelist only if necessary.

[Ziktofel]

Locked WoL units will make that ALL WoL units will take place, even if there's not enough space, if you whitelist them instead, they can appear but are subject to pool filtering

[MatthewMarinets]

Thought more on whitelist while chipping away at PR #192.

I see two main problems with whitelist:

1. It doesn't play nice with unit upgrades. If I whitelist Marines, I'd probably expect marine upgrades to also get whitelisted. However, the proposed whitelist above doesn't specify whether marine upgrades would be whitelisted or not, and there probably won't be an option that can express both
2. It doesn't allow mixing whitelist / blacklist. Imagine wanting to specify terran with a blacklist (e.g. no marines or BCs), but protoss with a whitelist (four-legs only). A whitelist that changes the "mode" would have a global effect, rather than being scoped.

Initially I thought the answer was to have some complicated expressions in excludes, but will be complicated and error prone and not worth it. I've come up with a better, and quite simple solution: an "un-exclude" option.

This option will resolve after exclude. A player can get it to function like a whitelist by first excluding everything first. This also allows scoping: a player can, say "exclude all protoss units" then un-exclude a list of four-legged units. This removes the contamination problem. It also solves the upgrade problem, as upgrade excludes are resolved after excludes, unexcludes, and locks have been resolved, so adding a unit back in will keep the upgrades in the pool, and upgrades can still be excluded explicitly.

I may add this to PR #192 or add it in a later PR. #192 is already humongous and I have limited time to test it / work on it this week.

[Ziktofel]

For whitelists we can have groups like Terran, Zerg or Protoss

Just thinking about the use-case we had with NCO - using WoL inventory

I personally think that whitelist is more straightforward than un-exclude to understand what's going on

Maybe this think needs more opinions about that

[MatthewMarinets]

That usecase seems pretty straightforward: Exclude all terran units + NCO specific upgrades, un-exclude WoL units. As I said before, unexclude lets the player choose between whitelist / blacklist style independently between races, but it also lets them choose between other categories like units vs upgrades. So they can choose to either exclude all terran upgrades then unexclude WoL upgrades (whitelist style, purist WoL-only experience), or only exclude NCO upgrades (blacklist style, allows e.g. broodwar or co-op upgrades).

[MatthewMarinets]

Things seem mostly implemented since #192 merged. Main things that should still be done are in other repos -- adding descriptions to the item groups page and possibly adding some sample yaml triggers to the yaml repo.