Closed MatthewMarinets closed 2 weeks ago
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)
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.
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.
For hiding options see https://github.com/ArchipelagoMW/Archipelago/pull/3125
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:
Dict[str, int] | Iterable[str | Dict[str, int]]
Iterable[str | Dict[str, int]]
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)
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.
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)
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
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.
whitelist_items:
- WoL
This allows Marine
but not Magrail Munitions (Marine)
whitelist_items:
- NCO
This allows Marine
but not Viking
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
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.
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
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:
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.
The whitelist would be one of the easy ways to specify that the user wants to play NCO with WoL inventory
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
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.
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
Thought more on whitelist while chipping away at PR #192.
I see two main problems with whitelist:
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.
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
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).
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.
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
andexcluded_items
should change from anItemSet
to anOptionDict
orOptionList
class with good validation.OptionList
has a.verify()
method which may allow for complicated validation; if it is insufficient, anOptionDict
is a good fallback option as it allows specifying a schema.Ideally, the syntax may look something like this in yaml (with lists):
This would be the most accepting syntax. The type annotation in Python would be:
This has a number of ways it could go wrong depending on existing core infrastructure:
-
would turn the whole structure into adict
; there may be an earlier type check that would choke on that and prevent us from fixing it in our own validation methodsOptionSet
/OptionList
verify methods may not be expressive enough to allow for all item names and item groupsIf things aren't working out, we can switch to an
OptionDict
as a backup and go with this syntax: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
andProgressive Stimpack (Marine)
), should the numbers add or override? Within adict
, 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:
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.item.exclude_amount + item.lock_amount <= item.quantity
:item.lock_amount
copies of the itemitem.exclude_amount
copies of the itemitem.lock_amount <= item.amount <= item.quantity - item.exclude amount
item.exclude_amount + item.lock_amount > item.quantity
:item.lock_amount
should "win". In other words, setitem.exclude_amount := item.quantity - item.lock_amount
item.lock_amount > item.quantity
:item.lock_amount := item.quantity
anditem.exclude_amount := 0
and continue to section 1.Consequences
The docs say
OptionList
,OptionSet
, andOptionDict
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 forexcluded_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.