Fix Trade Query influence filter: expose Ignore/Any and document pair semantics#9803
Draft
mcagnion wants to merge 5 commits intoPathOfBuildingCommunity:devfrom
Draft
Fix Trade Query influence filter: expose Ignore/Any and document pair semantics#9803mcagnion wants to merge 5 commits intoPathOfBuildingCommunity:devfrom
mcagnion wants to merge 5 commits intoPathOfBuildingCommunity:devfrom
Conversation
Port of bugfix/trade-query-influence-none onto current origin/dev. Adapted to upstream PathOfBuildingCommunity#9691 changes: combined normalizeInfluenceSelections with copyEldritch interaction, Watcher's Eye guard, SetSel pattern, ^7 label prefixes. Updated isSpecificInfluenceSelection check for eldritch weight skip in ExecuteQuery. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The influence callback was over-normalizing Ignore+None and None+specific pairs, forcing them back to None/None and locking users out of states the query state resolver already supports (exactCount=1, exactCount=1 plus that specific). Users stuck at None/None could not return to an unfiltered pair. Drop the UI normalization. The callback keeps only the eldritch side-effect it was carrying. All pair combinations now flow through to resolveInfluenceQueryState as intended. Same specific on both sides (Shaper/Shaper) is redundant at the item level, so resolveInfluenceQueryState treats the second slot as None: the pair now generates the same query as Shaper/None (exactly 1 of that type, pseudo_has_influence capped at 1). Without the None constraint the duplicate specific would have silently produced a looser query than the paired form. Test coverage added asserts state equality and query-cost equality with the paired form. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add a shared tooltip on both Influence fields listing the nine distinct pair semantics (no filter, no influences, exactly 1, at least 1, exactly 2, plus the specific variants). Ordered by the number of required influences so users can scan to the constraint they want. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
"Cost" was the only occurrence of the term in this file: the rest of the filter-budget path uses num_extra as the running count of structural filter slots consumed before weighted mods fill the remainder. Align with that convention: getInfluenceFilterCost becomes countInfluenceFilters and the intermediate influenceFilterCost variable is dropped in favour of a direct assignment to num_extra. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The three helpers resolveInfluenceQueryState / needsHasInfluenceFilter / countInfluenceFilters were each exported to the test mock so the spec could assert on intermediate state. That surface tested the shape of the computation rather than its outcome. Introduce buildInfluenceFilters(selection1, selection2) that returns the exact query fragments ExecuteQuery needs — the and-group filters, the top-level NOT stats (only populated for "no influences"), and the total filter-slot count. ExecuteQuery now computes the influence filters and the slot budget in a single call and inlines the fragments, dropping ~20 lines of duplicated query-building logic. Tests are rewritten to exercise the helper directly and assert on the resulting filter table, giving genuine end-to-end coverage of the nine pair combinations. Only _buildInfluenceFilters, _hasAnyInfluenceModId and the INFLUENCE_*_INDEX constants remain as test accessors; the three internal-state helpers are no longer reachable from outside the module. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description of the problem being solved:
The Trade Query popup's two Influence dropdowns only exposed
Noneand the six named influences (Shaper, Elder, Warlord, Hunter, Crusader, Redeemer). There was no way to express "ignore this slot" (match anything) or "any influence" (require at least one). On top of that, the callback over-normalized several pair combinations (Ignore + None,None + specific,None + Any) back toNone / None, so users who picked a meaningful pair that the query-state resolver actually supports would silently lose it — and users stuck atNone / Nonehad no way to return to an unfiltered pair.This PR exposes
IgnoreandAnyas first-class options, drops the UI-side normalization so every pair the user selects flows through toresolveInfluenceQueryStateunchanged, and documents the nine distinct pair semantics on a shared tooltip grouped by the number of required influences. Same specific on both sides (e.g.,Shaper / Shaper) is redundant at the item level, so the query state resolver treats the second slot as None — the pair now generates the same query asShaper / None(exactly 1 of that type,pseudo_has_influence_countcapped at 1).Steps taken to verify a working solution:
scripts/test-fast.sh --brief).TestTradeQueryGenerator_spec.lua:12 successes / 0 failures / 0 errors, including coverage for theShaper + Shaper → Shaper + Nonequery-state equivalence (exactCount, hasNoneConstraint, cost andneedsHasInfluenceFilterall asserted equal to the paired form).Ignore / None,None / Any,None / <specific>, etc.) are now reachable and stable.Before
After