Skip to content

Commit 8bd2c7d

Browse files
mcagnionclaude
andcommitted
refactor(trade): expose a single buildInfluenceFilters helper
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>
1 parent 822262b commit 8bd2c7d

2 files changed

Lines changed: 102 additions & 85 deletions

File tree

spec/System/TestTradeQueryGenerator_spec.lua

Lines changed: 63 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -37,88 +37,89 @@ describe("TradeQueryGenerator", function()
3737
end)
3838
end)
3939

40-
describe("Influence query state", function()
40+
describe("Influence query fragments", function()
4141
local IGNORE = mock_queryGen._INFLUENCE_IGNORE_INDEX -- 1
4242
local NONE = mock_queryGen._INFLUENCE_NONE_INDEX -- 2
4343
local ANY = mock_queryGen._INFLUENCE_ANY_INDEX -- 3
4444
local SHAPER = ANY + 1 -- 4
4545
local ELDER = ANY + 2 -- 5
46-
local resolve = mock_queryGen._resolveInfluenceQueryState
47-
local count = mock_queryGen._countInfluenceFilters
48-
local needs = mock_queryGen._needsHasInfluenceFilter
46+
local build = mock_queryGen._buildInfluenceFilters
47+
local HAS_INFLUENCE = mock_queryGen._hasAnyInfluenceModId -- "pseudo.pseudo_has_influence_count"
48+
local HAS_SHAPER = "pseudo.pseudo_has_shaper_influence"
4949

50-
-- None: uses pseudo_has_influence=0 (1 slot instead of 6-slot NOT filter)
51-
it("None uses 1-slot pseudo_has_influence=0", function()
52-
local state = resolve(NONE, IGNORE)
53-
assert.are.equal(state.exactCount, 0)
54-
assert.is_true(state.hasNoneConstraint)
55-
assert.are.equal(count(state), 1)
56-
assert.is_true(needs(state))
50+
it("Ignore / Ignore produces no filters", function()
51+
local andGroup, topStats, slots = build(IGNORE, IGNORE)
52+
assert.are.equal(#andGroup, 0)
53+
assert.are.equal(#topStats, 0)
54+
assert.are.equal(slots, 0)
5755
end)
5856

59-
-- Shaper+None: needs pseudo_has_influence=1 to cap at 1 influence (avoids Shaper+Elder matches)
60-
it("Shaper+None uses 2-slot filter (specific + pseudo_has_influence=1)", function()
61-
local state = resolve(SHAPER, NONE)
62-
assert.are.equal(state.exactCount, 1)
63-
assert.is_true(state.hasNoneConstraint)
64-
assert.are.equal(#state.specificInfluenceModIds, 1)
65-
assert.are.equal(count(state), 2)
66-
assert.is_true(needs(state))
57+
it("None / None emits a NOT clause at the top level (no influences)", function()
58+
local andGroup, topStats, slots = build(NONE, NONE)
59+
assert.are.equal(#andGroup, 0)
60+
assert.are.equal(#topStats, 1)
61+
assert.are.same(topStats[1], { type = "not", filters = { { id = HAS_INFLUENCE } } })
62+
assert.are.equal(slots, 1)
6763
end)
6864

69-
-- Shaper+Elder: 2 named influences, no None → no pseudo_has_influence needed (saves 1 slot)
70-
it("Shaper+Elder uses 2-slot filter (specific mods only, no pseudo_has_influence)", function()
71-
local state = resolve(SHAPER, ELDER)
72-
assert.are.equal(state.exactCount, 2)
73-
assert.is_false(state.hasNoneConstraint)
74-
assert.are.equal(#state.specificInfluenceModIds, 2)
75-
assert.are.equal(count(state), 2)
76-
assert.is_false(needs(state))
65+
it("Any / Ignore caps min=1 (at least one influence)", function()
66+
local andGroup, topStats, slots = build(ANY, IGNORE)
67+
assert.are.equal(#topStats, 0)
68+
assert.are.same(andGroup, { { id = HAS_INFLUENCE, value = { min = 1 } } })
69+
assert.are.equal(slots, 1)
7770
end)
7871

79-
-- Any+Ignore: minCount=1 → pseudo_has_influence min=1 (1 slot)
80-
it("Any uses 1-slot pseudo_has_influence min=1", function()
81-
local state = resolve(ANY, IGNORE)
82-
assert.are.equal(state.minCount, 1)
83-
assert.are.equal(state.exactCount, nil)
84-
assert.are.equal(count(state), 1)
85-
assert.is_true(needs(state))
72+
it("Shaper / None caps exactly 1 of that specific", function()
73+
local andGroup, topStats, slots = build(SHAPER, NONE)
74+
assert.are.equal(#topStats, 0)
75+
assert.are.same(andGroup, {
76+
{ id = HAS_INFLUENCE, value = { min = 1, max = 1 } },
77+
{ id = HAS_SHAPER },
78+
})
79+
assert.are.equal(slots, 2)
8680
end)
8781

88-
-- Any+Shaper: exactCount=2 with one unnamed slot → needs pseudo_has_influence=2
89-
it("Any+Shaper uses 2-slot filter (specific + pseudo_has_influence=2)", function()
90-
local state = resolve(ANY, SHAPER)
91-
assert.are.equal(state.exactCount, 2)
92-
assert.is_false(state.hasNoneConstraint)
93-
assert.are.equal(#state.specificInfluenceModIds, 1)
94-
assert.are.equal(count(state), 2)
95-
assert.is_true(needs(state))
82+
it("Shaper / Ignore requires the specific without a count cap", function()
83+
local andGroup, topStats, slots = build(SHAPER, IGNORE)
84+
assert.are.equal(#topStats, 0)
85+
assert.are.same(andGroup, { { id = HAS_SHAPER } })
86+
assert.are.equal(slots, 1)
9687
end)
9788

98-
-- pseudo_has_influence mod ID is correct
99-
it("hasAnyInfluenceModId is pseudo.pseudo_has_influence_count", function()
100-
assert.are.equal(mock_queryGen._hasAnyInfluenceModId, "pseudo.pseudo_has_influence_count")
89+
it("Any / Any caps exactly 2 influences", function()
90+
local andGroup, topStats, slots = build(ANY, ANY)
91+
assert.are.equal(#topStats, 0)
92+
assert.are.same(andGroup, { { id = HAS_INFLUENCE, value = { min = 2, max = 2 } } })
93+
assert.are.equal(slots, 1)
10194
end)
10295

103-
-- Same specific influence on both sides is redundant at the item level and must
104-
-- collapse to the <specific> / None semantic (exactly 1 of that type), not
105-
-- double-count and not collapse to <specific> / Ignore. Verify via the
106-
-- needsHasInfluenceFilter gate that builds the pseudo_has_influence cap — without
107-
-- it, the query would match items with any number of influences including Shaper.
108-
it("Shaper+Shaper produces the full Shaper+None query state", function()
109-
local dup = resolve(SHAPER, SHAPER)
110-
local paired = resolve(SHAPER, NONE)
111-
assert.are.equal(dup.exactCount, paired.exactCount)
112-
assert.are.equal(dup.hasNoneConstraint, paired.hasNoneConstraint)
113-
assert.are.equal(#dup.specificInfluenceModIds, #paired.specificInfluenceModIds)
114-
assert.are.equal(dup.specificInfluenceModIds[1], paired.specificInfluenceModIds[1])
115-
assert.are.equal(count(dup), count(paired))
116-
assert.are.equal(needs(dup), needs(paired))
96+
it("Shaper / Any caps exactly 2 including Shaper", function()
97+
local andGroup, topStats, slots = build(SHAPER, ANY)
98+
assert.are.equal(#topStats, 0)
99+
assert.are.same(andGroup, {
100+
{ id = HAS_INFLUENCE, value = { min = 2, max = 2 } },
101+
{ id = HAS_SHAPER },
102+
})
103+
assert.are.equal(slots, 2)
117104
end)
118105

119-
it("Shaper+Shaper needs the pseudo_has_influence cap to enforce exactly 1", function()
120-
local state = resolve(SHAPER, SHAPER)
121-
assert.is_true(needs(state))
106+
it("Shaper / Elder requires both specifics without a count cap", function()
107+
local andGroup, topStats, slots = build(SHAPER, ELDER)
108+
assert.are.equal(#topStats, 0)
109+
assert.are.equal(#andGroup, 2)
110+
assert.are.equal(slots, 2)
111+
end)
112+
113+
-- Same specific on both sides is redundant at the item level and must produce
114+
-- the exact same filter set as <specific> / None (exactly 1 of that type).
115+
-- Without the None-constraint dedup, it would silently fall back to the
116+
-- <specific> / Ignore form and fail to cap the influence count.
117+
it("Shaper / Shaper produces the same filters as Shaper / None", function()
118+
local dupAnd, dupStats, dupSlots = build(SHAPER, SHAPER)
119+
local pairedAnd, pairedStats, pairedSlots = build(SHAPER, NONE)
120+
assert.are.same(dupAnd, pairedAnd)
121+
assert.are.same(dupStats, pairedStats)
122+
assert.are.equal(dupSlots, pairedSlots)
122123
end)
123124
end)
124125

src/Classes/TradeQueryGenerator.lua

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -193,11 +193,39 @@ local function needsHasInfluenceFilter(influenceState)
193193
end
194194

195195
local function countInfluenceFilters(influenceState)
196-
local cost = #influenceState.specificInfluenceModIds
196+
local count = #influenceState.specificInfluenceModIds
197197
if needsHasInfluenceFilter(influenceState) then
198-
cost = cost + 1
198+
count = count + 1
199199
end
200-
return cost
200+
return count
201+
end
202+
203+
-- Builds the influence-related trade query fragments from the two dropdown
204+
-- selections. Returns three values so callers can plug the fragments into the
205+
-- final query and size the remaining filter budget in a single call:
206+
-- andGroup: filters to append inside the top-level AND group
207+
-- topStats: filters to append directly to queryTable.query.stats
208+
-- (only non-empty when the pair represents "no influences", which
209+
-- requires a NOT clause rather than a value range)
210+
-- slotCount: total filter slots the influence fragments will consume
211+
local function buildInfluenceFilters(selection1, selection2)
212+
local state = resolveInfluenceQueryState(selection1, selection2)
213+
local andGroup = { }
214+
local topStats = { }
215+
if needsHasInfluenceFilter(state) then
216+
if state.exactCount == 0 then
217+
-- "has 0 influences" cannot be queried with a value range; use NOT instead.
218+
t_insert(topStats, { type = "not", filters = { { id = hasAnyInfluenceModId } } })
219+
elseif state.exactCount ~= nil then
220+
t_insert(andGroup, { id = hasAnyInfluenceModId, value = { min = state.exactCount, max = state.exactCount } })
221+
else
222+
t_insert(andGroup, { id = hasAnyInfluenceModId, value = { min = state.minCount } })
223+
end
224+
end
225+
for _, modId in ipairs(state.specificInfluenceModIds) do
226+
t_insert(andGroup, { id = modId })
227+
end
228+
return andGroup, topStats, countInfluenceFilters(state)
201229
end
202230

203231
local TradeQueryGeneratorClass = newClass("TradeQueryGenerator", function(self, queryTab)
@@ -1081,8 +1109,8 @@ function TradeQueryGeneratorClass:FinishQuery()
10811109
}
10821110

10831111
local options = self.calcContext.options
1084-
local influenceState = resolveInfluenceQueryState(options.influence1, options.influence2)
1085-
local num_extra = countInfluenceFilters(influenceState)
1112+
local influenceAndGroup, influenceTopStats, influenceSlotCount = buildInfluenceFilters(options.influence1, options.influence2)
1113+
local num_extra = influenceSlotCount
10861114
if not options.includeMirrored then
10871115
num_extra = num_extra + 1
10881116
end
@@ -1115,22 +1143,13 @@ function TradeQueryGeneratorClass:FinishQuery()
11151143

11161144
local andFilters = { type = "and", filters = { } }
11171145
local options = self.calcContext.options
1118-
if needsHasInfluenceFilter(influenceState) then
1119-
if influenceState.exactCount == 0 then
1120-
-- "has 0 influences" cannot be queried with a value range; use NOT instead
1121-
t_insert(queryTable.query.stats, { type = "not", filters = { { id = hasAnyInfluenceModId } } })
1122-
elseif influenceState.exactCount ~= nil then
1123-
t_insert(andFilters.filters, { id = hasAnyInfluenceModId, value = { min = influenceState.exactCount, max = influenceState.exactCount } })
1124-
else
1125-
t_insert(andFilters.filters, { id = hasAnyInfluenceModId, value = { min = influenceState.minCount } })
1126-
end
1127-
filters = filters + 1
1146+
for _, stat in ipairs(influenceTopStats) do
1147+
t_insert(queryTable.query.stats, stat)
11281148
end
1129-
1130-
for _, modId in ipairs(influenceState.specificInfluenceModIds) do
1131-
t_insert(andFilters.filters, { id = modId })
1132-
filters = filters + 1
1149+
for _, filter in ipairs(influenceAndGroup) do
1150+
t_insert(andFilters.filters, filter)
11331151
end
1152+
filters = filters + influenceSlotCount
11341153

11351154
if #andFilters.filters > 0 then
11361155
t_insert(queryTable.query.stats, andFilters)
@@ -1222,10 +1241,7 @@ function TradeQueryGeneratorClass:FinishQuery()
12221241
main:ClosePopup()
12231242
end
12241243

1225-
-- Test accessors for influence query state logic (not used in production paths)
1226-
TradeQueryGeneratorClass._resolveInfluenceQueryState = resolveInfluenceQueryState
1227-
TradeQueryGeneratorClass._countInfluenceFilters = countInfluenceFilters
1228-
TradeQueryGeneratorClass._needsHasInfluenceFilter = needsHasInfluenceFilter
1244+
TradeQueryGeneratorClass._buildInfluenceFilters = buildInfluenceFilters
12291245
TradeQueryGeneratorClass._hasAnyInfluenceModId = hasAnyInfluenceModId
12301246
TradeQueryGeneratorClass._INFLUENCE_IGNORE_INDEX = INFLUENCE_IGNORE_INDEX
12311247
TradeQueryGeneratorClass._INFLUENCE_NONE_INDEX = INFLUENCE_NONE_INDEX

0 commit comments

Comments
 (0)