Fix Trade Query influence filter: expose Ignore/Any and document pair semantics - #9803
Closed
mcagnion wants to merge 5 commits into
Closed
Fix Trade Query influence filter: expose Ignore/Any and document pair semantics#9803mcagnion wants to merge 5 commits into
mcagnion wants to merge 5 commits into
Conversation
mcagnion
marked this pull request as draft
April 20, 2026 23:56
Contributor
|
I think the required filter dropdowns might've solved this. I think it might make more sense to remove these dropdowns completely since the required filters work for it |
Port of bugfix/trade-query-influence-none onto current origin/dev. Adapted to upstream PR 9691 changes: combined normalizeInfluenceSelections with copyEldritch interaction, Watcher's Eye guard, SetSel pattern, and ^7 label prefixes. Updated the eldritch weight skip in ExecuteQuery for every influence-positive selection. 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>
mcagnion
force-pushed
the
bugfix/trade-query-influence-none
branch
from
August 15, 2026 15:56
8bd2c7d to
f79efda
Compare
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>
mcagnion
force-pushed
the
bugfix/trade-query-influence-none
branch
from
August 15, 2026 16:01
f79efda to
70a0cd9
Compare
Contributor
Author
|
Good point. I checked the current Required Stat implementation. The Trade query schema supports Removing the dedicated dropdowns would be a substantial simplification, but it would drop those two cases. Would you prefer removing them and accepting that limitation, or extending Required Stats with generic max/NOT support instead? |
Contributor
|
I think a not filter should be added. It's been requested quite a few times, and it's probably useful for the unique item searches which sometimes have unrealistic mods. It shouldn't be too difficult, I think. I can take a look at it some time soon |
Contributor
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
2 participants
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:
Any / Ignoredoes not generate Eldritch weights whileIgnore / Nonestill permits them.Before
After