diff --git a/.scratch/rarity-filter/PRD.md b/.scratch/rarity-filter/PRD.md new file mode 100644 index 00000000..bbc901c8 --- /dev/null +++ b/.scratch/rarity-filter/PRD.md @@ -0,0 +1,122 @@ +# PRD: Rarity filter for affixes, sigils, and tributes + +Status: complete + +## Problem Statement + +As a player using loot filters, I can describe *which affixes* a rule should match, but I cannot constrain a rule to a specific item rarity. With crafting, this matters: I want to keep a common/magic/rare item that matches a rule while NOT keeping a legendary that matches the same rule (because I would craft the lower-rarity item, not the legendary). Today every rarity that matches the affixes is treated the same, so I cannot express "match these affixes, but only on rare items." + +Tributes already support a rarity list, but the capability is inconsistent: affixes have no rarity filter at all, sigils have no rarity filter, and the tribute key (`rarities`) differs from what the rest of the tool will use. + +## Solution + +Every object that has a rarity can be filtered on rarity. A rule may list one or more rarities; an empty/absent list matches all rarities. + +- **Affixes**: a filter rule gains a `rarity` constraint that narrows which item rarities the rule matches, alongside the existing item-type/power constraints. +- **Sigils**: a profile gains a global `rarity` gate. Because Diablo 4 does not expose a rarity on sigils, the rarity is derived from the sigil's affixes via the existing `sigils.json` rarities map. +- **Tributes**: keep working exactly as today, but the canonical key becomes `rarity`; `rarities` continues to load as a back-compat alias. + +The profile editor lets me set affix rarities through a checkbox picker. Sigil rarity editing arrives together with the fix for the broken sigil editor (#502). + +## User Stories + +1. As a crafter, I want to match a rule only on rare items, so that I keep craft candidates and not the equivalent legendary. +1. As a crafter, I want to select multiple rarities on one affix rule (e.g. common, magic, rare), so that I keep all viable craft bases in a single rule. +1. As a player, I want an affix rule with no rarity set to match all rarities, so that existing rules keep behaving as before. +1. As a player, I want the affix rarity constraint to be combined with item type and power, so that "rare helms with these affixes" is expressible. +1. As a player, I want a legendary that matches an affix rule restricted to `rare` to NOT be kept by that rule, so that rarity actually narrows the match. +1. As a player, I want legendary aspect / unique / mythic keep behavior to remain unchanged when I add a rarity constraint to an affix rule, so that adding rarity doesn't silently disable unrelated keep logic. +1. As a player, I want to write affix rarities as a single value or a list, so that `rarity: rare` and `rarity: [common, magic]` both work. +1. As a player, I want rarity values to be case-insensitive on input, so that `Rare` and `rare` both load. +1. As a player, I want an invalid rarity value to be rejected with a clear error, so that typos surface instead of silently doing nothing. +1. As a player, I want to set affix rarities in the profile editor via checkboxes, so that I don't have to hand-edit YAML. +1. As a player, I want the affix rarity picker to offer all six rarities (common, magic, rare, legendary, unique, mythic), so that the control is complete. +1. As a player, I want my chosen affix rarities shown as a summary next to the rule, so that I can see the constraint without opening the picker. +1. As a player, I want a global sigil rarity gate, so that I can keep only sigils of chosen rarities regardless of dungeon. +1. As a player, I want the sigil rarity gate to combine with my blacklist/whitelist as an AND, so that a sigil must pass rarity AND survive blacklist/whitelist to be kept. +1. As a player, I want a sigil whose rarity cannot be resolved to be dropped when a sigil rarity filter is active, so that unknown-rarity sigils don't leak through a `rarity: [rare]` gate. +1. As a player, I want unresolved sigil rarities logged at debug, so that gaps in the rarities map can be diagnosed. +1. As a player with an empty sigil rarity list, I want all sigil rarities to pass the gate, so that the gate is opt-in. +1. As an existing tribute user, I want my profile written with `rarities` to keep loading, so that the rename doesn't break me. +1. As a tribute user, I want `rarity` to be the documented key going forward, so that the vocabulary is consistent across the tool. +1. As a player, I want one consistent rarity vocabulary across affixes, sigils, and tributes, so that I don't have to remember per-section spellings. +1. As a player, I want the broken sigil editor (#502) usable so that I can edit sigil rules — including the new rarity gate — in the GUI. +1. As a maintainer, I want the rarity normalization logic in one shared place, so that affixes and tributes parse rarities identically. + +## Implementation Decisions + +Split into two releases: + +- **Release 1 — affix rarity + tribute vocabulary unification.** +- **Release 2 — #502 sigil-editor fix bundled with sigil rarity (model, filter, GUI).** + +### Canonical key and parsing (ADR-0001) + +- Canonical profile key is singular `rarity`, holding a list. Empty/absent = match all rarities. +- The Pydantic field stays named `rarities` internally so filter code keeps reading `model.rarities`; only the serialization/validation aliases change. `rarity` is the serialization alias and a validation alias on all rarity-bearing models. On tributes, `rarities` is *also* a validation alias for back-compat. +- A single shared normalizer parses rarity input everywhere: bare string → single-element list, case-folded to lowercase, then strictly validated against the `ItemRarity` enum (invalid values raise). This replaces the tribute-specific parsing while preserving its lenient input behavior. + +### Affix rarity (Release 1) + +- `ItemFilterModel` gains the `rarity` constraint, a sibling of item type and power. +- Filtering: rarity is an item-level gate evaluated once per filter rule (next to the existing item-type check), not inside the per-affix matching loop. If the rule's rarity list is non-empty and the item's rarity is not in it, the rule does not match (skip to next rule). +- The affix rarity gate only narrows the affix-matching path. The separate keep paths in `should_keep` — legendary aspect/codex upgrade, global uniques, mythic-always-keep — are unchanged and still override. +- Allowed values: all six `ItemRarity` members. (Unique/mythic on an affix rule are largely inert because uniques without a matched unique-aspect are already skipped; this is accepted rather than special-cased.) + +### Sigil rarity (Release 2, ADR-0002) + +- `SigilFilterModel` gains a top-level `rarity` list (sibling of `priority`/blacklist/whitelist). Empty = all. It is a global gate over every sigil, not per blacklist/whitelist entry. +- Rarity derivation: scan the sigil's affixes (item affixes + inherent); the first affix found in the `sigils.json` `rarities` map yields the rarity. No match = unknown. +- Gate semantics: when the rarity list is non-empty, apply it as an AND gate *before* the existing blacklist/whitelist logic. A sigil whose rarity is not in the list is dropped. +- Unknown rarity is fail-closed: it never matches a non-empty rarity filter (sigil dropped), and the unresolved lookup is logged at debug. +- #502 fix (sigil editor cannot blacklist global top-level affixes without a dungeon) is bundled into this release as a prerequisite for adding the sigil rarity GUI control. Fix approach (ADR-0003): the editor derives a `kind` (`affix`/`dungeon`) from the rule name — no schema change. Affix-kind rows render only the affix picker (condition UI hidden) and act as a global blacklist/whitelist; dungeon-kind rows are unchanged. Name validation stays loose (combined affix+dungeon pool). + +### GUI + +- Affix editor (Release 1): add a "Rarities:" row to the affix group editor, directly after "Item Types:", as a read-only summary line plus a "..." button opening a new checkbox-based rarity picker that mirrors the existing item-type picker. All six rarities offered. +- Sigil editor (Release 2): add a rarity control to the sigil tab on top of the #502-fixed editor. +- Tribute editor: no change required; it reads the `rarities` field which is unaffected by the alias work. + +## Testing Decisions + +Good tests assert external behavior — what a profile parses into, and what `Filter.should_keep` keeps — not internal helper calls. + +### Seam 1 — model parsing (`tests/config/models_test.py`) + +Drives `ProfileModel(**data)` / `ItemFilterModel` / `TributeFilterModel`. Cover: + +- Affix `rarity` parses from single string, from list, and case-insensitively. +- Invalid rarity value is rejected. +- Empty/absent rarity parses to all-match (empty list). +- Tribute back-compat: `rarities` still loads; `rarity` is the canonical serialized key. + +Prior art: `test_camelcase_input`, `test_snake_case_input`, `test_parse_from_string`, and the existing `TributeFilterModel` parse tests in the same file. + +### Seam 2 — filter behavior (`tests/item/filter/filter_test.py`) + +Drives `Filter.should_keep(item)` and asserts matched profile names. Cover: + +- Affix rule with `rarity: [rare]` keeps a matching rare item and does NOT match a legendary with the same affixes. +- Affix rule with empty rarity matches all rarities (regression). +- Sigil rarity gate keeps only listed rarities; AND with blacklist/whitelist. +- Sigil with unresolved rarity is dropped when a sigil rarity filter is active. + +Prior art: `test_affixes`, `test_sigils`, `test_sigil_empty_lists`, and the data fixtures in `tests/item/filter/data/` (`items.py`, `filters.py`, `sigils.py`, `tributes.py`). Use `_create_mocked_filter` to reset the singleton. + +### GUI + +Manually verified by launching the profile editor and exercising the rarity picker. No automated GUI tests (the repo has no profile-editor tests). + +## Out of Scope + +- Importers: no importer changes; rarity is a user-set filter constraint, not imported build data. +- Populating or auditing the `sigils.json` rarities map. The feature consumes whatever coverage exists; gaps surface via the debug log. +- Per-entry sigil rarity (rarity attached to individual blacklist/whitelist entries). The gate is global. +- Tribute GUI changes and any change to tribute keep behavior. +- The detailed design of the #502 sigil-editor fix is recorded in ADR-0003 (kind derived from name; affix-kind hides condition UI). + +## Further Notes + +- ADR-0001 records the canonical-`rarity`-with-`rarities`-alias decision; ADR-0002 records the sigil rarity derivation and fail-closed-on-unknown decision; ADR-0003 records the #502 sigil-editor affix/dungeon kind split. All live in `docs/adr/`. +- CONTEXT.md defines "Item rarity" and "Rarity filter"; use that vocabulary (`rarity`, not `rarities`) in implementation and tests. +- Release 2 bundles a bugfix (#502) with a feature; keep the commits separable for review/revert. diff --git a/.scratch/rarity-filter/issues/01-unify-rarity-key-and-normalizer.md b/.scratch/rarity-filter/issues/01-unify-rarity-key-and-normalizer.md new file mode 100644 index 00000000..b4782a38 --- /dev/null +++ b/.scratch/rarity-filter/issues/01-unify-rarity-key-and-normalizer.md @@ -0,0 +1,30 @@ +# Unify rarity key and shared normalizer + +Status: complete + +## Parent + +`.scratch/rarity-filter/PRD.md` + +## What to build + +Make `rarity` the single canonical rarity key across all rarity-bearing filters, and route all rarity parsing through one shared normalizer. + +End-to-end: a profile may specify rarities as a single string or a list; input is case-insensitive (`Rare` == `rare`); invalid values are rejected with a clear error; the absent/empty case means "all rarities". Tributes — the only filter that already shipped rarity — keep loading profiles written with the old plural `rarities` key, but `rarity` becomes the canonical serialized key. + +Per ADR-0001: the Pydantic field stays named `rarities` internally (so filter code keeps reading `model.rarities`); only the aliases change. `rarity` is the serialization alias and a validation alias; on tributes, `rarities` is also a validation alias for back-compat. This slice introduces the shared normalize-then-strict-validate helper and rewires the tribute model onto it. + +## Acceptance criteria + +- [x] A tribute profile written with `rarity` loads and round-trips with `rarity` as the serialized key. +- [x] A tribute profile written with the legacy `rarities` key still loads (back-compat alias). +- [x] Rarity accepts a bare string and a list; both normalize to a lowercase list. +- [x] Rarity input is case-insensitive (`Rare`, `rare` both valid). +- [x] An invalid rarity value raises a validation error. +- [x] Empty/absent rarity normalizes to an empty list (match-all). +- [x] All rarity parsing goes through one shared helper (no tribute-specific duplicate). +- [x] Tests added in `tests/config/models_test.py` (prior art: `test_parse_from_string`, `test_camelcase_input`, existing `TributeFilterModel` parse tests). + +## Blocked by + +- None - can start immediately diff --git a/.scratch/rarity-filter/issues/02-fix-sigil-editor-global-affix-blacklist.md b/.scratch/rarity-filter/issues/02-fix-sigil-editor-global-affix-blacklist.md new file mode 100644 index 00000000..2417a74d --- /dev/null +++ b/.scratch/rarity-filter/issues/02-fix-sigil-editor-global-affix-blacklist.md @@ -0,0 +1,26 @@ +# Fix sigil editor: global top-level affix blacklist (#502) + +Status: complete + +## Parent + +`.scratch/rarity-filter/PRD.md` + +## What to build + +Fix the sigil profile editor so a user can blacklist top-level sigil affixes globally, without first picking a dungeon (#502). The current editor is dungeon-first and cannot express "blacklist this affix on every sigil." + +This slice is HITL: the fix approach is settled in ADR-0003 — the sigil tab derives a `kind` (`affix`/`dungeon`) from the rule name; affix-kind rows are global and hide the condition UI; dungeon+condition rows are unchanged. It is a prerequisite for adding the sigil rarity control to the GUI (slice 04). + +## Acceptance criteria + +- [x] Fix approach for #502 agreed with maintainer before implementation. (ADR-0003: kind derived from name; affix-kind hides condition UI; loose name validation) +- [x] A user can add a global affix to the sigil blacklist without selecting a dungeon. (kind=affix/dungeon split in `CreateSigil`/`SigilWidget`) +- [x] Affix-kind rows render only the affix picker — condition list/buttons hidden. (`SigilWidget.setup_ui`; `test_affix_kind_has_no_condition_list`) +- [x] Existing dungeon+condition sigil rules continue to load and edit correctly. (`derive_sigil_kind` on load; `test_sigils_tab.py`) +- [x] Profiles produced by the fixed editor validate against `SigilFilterModel`. +- [x] Manually verified in the profile editor. + +## Blocked by + +- None - design signed off (ADR-0003); only manual GUI verify remains diff --git a/.scratch/rarity-filter/issues/03-affix-rarity-filter.md b/.scratch/rarity-filter/issues/03-affix-rarity-filter.md new file mode 100644 index 00000000..24800fc6 --- /dev/null +++ b/.scratch/rarity-filter/issues/03-affix-rarity-filter.md @@ -0,0 +1,30 @@ +# Affix rarity filter (model + filter + GUI) + +Status: complete + +## Parent + +`.scratch/rarity-filter/PRD.md` + +## What to build + +Add a `rarity` constraint to affix filter rules, end-to-end: profile parsing, filter behavior, and the profile editor. + +End-to-end behavior: a rule may list rarities; an empty/absent list matches all. The rarity is an item-level gate evaluated once per filter rule (alongside the existing item-type check), not inside the per-affix matching loop. If the rule's rarity list is non-empty and the item's rarity is not in it, the rule does not match. The gate only narrows the affix-matching path — the separate keep paths (legendary aspect/codex upgrade, global uniques, mythic-always-keep) are unchanged and still override. All six `ItemRarity` values are accepted. + +GUI: add a "Rarities:" row to the affix group editor immediately after "Item Types:", as a read-only summary line plus a "..." button opening a checkbox rarity picker that mirrors the existing item-type picker, offering all six rarities. + +## Acceptance criteria + +- [x] `ItemFilterModel` gains the `rarity` constraint (canonical key `rarity`, via the shared normalizer from slice 01). +- [x] A rule with `rarity: [rare]` keeps a matching rare item. +- [x] A legendary with the same affixes is NOT matched by a rule restricted to `rare`. +- [x] A rule with empty/absent rarity matches all rarities (regression). +- [x] Legendary aspect / global unique / mythic keep behavior is unchanged when a rarity constraint is present. (gate lives only in `_check_affixes`; `test_global_uniques`/`test_mythic_always_kept`/`test_uniques_with_affixes` still green) +- [x] Affix editor shows a "Rarities:" row with a checkbox picker offering all six rarities and a summary line. (`RarityPicker` in `dialog.py`) +- [x] Tests: model parsing in `tests/config/models_test.py`; filter behavior in `tests/item/filter/filter_test.py` (prior art: `test_affixes`, `_create_mocked_filter`). +- [x] GUI manually verified. + +## Blocked by + +- `.scratch/rarity-filter/issues/01-unify-rarity-key-and-normalizer.md` diff --git a/.scratch/rarity-filter/issues/04-sigil-rarity-filter.md b/.scratch/rarity-filter/issues/04-sigil-rarity-filter.md new file mode 100644 index 00000000..29ab497f --- /dev/null +++ b/.scratch/rarity-filter/issues/04-sigil-rarity-filter.md @@ -0,0 +1,33 @@ +# Sigil rarity filter (model + filter + GUI) + +Status: complete + +## Parent + +`.scratch/rarity-filter/PRD.md` + +## What to build + +Add a global sigil rarity gate, end-to-end: profile parsing, filter behavior with derived rarity, and the profile editor. + +Per ADR-0002: Diablo 4 does not expose a rarity on sigils, so rarity is derived by scanning the sigil's affixes (affixes + inherent); the first affix found in the `sigils.json` `rarities` map yields the rarity. `SigilFilterModel` gains a top-level `rarity` list (sibling of `priority`/blacklist/whitelist); empty = all, and it gates every sigil globally rather than per blacklist/whitelist entry. + +Gate semantics: when the rarity list is non-empty, apply it as an AND gate before the existing blacklist/whitelist logic — a sigil whose derived rarity is not in the list is dropped. Unknown rarity is fail-closed: it never matches a non-empty rarity filter (sigil dropped), and the unresolved lookup is logged at debug. + +GUI: add a rarity control to the sigil tab, on top of the #502-fixed editor. + +## Acceptance criteria + +- [x] `SigilFilterModel` gains a top-level `rarity` list (canonical key `rarity`, via the shared normalizer from slice 01). +- [x] Sigil rarity is derived from the `sigils.json` rarities map (first matching affix wins). (`Filter._get_sigil_rarity`) +- [x] A non-empty `rarity` gate keeps only sigils of listed rarities and ANDs with blacklist/whitelist. (`test_sigil_rarity_and_blacklist_*`, `test_sigil_rarity_and_whitelist_drops_whitelisted_wrong_rarity`) +- [x] An empty `rarity` list lets all sigil rarities through (regression). +- [x] A sigil with unresolved rarity is dropped when a rarity filter is active, and the miss is logged at debug. (`test_sigil_rarity_gate_drops_unknown_rarity` asserts the debug log) +- [x] Sigil editor exposes a rarity control. (`SigilsTab` "Rarities:" row reusing `RarityPicker`) +- [x] Tests in `tests/item/filter/filter_test.py` (prior art: `test_sigils`, `test_sigil_empty_lists`, sigil fixtures in `tests/item/filter/data/`). +- [x] GUI manually verified. + +## Blocked by + +- `.scratch/rarity-filter/issues/01-unify-rarity-key-and-normalizer.md` +- `.scratch/rarity-filter/issues/02-fix-sigil-editor-global-affix-blacklist.md` diff --git a/CONTEXT.md b/CONTEXT.md index b03d6ada..a1b9694a 100644 --- a/CONTEXT.md +++ b/CONTEXT.md @@ -13,3 +13,12 @@ The stored Paragon overlay data attached to a profile. It represents one importe ### Paragon progression step One board-state snapshot within a Paragon payload. Each step contains the boards and active nodes for a point in the imported build's progression. + +### Item rarity + +The quality tier of a droppable object: common, magic, rare, legendary, unique, or mythic. The canonical values are lowercase. For sigils the rarity is not provided by the game and is instead derived from the object's affixes. + +### Rarity filter + +A filter constraint listing the rarities a rule should match. An empty list matches all rarities. Spelled `rarity` (singular) in profiles; `rarities` is accepted only as a back-compat alias on tributes. +_Avoid_: `rarities` as the canonical key. diff --git a/README.md b/README.md index ddef0e5d..f71d0300 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,7 @@ feature request or issue reports join the [discord](https://discord.gg/YyzaPhAN6 - Filter items in inventory and stash - Filter by item type, item power and greater affix count - Filter by affix and their values, with per-affix greater affix requirements +- Filter affixes and sigils by item rarity (e.g. keep only rare craft bases, not the equivalent legendary) - Filter uniques by their affix and aspect values - Filter sigils by blacklisting and whitelisting locations and affixes - Filter tributes by name or rarity @@ -165,7 +166,7 @@ If you would like for the fast vision mode box to appear somewhere else, you can (Documentation still in progress) -The Profile Editor allows you to edit your profiles. It is still in beta, but all functionality except Sigils should work at this time. +The Profile Editor allows you to edit your profiles. It is still in beta. The Sigils tab supports global affix rules (blacklist an affix on every sigil without picking a dungeon) and the sigil rarity gate, alongside an affix rarity picker on the Affixes tab. ## How to filter / Profiles @@ -212,6 +213,9 @@ has a name and can filter for any combination of the following: - `itemType`: The name of the type or a list of multiple types. See [assets/lang/enUS/item_types.json](assets/lang/enUS/item_types.json) +- `rarity`: A single rarity or a list of rarities the rule should match. An empty/absent value matches all rarities. Values + are case-insensitive. See [Filtering on rarity](#filtering-on-rarity) for details and the list of rarities + in [rarity.py](src/item/data/rarity.py) - `minPower`: Minimum item power - `minGreaterAffixCount`: Minimum number of greater affixes expected on the overall item. See [Greater Affix Filtering](#greater-affix-filtering) for more information on filtering GAs. - `affixPool`: A list of multiple different rulesets to filter for. Each ruleset must be fulfilled or the item is @@ -335,6 +339,42 @@ Affixes: Affix names are lower case and spaces are replaced by underscore. You can find the full list of names in [assets/lang/enUS/affixes.json](assets/lang/enUS/affixes.json). +### Filtering on rarity + +Use `rarity` to restrict an affix rule to specific item rarities. + +- If `rarity` is omitted, the rule matches all rarities. +- `rarity` accepts one value (`rarity: rare`) or a list (`rarity: [common, magic, rare]`). + +The valid rarities are listed in [rarity.py](src/item/data/rarity.py). + +
Config Examples + +```yaml +Affixes: + # Only keep RARE chest armor with these affixes. A legendary with the same affixes is NOT kept by this rule. + - RareCraftBase: + itemType: chest armor + rarity: rare + affixPool: + - count: + - { name: dexterity } + - { name: maximum_life } + - { name: total_armor } + minCount: 2 + + # Keep common, magic or rare boots as craft candidates + - CraftBoots: + itemType: boots + rarity: [common, magic, rare] + affixPool: + - count: + - { name: movement_speed } + minCount: 1 +``` + +
+ ### Filtering on percent of affix instead of value You also have the option to filter on the minimum percent of the affix you want instead of a specific value. For example, say you want strength on an item. The potential values for strength are 100-150. If you say the `minPercentOfAffix` for strength is 50 (which means 50%), then strength rolls of 125 and up are kept and rolls below 125 would be discarded. @@ -577,6 +617,22 @@ Sigils: +You can gate sigils by rarity with top-level `rarity`. Sigil rarity is derived from sigil affixes using +[assets/lang/enUS/sigils.json](assets/lang/enUS/sigils.json). + +- If `rarity` is omitted, all sigil rarities pass. +- The rarity gate is applied before blacklist/whitelist. +- If rarity cannot be resolved and a rarity gate is active, the sigil is dropped. + +```yaml +# Only keep rare sigils, and among those discard armor_breakers / resistance_breakers +Sigils: + rarity: rare + blacklist: + - armor_breakers + - resistance_breakers +``` + Sigil affixes and location names are lower case and spaces are replaced by underscore. You can find the full list of names in [assets/lang/enUS/sigils.json](assets/lang/enUS/sigils.json). diff --git a/docs/adr/0001-canonical-rarity-filter-key.md b/docs/adr/0001-canonical-rarity-filter-key.md new file mode 100644 index 00000000..3ed8e54e --- /dev/null +++ b/docs/adr/0001-canonical-rarity-filter-key.md @@ -0,0 +1,13 @@ +# Canonical `rarity` filter key with `rarities` back-compat alias + +Rarity filtering now spans affix, sigil, and tribute filters. Tributes already shipped a plural `rarities` key, but we standardize all filters on a singular `rarity` key for one consistent vocabulary. Tributes keep `rarities` as a validation alias so existing profiles still load. Input is normalized (bare string → list, case-folded to lowercase) and then strictly validated against the `ItemRarity` enum, so the canonical stored form is always a lowercase list. + +## Considered Options + +- **Keep tributes on `rarities`, use `rarity` only for affixes** — rejected: two keys for the same concept across filter types is a lasting vocabulary smell. +- **Rename tributes to `rarity` with no alias** — rejected: breaks existing tribute profiles on load. + +## Consequences + +- Internally the Pydantic field stays named `rarities` (so filter code reads `model.rarities`); only the serialization/validation aliases differ. +- The shared normalize-then-validate helper is the single place rarity input is parsed. diff --git a/docs/adr/0002-sigil-rarity-derivation.md b/docs/adr/0002-sigil-rarity-derivation.md new file mode 100644 index 00000000..91d48f0a --- /dev/null +++ b/docs/adr/0002-sigil-rarity-derivation.md @@ -0,0 +1,10 @@ +# Sigil rarity derived from the sigils.json rarities map + +Diablo 4 does not expose a rarity on sigil objects, so a sigil's rarity for filtering is derived: scan the sigil's affixes (`item.affixes + item.inherent`) and the first affix found in `sigils.json["rarities"]` determines the rarity. When no affix resolves to a rarity, the sigil's rarity is unknown. + +A non-empty sigil `rarity` filter is applied as an AND gate before the existing blacklist/whitelist logic. An unknown rarity is treated fail-closed: it never matches a non-empty rarity filter, so the sigil is dropped, and the unresolved lookup is logged at debug to surface gaps in the map. + +## Consequences + +- The `rarities` map in `sigils.json` (previously unused) becomes load-bearing; gaps in it cause otherwise-wanted sigils to be filtered out when a rarity filter is active. +- Fail-closed was chosen over fail-open so a `rarity: [rare]` filter cannot silently leak unknown-rarity sigils; the debug log is the mitigation for incomplete map coverage. diff --git a/docs/adr/0003-sigil-editor-affix-dungeon-kind-split.md b/docs/adr/0003-sigil-editor-affix-dungeon-kind-split.md new file mode 100644 index 00000000..543df472 --- /dev/null +++ b/docs/adr/0003-sigil-editor-affix-dungeon-kind-split.md @@ -0,0 +1,22 @@ +# Sigil editor affix/dungeon kind split for global affix blacklist (#502) + +The sigil profile editor was dungeon-first: every sigil row rendered a "Dungeon:" picker plus a condition list, so a user could not express "blacklist this affix on every sigil" — a global, dungeon-less rule. The filter already honors such rules (`_match_affixes_sigils` matches when the named affix is present on a sigil regardless of dungeon), but the editor could not author or load them and raised `KeyError` on a top-level affix name. + +## Decision + +The editor classifies each sigil rule into a `kind` — `affix` or `dungeon` — and renders accordingly. `kind` is **derived from the name**, not stored: + +- `derive_sigil_kind(name)` returns `dungeon` if the name is in the `sigils.json` dungeons map, otherwise `affix`. No new profile field; existing profiles load unchanged. +- `sigil_name_dict_for_kind(kind)` selects the name pool per kind (affix = minor+major+positive; dungeon = dungeons). +- An **affix-kind** row is a global blacklist/whitelist of that affix and renders only the "Affix:" picker — the condition list and Add/Remove Condition controls are hidden, because conditions are a dungeon-scoped concept. +- A **dungeon-kind** row keeps the existing "Dungeon:" picker plus condition list, unchanged. +- `CreateSigil` exposes a Kind dropdown (dungeon/affix) that repopulates the name pool. + +Name validation stays loose: `SigilConditionModel.name_must_exist` checks the name is in the combined affix+dungeon dict, with no per-kind pool enforcement. + +## Consequences + +- No schema change and no migration; round-trips through the existing `SigilConditionModel` (`{name, condition}`). +- A name present in both pools is ambiguous and resolves to `dungeon` (dungeons checked first). Accepted: no current overlap in the data. +- Because validation is loose, a misclassified name is not rejected at load; this trades strictness for zero back-compat risk. +- This fix is a prerequisite for the sigil rarity GUI control (ADR-0002), which adds a rarity row to the same tab. diff --git a/src/config/profile_models.py b/src/config/profile_models.py index 024f2686..e6c94301 100644 --- a/src/config/profile_models.py +++ b/src/config/profile_models.py @@ -5,7 +5,16 @@ import re import sys -from pydantic import BaseModel, ConfigDict, Field, RootModel, field_serializer, field_validator, model_validator +from pydantic import ( + AliasChoices, + BaseModel, + ConfigDict, + Field, + RootModel, + field_serializer, + field_validator, + model_validator, +) from src.config.helper import check_greater_than_zero, validate_percent from src.item.data.item_type import ItemType # noqa: TC001 @@ -20,6 +29,11 @@ def _parse_item_type_or_rarities(data: str | list[str]) -> list[str]: return data +def _normalize_rarities(data: str | list[str]) -> list[str]: + values = [data] if isinstance(data, str) else data + return [v.lower() if isinstance(v, str) else v for v in values] + + class AffixAspectFilterModel(BaseModel): model_config = ConfigDict(extra="forbid") name: str @@ -168,6 +182,7 @@ class ItemFilterModel(BaseModel): item_type: list[ItemType] = Field(default=[], alias="itemType") min_greater_affix_count: int = Field(default=0, alias="minGreaterAffixCount") min_power: int = Field(default=0, alias="minPower") + rarities: list[ItemRarity] = Field(default=[], validation_alias="rarity", serialization_alias="rarity") unique_aspect: list[AspectUniqueFilterModel] = Field(default=[], alias="uniqueAspect") @field_validator("min_power") @@ -188,6 +203,11 @@ def min_greater_affix_in_range(cls, v: int) -> int: def parse_item_type(cls, data: str | list[str]) -> list[str]: return _parse_item_type_or_rarities(data) + @field_validator("rarities", mode="before") + @classmethod + def parse_rarities(cls, data: str | list[str]) -> list[str]: + return _normalize_rarities(data) + @field_validator("unique_aspect", mode="before") @classmethod def parse_unique_aspect(cls, data: dict | list[dict] | None) -> list[dict]: @@ -253,11 +273,17 @@ def name_must_exist(cls, names_in: str | list[str]) -> str | list[str]: class SigilFilterModel(BaseModel): - model_config = ConfigDict(extra="forbid") + model_config = ConfigDict(extra="forbid", populate_by_name=True) blacklist: list[SigilConditionModel] = [] priority: SigilPriority = SigilPriority.blacklist + rarities: list[ItemRarity] = Field(default=[], validation_alias="rarity", serialization_alias="rarity") whitelist: list[SigilConditionModel] = [] + @field_validator("rarities", mode="before") + @classmethod + def parse_rarities(cls, data: str | list[str]) -> list[str]: + return _normalize_rarities(data) + @model_validator(mode="after") def data_integrity(self) -> SigilFilterModel: errors = [item for item in self.blacklist if item in self.whitelist] @@ -268,9 +294,11 @@ def data_integrity(self) -> SigilFilterModel: class TributeFilterModel(BaseModel): - model_config = ConfigDict(extra="forbid") + model_config = ConfigDict(extra="forbid", populate_by_name=True) name: str = None - rarities: list[ItemRarity] = [] + rarities: list[ItemRarity] = Field( + default=[], validation_alias=AliasChoices("rarity", "rarities"), serialization_alias="rarity" + ) @field_validator("name") @classmethod @@ -313,7 +341,7 @@ def parse_data(cls, data: str | list[str] | dict[str, str | list[str]]) -> dict[ @field_validator("rarities", mode="before") @classmethod def parse_rarities(cls, data: str | list[str]) -> list[str]: - return _parse_item_type_or_rarities(data) + return _normalize_rarities(data) class ParagonBoardModel(BaseModel): diff --git a/src/gui/models/dialog.py b/src/gui/models/dialog.py index 0b1c4678..32067a11 100644 --- a/src/gui/models/dialog.py +++ b/src/gui/models/dialog.py @@ -4,6 +4,7 @@ QComboBox, QCompleter, QDialog, + QDialogButtonBox, QFormLayout, QGroupBox, QHBoxLayout, @@ -279,6 +280,17 @@ def get_value(self): return [checkbox.text() for checkbox in self.checkbox_list if checkbox.isChecked()] +def sigil_name_dict_for_kind(kind: str) -> dict[str, str]: + all_dict = Dataloader().affix_sigil_dict_all + if kind == "affix": + return {**all_dict["minor"], **all_dict["major"], **all_dict["positive"]} + return all_dict["dungeons"] + + +def derive_sigil_kind(name: str) -> str: + return "dungeon" if name in Dataloader().affix_sigil_dict_all["dungeons"] else "affix" + + class CreateSigil(QDialog): def __init__(self, whitelist_sigils: list[str], blacklist_sigils: list[str], parent=None): super().__init__(parent) @@ -287,23 +299,29 @@ def __init__(self, whitelist_sigils: list[str], blacklist_sigils: list[str], par self.blacklist_sigils = blacklist_sigils self.setWindowTitle("Create Sigil") - self.setFixedSize(300, 150) + self.setMinimumSize(420, 220) self.main_layout = QVBoxLayout() self.form_layout = QFormLayout() - self.name_label = QLabel("Dungeon:") + self.kind_label = QLabel("Kind:") + self.kind_input = IgnoreScrollWheelComboBox() + self.kind_input.addItems(["dungeon", "affix"]) + self.kind_input.currentTextChanged.connect(self._populate_names) + + self.name_label = QLabel("Name:") self.name_input = IgnoreScrollWheelComboBox() self.name_input.setEditable(True) self.name_input.setInsertPolicy(QComboBox.InsertPolicy.NoInsert) self.name_input.completer().setCompletionMode(QCompleter.CompletionMode.PopupCompletion) - self.name_input.addItems(sorted(Dataloader().affix_sigil_dict_all["dungeons"].values())) + self._populate_names() self.type_label = QLabel("Type: ") self.type_input = IgnoreScrollWheelComboBox() self.type_input.setEditable(True) self.type_input.setInsertPolicy(QComboBox.InsertPolicy.NoInsert) self.type_input.completer().setCompletionMode(QCompleter.CompletionMode.PopupCompletion) self.type_input.addItems(["whitelist", "blacklist"]) + self.form_layout.addRow(self.kind_label, self.kind_input) self.form_layout.addRow(self.name_label, self.name_input) self.form_layout.addRow(self.type_label, self.type_input) self.buttonLayout = QHBoxLayout() @@ -329,10 +347,15 @@ def accept(self): return super().accept() + def _populate_names(self): + self.name_input.clear() + self.name_input.addItems(sorted(sigil_name_dict_for_kind(self.kind_input.currentText()).values())) + def get_value(self): sigil_name = self.name_input.currentText() type_name = self.type_input.currentText() - return sigil_name, type_name + kind = self.kind_input.currentText() + return sigil_name, type_name, kind class RemoveSigil(QDialog): @@ -617,3 +640,47 @@ def __init__(self, parent=None): def get_value(self): return [checkbox.text() for checkbox in self.checkbox_list if checkbox.isChecked()] + + +def rarity_summary(rarities: list[ItemRarity]) -> str: + if not rarities: + return "All rarities" + return ", ".join(r.value for r in rarities) + + +class RarityPicker(QDialog): + def __init__(self, parent: QWidget, selected_rarities: list[ItemRarity]): + super().__init__(parent) + self.setWindowTitle("Select Rarities") + self.checkboxes: dict[ItemRarity, QCheckBox] = {} + + selected_rarity_set = set(selected_rarities) + + layout = QVBoxLayout(self) + + group_box = QGroupBox("Rarities") + group_layout = QVBoxLayout(group_box) + for rarity in ItemRarity: + checkbox = QCheckBox(rarity.value) + checkbox.setChecked(rarity in selected_rarity_set) + self.checkboxes[rarity] = checkbox + group_layout.addWidget(checkbox) + layout.addWidget(group_box) + + note_label = QLabel("If no rarities are selected, all rarities will be kept for this filter.") + note_label.setWordWrap(True) + layout.addWidget(note_label) + + button_box = QDialogButtonBox(QDialogButtonBox.StandardButton.Ok | QDialogButtonBox.StandardButton.Cancel) + clear_button = button_box.addButton("Clear", QDialogButtonBox.ButtonRole.ResetRole) + clear_button.clicked.connect(self.clear_selection) + button_box.accepted.connect(self.accept) + button_box.rejected.connect(self.reject) + layout.addWidget(button_box) + + def clear_selection(self): + for checkbox in self.checkboxes.values(): + checkbox.setChecked(False) + + def get_selected_rarities(self) -> list[ItemRarity]: + return [rarity for rarity, checkbox in self.checkboxes.items() if checkbox.isChecked()] diff --git a/src/gui/profile_editor/affixes_tab.py b/src/gui/profile_editor/affixes_tab.py index 4f6bdcfd..2408de15 100644 --- a/src/gui/profile_editor/affixes_tab.py +++ b/src/gui/profile_editor/affixes_tab.py @@ -45,6 +45,8 @@ MinGreaterDialog, MinPercentDialog, MinPowerDialog, + RarityPicker, + rarity_summary, ) from src.item.data.item_type import ItemType, is_armor, is_jewelry, is_weapon @@ -165,6 +167,21 @@ def setup_ui(self): item_type_layout.addStretch() general_form.addRow("Item Types:", item_type_layout) + self.rarity_line_edit = QLineEdit() + self.rarity_line_edit.setReadOnly(True) + self.rarity_line_edit.setMinimumWidth(360) + self.rarity_line_edit.setSizePolicy(QSizePolicy.Policy.Expanding, QSizePolicy.Policy.Fixed) + self.refresh_rarity_summary() + + rarity_layout = QHBoxLayout() + rarity_layout.addWidget(self.rarity_line_edit) + edit_rarities_btn = QPushButton("...") + edit_rarities_btn.setMaximumWidth(40) + edit_rarities_btn.clicked.connect(self.edit_rarities) + rarity_layout.addWidget(edit_rarities_btn) + rarity_layout.addStretch() + general_form.addRow("Rarities:", rarity_layout) + self.min_power = IgnoreScrollWheelSpinBox() self.min_power.setMaximum(MAX_POWER) self.min_power.setValue(self.config.min_power) @@ -434,6 +451,15 @@ def edit_item_types(self): self.config.item_type = item_type_picker.get_selected_item_types() self.refresh_item_type_summary() + def refresh_rarity_summary(self): + self.rarity_line_edit.setText(rarity_summary(self.config.rarities)) + + def edit_rarities(self): + rarity_picker = RarityPicker(self, self.config.rarities) + if rarity_picker.exec() == QDialog.DialogCode.Accepted: + self.config.rarities = rarity_picker.get_selected_rarities() + self.refresh_rarity_summary() + def update_min_power(self): self.config.min_power = self.min_power.value() diff --git a/src/gui/profile_editor/sigils_tab.py b/src/gui/profile_editor/sigils_tab.py index 7c09a456..b4852a52 100644 --- a/src/gui/profile_editor/sigils_tab.py +++ b/src/gui/profile_editor/sigils_tab.py @@ -6,6 +6,7 @@ QFormLayout, QHBoxLayout, QLabel, + QLineEdit, QListWidget, QListWidgetItem, QMessageBox, @@ -17,7 +18,15 @@ from src.config.profile_models import SigilConditionModel, SigilFilterModel, SigilPriority from src.dataloader import Dataloader from src.gui.models.collapsible_widget import Container -from src.gui.models.dialog import CreateSigil, IgnoreScrollWheelComboBox, RemoveSigil +from src.gui.models.dialog import ( + CreateSigil, + IgnoreScrollWheelComboBox, + RarityPicker, + RemoveSigil, + derive_sigil_kind, + rarity_summary, + sigil_name_dict_for_kind, +) SIGILS_TABNAME = "Sigils" @@ -55,11 +64,12 @@ def update_condition(self): class SigilWidget(Container): dungeon_changed = pyqtSignal() - def __init__(self, sigil_name: str, sigil: SigilConditionModel, whitelist: bool): + def __init__(self, sigil_name: str, sigil: SigilConditionModel, whitelist: bool, kind: str = "dungeon"): super().__init__(sigil_name, color_background=True) self.sigil = sigil self.sigil_name = sigil_name self.whitelist = whitelist + self.kind = kind self.setup_ui() def setup_ui(self): @@ -75,12 +85,13 @@ def setup_ui(self): self.sigil_name_combo.setEditable(True) self.sigil_name_combo.setInsertPolicy(QComboBox.InsertPolicy.NoInsert) self.sigil_name_combo.completer().setCompletionMode(QCompleter.CompletionMode.PopupCompletion) - self.sigil_name_combo.addItems(sorted(Dataloader().affix_sigil_dict_all["dungeons"].values())) + self.sigil_name_combo.addItems(sorted(sigil_name_dict_for_kind(self.kind).values())) self.sigil_name_combo.setCurrentText(self.sigil_name) self.sigil_name_combo.setMaximumWidth(150) self.sigil_name_combo.currentIndexChanged.connect(self.update_sigil_dungeon) - form_layout.addRow("Dungeon:", self.sigil_name_combo) + form_layout.addRow("Affix:" if self.kind == "affix" else "Dungeon:", self.sigil_name_combo) + layout.addLayout(form_layout) comparison_label = QLabel("Condition") title_layout.addSpacing(100) title_layout.addWidget(comparison_label) @@ -99,7 +110,7 @@ def setup_ui(self): remove_condition_btn = QPushButton("Remove Condition") remove_condition_btn.clicked.connect(self.remove_selected) condition_btn_layout.addWidget(remove_condition_btn) - layout.addLayout(form_layout) + layout.addLayout(condition_btn_layout) layout.addLayout(title_layout) layout.addWidget(self.condition_list) @@ -136,7 +147,7 @@ def update_sigil_dungeon(self, classic=True): self.old_name = self.sigil_name self.sigil_name = new_name self.header.set_name(new_name) - reverse_dict = {v: k for k, v in Dataloader().affix_sigil_dict_all["dungeons"].items()} + reverse_dict = {v: k for k, v in sigil_name_dict_for_kind(self.kind).items()} self.sigil.name = reverse_dict.get(new_name) if classic: self.dungeon_changed.emit() @@ -196,6 +207,19 @@ def create_form(self): self.priority_combobox.setMaximumWidth(150) self.priority_combobox.currentIndexChanged.connect(self.update_priority) self.general_form.addRow("Priority:", self.priority_combobox) + + self.rarity_line_edit = QLineEdit() + self.rarity_line_edit.setReadOnly(True) + self.refresh_rarity_summary() + rarity_layout = QHBoxLayout() + rarity_layout.addWidget(self.rarity_line_edit) + edit_rarities_btn = QPushButton("...") + edit_rarities_btn.setMaximumWidth(40) + edit_rarities_btn.clicked.connect(self.edit_rarities) + rarity_layout.addWidget(edit_rarities_btn) + rarity_layout.addStretch() + self.general_form.addRow("Rarities:", rarity_layout) + self.main_layout.addLayout(self.general_form) def create_containers(self): @@ -221,30 +245,31 @@ def create_containers(self): self.main_layout.addWidget(self.blacklist_container) def add_sigil(self, sigil_condition: SigilConditionModel, whitelist: bool = False): - name = Dataloader().affix_sigil_dict_all["dungeons"][sigil_condition.name] + kind = derive_sigil_kind(sigil_condition.name) + name = sigil_name_dict_for_kind(kind)[sigil_condition.name] if whitelist: - widget = SigilWidget(name, sigil_condition, whitelist=True) + widget = SigilWidget(name, sigil_condition, whitelist=True, kind=kind) widget.dungeon_changed.connect(lambda: self.on_dungeon_changed(widget)) self.whitelist_layout.addWidget(widget) else: - widget = SigilWidget(name, sigil_condition, whitelist=False) + widget = SigilWidget(name, sigil_condition, whitelist=False, kind=kind) widget.dungeon_changed.connect(lambda: self.on_dungeon_changed(widget)) self.blacklist_layout.addWidget(widget) def create_sigil(self): dialog = CreateSigil(self.whitelist_sigils, self.blacklist_sigils) if dialog.exec() == QDialog.DialogCode.Accepted: - sigil_name, type_name = dialog.get_value() - reverse_dict = {v: k for k, v in Dataloader().affix_sigil_dict_all["dungeons"].items()} + sigil_name, type_name, kind = dialog.get_value() + reverse_dict = {v: k for k, v in sigil_name_dict_for_kind(kind).items()} sigil_condition = SigilConditionModel(name=reverse_dict.get(sigil_name), condition=[]) if type_name == "whitelist": - widget = SigilWidget(sigil_name, sigil_condition, whitelist=True) + widget = SigilWidget(sigil_name, sigil_condition, whitelist=True, kind=kind) widget.dungeon_changed.connect(lambda: self.on_dungeon_changed(widget)) self.whitelist_layout.addWidget(widget) self.whitelist_sigils.append(sigil_name) self.sigil_model.whitelist.append(sigil_condition) elif type_name == "blacklist": - widget = SigilWidget(sigil_name, sigil_condition, whitelist=False) + widget = SigilWidget(sigil_name, sigil_condition, whitelist=False, kind=kind) widget.dungeon_changed.connect(lambda: self.on_dungeon_changed(widget)) self.blacklist_layout.addWidget(widget) self.blacklist_sigils.append(sigil_name) @@ -280,6 +305,15 @@ def remove_sigil(self, blacklist: bool = False): def update_priority(self): self.sigil_model.priority = SigilPriority(self.priority_combobox.currentText()) + def refresh_rarity_summary(self): + self.rarity_line_edit.setText(rarity_summary(self.sigil_model.rarities)) + + def edit_rarities(self): + picker = RarityPicker(self, self.sigil_model.rarities) + if picker.exec() == QDialog.DialogCode.Accepted: + self.sigil_model.rarities = picker.get_selected_rarities() + self.refresh_rarity_summary() + def on_dungeon_changed(self, sigil_widget: SigilWidget): whitelist = sigil_widget.whitelist new_name = sigil_widget.sigil_name diff --git a/src/item/descr/read_descr_tts.py b/src/item/descr/read_descr_tts.py index 2836b5f6..284023e2 100644 --- a/src/item/descr/read_descr_tts.py +++ b/src/item/descr/read_descr_tts.py @@ -206,6 +206,13 @@ def _add_sigil_affixes_from_tts(tts_section: list[str], item: Item) -> Item: affix.type = AffixType.normal item.affixes.append(affix) + rarity_map = Dataloader().affix_sigil_dict_all["rarities"] + item.rarity = next( + (ItemRarity(rarity_map[affix.name].lower()) for affix in item.affixes if affix.name in rarity_map), None + ) + if item.rarity is None: + LOGGER.warning(f"Could not resolve sigil rarity from affixes: {[affix.name for affix in item.affixes]}") + return item @@ -218,9 +225,9 @@ def _create_base_item_from_tts(tts_item: list[str]) -> Item | None: return None if "bloodied" in tts_item[1].lower(): item.seasonal_attribute = SeasonalAttribute.bloodied - return _update_item_object(item, rarity=ItemRarity.Common, item_type=ItemType.Sigil) + return _update_item_object(item, item_type=ItemType.Sigil) if tts_item[0].startswith(ItemIdentifiers.ESCALATION_SIGIL.value): - return _update_item_object(item, rarity=ItemRarity.Common, item_type=ItemType.EscalationSigil) + return _update_item_object(item, item_type=ItemType.EscalationSigil) if ItemIdentifiers.TRIBUTE.value in tts_item[0]: item.item_type = ItemType.Tribute search_string_split = tts_item[1].split(" ") diff --git a/src/item/filter.py b/src/item/filter.py index 367c5c70..715a94cb 100644 --- a/src/item/filter.py +++ b/src/item/filter.py @@ -25,6 +25,7 @@ TributeFilterModel, ) from src.config.settings_models import AspectFilterType, CosmeticFilterType, UnfilteredUniquesType +from src.dataloader import Dataloader from src.item.data.affix import Affix, AffixType from src.item.data.item_type import ItemType, is_sigil from src.item.data.rarity import ItemRarity @@ -96,6 +97,9 @@ def _check_affixes(self, item: Item) -> FilterResult: # check item type if not self._match_item_type(expected_item_types=filter_spec.item_type, item_type=item.item_type): continue + # check item rarity + if filter_spec.rarities and item.rarity not in filter_spec.rarities: + continue # check item power if not self._match_item_power(min_power=filter_spec.min_power, item_power=item.power): continue @@ -196,6 +200,17 @@ def _check_cosmetic(item: Item) -> FilterResult: res.matched.append(MatchedFilter("Cosmetics")) return res + def _get_sigil_rarity(self, item: Item) -> ItemRarity | None: + if item.rarity is not None: + return item.rarity + rarity_map = Dataloader().affix_sigil_dict_all["rarities"] + sigil_affixes = item.affixes + item.inherent + for affix in sigil_affixes: + if affix.name in rarity_map: + return ItemRarity(rarity_map[affix.name].lower()) + LOGGER.warning(f"Could not resolve sigil rarity from affixes: {[a.name for a in sigil_affixes]}") + return None + def _check_sigil(self, item: Item) -> FilterResult: res = FilterResult(keep=False, matched=[]) if not self.sigil_filters.items(): @@ -203,6 +218,10 @@ def _check_sigil(self, item: Item) -> FilterResult: res.keep = True res.matched.append(MatchedFilter("Sigils not filtered")) for profile_name, profile_filter in self.sigil_filters.items(): + if profile_filter.rarities: + sigil_rarity = self._get_sigil_rarity(item) + if sigil_rarity is None or sigil_rarity not in profile_filter.rarities: + continue # fail-closed; warning logged in _get_sigil_rarity blacklist_empty = not profile_filter.blacklist is_in_blacklist = self._match_affixes_sigils( expected_affixes=profile_filter.blacklist, @@ -540,7 +559,7 @@ def load_files(self): if data.aspect_upgrades: self.aspect_upgrade_filters[data.name] = data.aspect_upgrades sections.append(ASPECT_UPGRADES_LABEL) - if data.sigils and (data.sigils.blacklist or data.sigils.whitelist): + if data.sigils and (data.sigils.blacklist or data.sigils.whitelist or data.sigils.rarities): self.sigil_filters[data.name] = data.sigils sections.append("Sigils") if data.tributes: diff --git a/tests/config/models_test.py b/tests/config/models_test.py index 7c0602bd..732e4c13 100644 --- a/tests/config/models_test.py +++ b/tests/config/models_test.py @@ -581,6 +581,79 @@ def test_rarities_parse_list(self) -> None: assert ItemRarity.Legendary in model.rarities +class TestTributeFilterModelRarity: + """Test rarity key unification on TributeFilterModel (Slice 01).""" + + def test_rarity_key_loads(self) -> None: + """New 'rarity' key (singular) is accepted.""" + model = TributeFilterModel.model_validate({"rarity": ["legendary"]}) + assert model.rarities == [ItemRarity.Legendary] + + def test_legacy_rarities_key_loads(self) -> None: + """Legacy 'rarities' key still works for back-compat.""" + model = TributeFilterModel.model_validate({"rarities": ["legendary"]}) + assert model.rarities == [ItemRarity.Legendary] + + def test_serialization_alias_is_rarity(self) -> None: + """model_dump(by_alias=True) uses 'rarity' as the key, not 'rarities'.""" + model = TributeFilterModel.model_validate({"rarity": ["legendary"]}) + dumped = model.model_dump(by_alias=True) + assert "rarity" in dumped + assert "rarities" not in dumped + + def test_serialization_alias_json(self) -> None: + """model_dump_json(by_alias=True) uses 'rarity' as the key.""" + model = TributeFilterModel.model_validate({"rarity": ["legendary"]}) + exported = json.loads(model.model_dump_json(by_alias=True)) + assert "rarity" in exported + assert "rarities" not in exported + + def test_bare_string_lowercase(self) -> None: + """Bare lowercase rarity string normalizes to list of ItemRarity.""" + model = TributeFilterModel.model_validate("legendary") + assert model.rarities == [ItemRarity.Legendary] + + def test_bare_string_uppercase(self) -> None: + """Bare mixed-case rarity string is case-insensitively parsed.""" + model = TributeFilterModel.model_validate("Legendary") + assert model.rarities == [ItemRarity.Legendary] + + def test_rarity_list_normalizes(self) -> None: + """List of rarity strings (mixed case) all normalize correctly.""" + model = TributeFilterModel.model_validate({"rarity": ["Rare", "LEGENDARY", "unique"]}) + assert ItemRarity.Rare in model.rarities + assert ItemRarity.Legendary in model.rarities + assert ItemRarity.Unique in model.rarities + + def test_case_insensitive_rare(self) -> None: + """'Rare' and 'rare' both parse to ItemRarity.Rare.""" + m1 = TributeFilterModel.model_validate({"rarity": ["Rare"]}) + m2 = TributeFilterModel.model_validate({"rarity": ["rare"]}) + assert m1.rarities == [ItemRarity.Rare] + assert m2.rarities == [ItemRarity.Rare] + + def test_invalid_rarity_raises(self) -> None: + """An unrecognised rarity value raises ValidationError.""" + with pytest.raises(ValidationError): + TributeFilterModel.model_validate({"rarity": ["notararity"]}) + + def test_empty_rarity_field_defaults_to_empty_list(self) -> None: + """Absent rarity field defaults to empty list.""" + model = TributeFilterModel.model_validate({"name": "harmony"}) + assert model.rarities == [] + + def test_keyword_construction_with_rarities(self) -> None: + """TributeFilterModel(rarities=[...]) still works (populate_by_name).""" + model = TributeFilterModel(rarities=[ItemRarity.Legendary]) + assert model.rarities == [ItemRarity.Legendary] + + def test_parse_data_returns_rarities_key_still_works(self) -> None: + """parse_data returns {'rarities': [...]}, which flows through _normalize_rarities.""" + model = TributeFilterModel.model_validate(["Legendary", "Rare"]) + assert ItemRarity.Legendary in model.rarities + assert ItemRarity.Rare in model.rarities + + class TestSigilConditionModel: """Test SigilConditionModel.""" @@ -946,3 +1019,52 @@ def test_serialize_paragon_excludes_null_metadata(self) -> None: assert "Source" not in paragon assert "GeneratedAt" not in paragon assert "Generator" not in paragon + + +class TestItemFilterModelRarity: + """Test rarity field on ItemFilterModel (Slice 03).""" + + def test_rarity_parses_from_single_string(self) -> None: + """Single string under 'rarity' key is accepted.""" + model = ItemFilterModel.model_validate({"rarity": "rare"}) + assert model.rarities == [ItemRarity.Rare] + + def test_rarity_parses_from_list(self) -> None: + """List of strings under 'rarity' key is accepted.""" + model = ItemFilterModel.model_validate({"rarity": ["rare", "legendary"]}) + assert ItemRarity.Rare in model.rarities + assert ItemRarity.Legendary in model.rarities + + def test_rarity_is_case_insensitive(self) -> None: + """Mixed-case rarity values are normalised.""" + model = ItemFilterModel.model_validate({"rarity": ["Rare", "LEGENDARY", "unique"]}) + assert ItemRarity.Rare in model.rarities + assert ItemRarity.Legendary in model.rarities + assert ItemRarity.Unique in model.rarities + + def test_invalid_rarity_raises(self) -> None: + """An unrecognised rarity value raises ValidationError.""" + with pytest.raises(ValidationError): + ItemFilterModel.model_validate({"rarity": ["notararity"]}) + + def test_absent_rarity_defaults_to_empty_list(self) -> None: + """Absent 'rarity' field defaults to empty list.""" + model = ItemFilterModel() + assert model.rarities == [] + + def test_empty_rarity_list_defaults_to_empty(self) -> None: + """Explicit empty list stays empty.""" + model = ItemFilterModel.model_validate({"rarity": []}) + assert model.rarities == [] + + def test_keyword_construction_with_rarities(self) -> None: + """ItemFilterModel(rarities=[...]) works via populate_by_name.""" + model = ItemFilterModel(rarities=[ItemRarity.Rare]) + assert model.rarities == [ItemRarity.Rare] + + def test_serialization_alias_is_rarity(self) -> None: + """model_dump(by_alias=True) emits 'rarity', not 'rarities'.""" + model = ItemFilterModel(rarities=[ItemRarity.Legendary]) + dumped = model.model_dump(by_alias=True) + assert "rarity" in dumped + assert "rarities" not in dumped diff --git a/tests/conftest.py b/tests/conftest.py index 3d258d05..2c0f85bd 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -23,6 +23,7 @@ "char_inventory_test.py", "chest_test.py", "paragon_overlay_test.py", + "test_sigils_tab.py", ] diff --git a/tests/gui/profile_editor/__init__.py b/tests/gui/profile_editor/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/gui/profile_editor/test_sigils_tab.py b/tests/gui/profile_editor/test_sigils_tab.py new file mode 100644 index 00000000..f299fb4d --- /dev/null +++ b/tests/gui/profile_editor/test_sigils_tab.py @@ -0,0 +1,58 @@ +import os + +import pytest + +os.environ.setdefault("QT_QPA_PLATFORM", "offscreen") + +from PyQt6.QtWidgets import QApplication + +from src.config.profile_models import SigilConditionModel, SigilFilterModel +from src.dataloader import Dataloader +from src.gui.profile_editor.sigils_tab import SigilsTab, SigilWidget + + +@pytest.fixture(scope="module") +def qapp(): + return QApplication.instance() or QApplication([]) + + +def _first_affix_key() -> str: + data = Dataloader().affix_sigil_dict_all + return next(iter({**data["minor"], **data["major"], **data["positive"]})) + + +def _first_dungeon_key() -> str: + return next(iter(Dataloader().affix_sigil_dict_all["dungeons"])) + + +def _loaded_tab(name: str) -> SigilsTab: + model = SigilFilterModel(blacklist=[SigilConditionModel(name=name, condition=[])]) + tab = SigilsTab(model) + tab.load() # regression: used to raise KeyError for a top-level affix name + return tab + + +def test_global_affix_blacklist_loads_as_affix_kind(qapp, mock_ini_loader): + tab = _loaded_tab(_first_affix_key()) + widget = tab.blacklist_layout.itemAt(0).widget() + assert isinstance(widget, SigilWidget) + assert widget.kind == "affix" + + +def test_dungeon_blacklist_loads_as_dungeon_kind(qapp, mock_ini_loader): + tab = _loaded_tab(_first_dungeon_key()) + widget = tab.blacklist_layout.itemAt(0).widget() + assert isinstance(widget, SigilWidget) + assert widget.kind == "dungeon" + + +def test_affix_kind_has_condition_list(qapp, mock_ini_loader): + tab = _loaded_tab(_first_affix_key()) + widget = tab.blacklist_layout.itemAt(0).widget() + assert hasattr(widget, "condition_list") + + +def test_dungeon_kind_has_condition_list(qapp, mock_ini_loader): + tab = _loaded_tab(_first_dungeon_key()) + widget = tab.blacklist_layout.itemAt(0).widget() + assert hasattr(widget, "condition_list") diff --git a/tests/item/filter/data/filters.py b/tests/item/filter/data/filters.py index 7b801daa..677de40f 100644 --- a/tests/item/filter/data/filters.py +++ b/tests/item/filter/data/filters.py @@ -240,6 +240,23 @@ ), ) +# Rarity gate: only Rare sigils pass +sigil_rarity_rare_only = ProfileModel(name="rarity_rare_only", sigils=SigilFilterModel(rarities=[ItemRarity.Rare])) + +# Rarity gate + blacklist: Rare sigils pass UNLESS blacklisted +sigil_rarity_rare_with_blacklist = ProfileModel( + name="rarity_with_blacklist", + sigils=SigilFilterModel( + rarities=[ItemRarity.Rare], blacklist=[SigilConditionModel(name="reduce_cooldowns_on_kill")] + ), +) + +# Rarity gate + whitelist: a whitelisted sigil is still dropped when its rarity fails the gate +sigil_rarity_rare_with_whitelist = ProfileModel( + name="rarity_with_whitelist", + sigils=SigilFilterModel(rarities=[ItemRarity.Rare], whitelist=[SigilConditionModel(name="jalals_vigil")]), +) + # noinspection PyTypeChecker unique_affixes = ProfileModel( name="test", @@ -323,3 +340,33 @@ TributeFilterModel(rarities=[ItemRarity.Legendary, ItemRarity.Unique]), ], ) + +# noinspection PyTypeChecker +affix_rarity = ProfileModel( + name="rarity_test", + affixes=[ + { + "RareOnly": ItemFilterModel( + item_type=[ItemType.Boots], + rarities=[ItemRarity.Rare], + affix_pool=[ + AffixFilterCountModel( + count=[AffixFilterModel(name="movement_speed"), AffixFilterModel(name="cold_resistance")], + min_count=1, + ) + ], + ) + }, + { + "AnyRarity": ItemFilterModel( + item_type=[ItemType.Boots], + affix_pool=[ + AffixFilterCountModel( + count=[AffixFilterModel(name="movement_speed"), AffixFilterModel(name="cold_resistance")], + min_count=1, + ) + ], + ) + }, + ], +) diff --git a/tests/item/filter/data/sigils.py b/tests/item/filter/data/sigils.py index 199839e3..d2147275 100644 --- a/tests/item/filter/data/sigils.py +++ b/tests/item/filter/data/sigils.py @@ -82,3 +82,30 @@ def __init__(self, rarity=ItemRarity.Common, item_type=ItemType.Sigil, **kwargs) affixes=[Affix(name="shadow_damage", value=2.0), Affix(name="reduce_cooldowns_on_kill")], inherent=[Affix(name="iron_hold")], ) + +# Rarity-derivation fixtures +# "amethyst_reserve" maps to "Rare" in sigils.json rarities map +sigil_derived_rare = TestSigil( + rarity=ItemRarity.Rare, + affixes=[Affix(name="amethyst_reserve"), Affix(name="shadow_damage", value=2.0)], + inherent=[Affix(name="jalals_vigil")], +) + +# "astral_prophecy" maps to "Legendary" in sigils.json rarities map +sigil_derived_legendary = TestSigil( + rarity=ItemRarity.Legendary, + affixes=[Affix(name="astral_prophecy"), Affix(name="shadow_damage", value=2.0)], + inherent=[Affix(name="jalals_vigil")], +) + +# Unknown rarity: no affix in the rarities map +sigil_unknown_rarity = TestSigil( + rarity=None, affixes=[Affix(name="shadow_damage", value=2.0)], inherent=[Affix(name="jalals_vigil")] +) + +# Rare sigil that is also on the blacklist (used for AND-gate test) +sigil_rare_blacklisted = TestSigil( + rarity=ItemRarity.Rare, + affixes=[Affix(name="amethyst_reserve"), Affix(name="reduce_cooldowns_on_kill")], + inherent=[Affix(name="jalals_vigil")], +) diff --git a/tests/item/filter/filter_test.py b/tests/item/filter/filter_test.py index 6a0bb514..2d6cf3e5 100644 --- a/tests/item/filter/filter_test.py +++ b/tests/item/filter/filter_test.py @@ -1,5 +1,6 @@ from __future__ import annotations +import logging import sys import typing @@ -13,19 +14,29 @@ from src.config.profile_models import ParagonPayloadModel, ProfileModel, SigilPriority from src.config.settings_models import AspectFilterType from src.gui.importer.gui_common import save_as_profile +from src.item.data.affix import Affix +from src.item.data.item_type import ItemType +from src.item.data.rarity import ItemRarity from src.item.filter import Filter, FilterResult +from src.item.models import Item from tests.item.filter.data import filters from tests.item.filter.data.affixes import affixes from tests.item.filter.data.aspects import aspects -from tests.item.filter.data.sigils import sigil_jalal, sigil_priority, sigils +from tests.item.filter.data.sigils import ( + sigil_derived_legendary, + sigil_derived_rare, + sigil_jalal, + sigil_priority, + sigil_rare_blacklisted, + sigil_unknown_rarity, + sigils, +) from tests.item.filter.data.tributes import tributes from tests.item.filter.data.uniques import global_uniques, simple_mythics, uniques_with_affixes if typing.TYPE_CHECKING: from pytest_mock import MockerFixture - from src.item.models import Item - def _create_mocked_filter(mocker: MockerFixture) -> Filter: filter_obj = Filter() @@ -128,6 +139,107 @@ def test_mythic_always_kept(_name: str, result: bool, item: Item, mocker: Mocker assert test_filter.should_keep(item).keep == result +@pytest.mark.parametrize( + ("rarity", "expected"), + [ + # Rare matches the rarity-gated RareOnly rule AND the ungated AnyRarity rule. + (ItemRarity.Rare, {"rarity_test.RareOnly", "rarity_test.AnyRarity"}), + # Legendary fails the RareOnly gate but the empty-rarity AnyRarity rule still matches (regression). + (ItemRarity.Legendary, {"rarity_test.AnyRarity"}), + ], +) +def test_affix_rarity_gate(rarity: ItemRarity, expected: set[str], mocker: MockerFixture): + boots = Item( + item_type=ItemType.Boots, + power=900, + rarity=rarity, + affixes=[Affix(name="movement_speed", value=10)], + inherent=[], + ) + test_filter = _create_mocked_filter(mocker) + test_filter.affix_filters = {filters.affix_rarity.name: filters.affix_rarity.affixes} + assert {m.profile for m in test_filter.should_keep(boots).matched} == expected + + +def test_sigil_rarity_gate_keeps_matching_rarity(mocker: MockerFixture): + """A sigil whose derived rarity matches the gate passes through.""" + test_filter = _create_mocked_filter(mocker) + test_filter.sigil_filters = {filters.sigil_rarity_rare_only.name: filters.sigil_rarity_rare_only.sigils} + assert test_filter.should_keep(sigil_derived_rare).matched[0].profile == filters.sigil_rarity_rare_only.name + + +def test_sigil_rarity_gate_drops_non_matching_rarity(mocker: MockerFixture): + """A sigil whose derived rarity does NOT match the gate is dropped.""" + test_filter = _create_mocked_filter(mocker) + test_filter.sigil_filters = {filters.sigil_rarity_rare_only.name: filters.sigil_rarity_rare_only.sigils} + assert test_filter.should_keep(sigil_derived_legendary).matched == [] + + +def test_sigil_rarity_gate_drops_unknown_rarity(mocker: MockerFixture, caplog): + """A sigil whose rarity cannot be derived is dropped when the gate is active (fail-closed).""" + with caplog.at_level(logging.DEBUG, logger="src.item.filter"): + test_filter = _create_mocked_filter(mocker) + test_filter.sigil_filters = {filters.sigil_rarity_rare_only.name: filters.sigil_rarity_rare_only.sigils} + result = test_filter.should_keep(sigil_unknown_rarity) + assert result.matched == [] + assert any("Could not resolve sigil rarity" in r.message for r in caplog.records) + + +def test_sigil_rarity_gate_empty_passes_all(mocker: MockerFixture): + """When rarities list is empty, all sigils pass (regression — preserves current behavior).""" + test_filter = _create_mocked_filter(mocker) + # Use a filter that only has blacklist, no rarity gate + test_filter.sigil_filters = {filters.sigil_blacklist_only.name: filters.sigil_blacklist_only.sigils} + # sigil_derived_rare is not blacklisted — should pass + assert test_filter.should_keep(sigil_derived_rare).matched[0].profile == filters.sigil_blacklist_only.name + + +def test_sigil_rarity_and_blacklist_drops_blacklisted(mocker: MockerFixture): + """A sigil that passes the rarity gate but is blacklisted is still dropped.""" + test_filter = _create_mocked_filter(mocker) + test_filter.sigil_filters = { + filters.sigil_rarity_rare_with_blacklist.name: filters.sigil_rarity_rare_with_blacklist.sigils + } + # sigil_rare_blacklisted is Rare (passes rarity gate) but has reduce_cooldowns_on_kill (blacklisted) + assert test_filter.should_keep(sigil_rare_blacklisted).matched == [] + + +def test_sigil_rarity_and_whitelist_drops_whitelisted_wrong_rarity(mocker: MockerFixture): + """A whitelisted sigil whose rarity fails the gate is still dropped (rarity ANDs with whitelist).""" + test_filter = _create_mocked_filter(mocker) + test_filter.sigil_filters = { + filters.sigil_rarity_rare_with_whitelist.name: filters.sigil_rarity_rare_with_whitelist.sigils + } + # sigil_derived_legendary has jalals_vigil (whitelisted) but is Legendary — rarity gate drops it + assert test_filter.should_keep(sigil_derived_legendary).matched == [] + # sigil_derived_rare has jalals_vigil (whitelisted) and is Rare — passes both + assert ( + test_filter.should_keep(sigil_derived_rare).matched[0].profile == filters.sigil_rarity_rare_with_whitelist.name + ) + + +def test_sigil_rarity_and_blacklist_keeps_rare_not_blacklisted(mocker: MockerFixture): + """A sigil that passes the rarity gate and is not blacklisted is kept.""" + test_filter = _create_mocked_filter(mocker) + test_filter.sigil_filters = { + filters.sigil_rarity_rare_with_blacklist.name: filters.sigil_rarity_rare_with_blacklist.sigils + } + # sigil_derived_rare is Rare and not blacklisted — should pass + assert ( + test_filter.should_keep(sigil_derived_rare).matched[0].profile == filters.sigil_rarity_rare_with_blacklist.name + ) + + +def test_sigil_rarity_gate_drops_legendary_regardless_of_blacklist_state(mocker: MockerFixture): + """A sigil failing the rarity gate is dropped even when no blacklist entry matches it.""" + test_filter = _create_mocked_filter(mocker) + test_filter.sigil_filters = { + filters.sigil_rarity_rare_with_blacklist.name: filters.sigil_rarity_rare_with_blacklist.sigils + } + # sigil_derived_legendary is Legendary — fails rarity gate, dropped regardless of blacklist + assert test_filter.should_keep(sigil_derived_legendary).matched == [] + + def test_filter_loads_typed_paragon_payload(tmp_path, mock_ini_loader: IniConfigLoader, mocker: MockerFixture) -> None: mock_ini_loader._user_dir = tmp_path mock_ini_loader.general.profiles = ["typed_paragon"] diff --git a/tests/item/read_descr_season_11_tts_test.py b/tests/item/read_descr_season_11_tts_test.py index 583178c5..607a8ca2 100644 --- a/tests/item/read_descr_season_11_tts_test.py +++ b/tests/item/read_descr_season_11_tts_test.py @@ -351,7 +351,7 @@ item_type=ItemType.Sigil, name="mercys_reach", power=None, - rarity=ItemRarity.Common, + rarity=ItemRarity.Legendary, ), ), # Whether or not all resist is considered an inherent changes diff --git a/tests/item/read_descr_season_12_tts_test.py b/tests/item/read_descr_season_12_tts_test.py index cde00c1f..1c52d84c 100644 --- a/tests/item/read_descr_season_12_tts_test.py +++ b/tests/item/read_descr_season_12_tts_test.py @@ -186,7 +186,7 @@ name="uldurs_cave", original_name="ULDURS CAVE NIGHTMARE SIGIL", power=None, - rarity=ItemRarity.Common, + rarity=ItemRarity.Rare, seasonal_attribute=SeasonalAttribute.bloodied, ), ),