Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions config/default.json
Original file line number Diff line number Diff line change
Expand Up @@ -539,6 +539,7 @@
"candy": true,
"xlCandy": true,
"pokemon": true,
"tasks": false,
"invasions": false,
"allInvasions": true,
"invasionPokemon": true,
Expand Down
168 changes: 168 additions & 0 deletions docs/rocket-reward-filter-bug.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,168 @@
# Duplicate Rocket Pokemon filters in ReactMap main

## Scope

This document describes the original bug in ReactMap `main` at commit
`50cb6cfd` (`v1.49.1`): some Rocket reward Pokemon appeared twice in the
filter menu. Taillow is the known example used while fixing it.

This is separate from later issues encountered while developing the fix.

## Symptom

The Rocket Pokemon tab could contain two tiles both labelled **Taillow**.
Internally, however, they were different filters:

```text
a276-0
a276-3163
```

The key format is:

```text
a<pokemonId>-<formId>
```

Both keys identify Pokemon 276, but they contain different form IDs.

## Why main creates two entries

ReactMap builds its available Rocket reward filters from two sources.

### Confirmed scanner data

The `rocketPokemon` query reads the Pokemon and form stored in a confirmed
incident lineup. For Taillow, the scanner can contribute:

```text
a276-0
```

### Event masterfile fallback

`applyRocketPokemonFallback` also adds every configured possible reward from
`state.event.invasions`. For the same Taillow reward, the masterfile
contributes:

```text
a276-3163
```

Both values are added to a JavaScript `Set`. A `Set` removes only identical
strings; it does not know that both strings represent the same Pokemon.
Therefore `a276-0` and `a276-3163` both remain in the available list.

The client then creates the label using only the Pokemon portion of each key:

```js
const name = t(`poke_${id.slice(1).split('-')[0]}`)
```

For both keys, that expression extracts `276`. Consequently, two different
internal keys are rendered with the same visible name: **Taillow**.

## Why only one duplicate may work

On main, the database query first strips the form and searches by Pokemon ID:

```js
rocketPokemon.push(pokestop.slice(1).split('-')[0])
```

Both Taillow keys therefore retrieve candidate grunts for Pokemon 276.

ReactMap then runs `invasionMatchesFilters` as a secondary filter. Its original
matcher requires the complete form-specific key:

```js
filters[`a${pokemonId}-${formId ?? 0}`] || filters[`a${pokemonId}`]
```

This creates inconsistent behaviour:

- A confirmed scanner record using form `0` matches `a276-0`.
- An unconfirmed grunt checked against the masterfile form matches
`a276-3163`.
- Both tiles look like Taillow, but each can affect a different data path.

The SQL stage treats Rocket rewards as a species, while the secondary filter
treats them as a species-and-form combination.

## Direct fix

Commit `ce0f73f0` fixes the original duplicate bug with two changes.

### 1. Match Rocket rewards by Pokemon ID

`hasRocketPokemonFilter` now checks whether any selected Rocket key belongs to
the requested Pokemon ID, regardless of its form suffix.

It accepts both supported shapes:

```text
a276
a276-3163
```

The comparison includes a key boundary, so Pokemon 1 cannot accidentally match
Pokemon 12.

This makes the secondary filter agree with the existing SQL query: Rocket
rewards are filtered by species ID, not by scanner/masterfile form ID.

### 2. Deduplicate available keys by species

`dedupeRocketPokemonKeys` groups every `a` key by Pokemon ID and keeps only one
key per species. For the original Taillow pair:

```text
Input: a276-0, a276-3163
Output: a276-3163
```

The non-zero masterfile form is retained for compatibility with existing icon
and saved-filter handling. Because matching now uses Pokemon ID, that surviving
key works for scanner form `0` and masterfile form `3163` alike.

Deduplication is applied to both ways ReactMap obtains available Pokestop data:

- Golbat `/api/fort/available`
- The SQL `getAvailable` path

## Files changed by the direct fix

```text
server/src/filters/pokestop/rocketPokemonKeys.js
server/src/filters/pokestop/rocketPokemonKeys.test.js
server/src/models/Pokestop.js
```

## Later hardening in the final branch

The direct fix removes the duplicate entries found on main. Two follow-up
changes make that species-level approach reliable over time:

- `425077ba` derives the displayed key from the masterfile consistently, so a
later poll cannot rename the same species because its scanner form changed.
- `a2fe5081` removes hidden all-form Rocket filters from the default filter
builder, ensuring the one visible species filter is authoritative.

Those follow-ups support the completed implementation, but they are not the
reason main originally displayed two Taillow tiles. Changing Rocket filters to
default off was only a temporary workaround and is not part of the fix.

## Verification

After deploying and restarting ReactMap:

1. Open the Rocket Pokemon filters while Taillow is available.
2. Confirm that exactly one Taillow tile is shown.
3. Confirm that the same tile matches confirmed and unconfirmed Taillow grunts.
4. Turn off both Taillow and the relevant grunt-type filter; the grunt should
disappear.
5. Turn on either Taillow or the grunt type; the grunt should appear through
the intended OR filter behaviour.

Automated tests cover duplicate removal, form-independent matching, legacy
form-less keys, and Pokemon IDs with common numeric prefixes.
3 changes: 3 additions & 0 deletions packages/locales/lib/human/en.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@
"nests": "Nests",
"pokestops": "PokéStops",
"pokemon": "Pokémon",
"tasks": "Tasks",
"wayfarer": "Wayfarer",
"scan_areas": "Scan Areas",
"jump_to_areas_attribution": "Search powered by OpenStreetMap",
Expand Down Expand Up @@ -596,6 +597,7 @@
"cell_blocked": "Cell Blocked",
"poi_color": "POI Color",
"quest_condition": "Quest Condition",
"task_reward": "Reward",
"always_show_labels": "Always Show Labels",
"scan_areas_options": "Scan Areas Options",
"historic_rarity": "Historic Rarity",
Expand Down Expand Up @@ -755,6 +757,7 @@
"developer": "Developer",
"raid_override": "Raid Override",
"search_rocket_pokemon": "Search Rocket Pokémon",
"search_tasks": "Search Tasks",
"main": "Main",
"extra": "Extra",
"select": "Select",
Expand Down
1 change: 0 additions & 1 deletion server/src/filters/builder/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,6 @@ function buildDefaultFilters(perms) {
hasDualQuestLayer && perms.pokestops ? false : undefined,
standard: new BaseFilter(),
filter: {
...pokemon.rocket,
...buildPokestops(perms, defaultFilters.pokestops),
...pokemon.quests,
},
Expand Down
7 changes: 0 additions & 7 deletions server/src/filters/builder/pokemon.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const { getWildFilterKey } = require('../pokemon/getWildFilterKey')
* raids: { [key: string]: BaseFilter },
* quests: { [key: string]: BaseFilter },
* nests: { [key: string]: BaseFilter },
* rocket: { [key: string]: BaseFilter },
* stations: { [key: string]: BaseFilter },
* }}
*/
Expand All @@ -24,7 +23,6 @@ function buildPokemon(defaults, base, custom) {
stations: { global: new BaseFilter() },
quests: { global: new BaseFilter() },
nests: { global: new BaseFilter() },
rocket: { global: new BaseFilter() },
}
const energyAmounts = new Set([
...defaults.pokestops.baseMegaEnergyAmounts,
Expand All @@ -43,11 +41,6 @@ function buildPokemon(defaults, base, custom) {
pokemon.raids[rawKey] = new BaseFilter(defaults.gyms.pokemon)
pokemon.stations[rawKey] = new BaseFilter(defaults.stations.pokemon)
pokemon.quests[rawKey] = new BaseFilter(defaults.pokestops.pokemon)
if (state.db.filterContext.Pokestop.hasConfirmedInvasions) {
pokemon.rocket[`a${rawKey}`] = new BaseFilter(
defaults.pokestops.invasionPokemon,
)
}
pokemon.nests[rawKey] = new BaseFilter(defaults.nests.allPokemon)
})
if ('family' in pkmn) {
Expand Down
10 changes: 10 additions & 0 deletions server/src/filters/builder/pokestop.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,13 +63,19 @@ function buildPokestops(perms, defaults) {
quests[avail] = new BaseFilter(defaults.xlCandy)
} else if (avail.startsWith('m')) {
quests[avail] = new BaseFilter(defaults.megaEnergy)
} else if (avail.startsWith('k')) {
// Task-primary filter: the reverse of the per-reward `.adv` narrowing
// above. Same reward-type default (`tasks`) since a task is just
// another way of selecting the same underlying quest rewards.
quests[avail] = new BaseFilter(defaults.tasks)
} else if (
!avail.startsWith('i') &&
!avail.startsWith('l') &&
!avail.startsWith('a') &&
!avail.startsWith('b') &&
!avail.startsWith('f') &&
!avail.startsWith('h') &&
!avail.startsWith('k') &&
!Number.isInteger(+avail.charAt(0))
) {
log.warn(
Expand All @@ -83,6 +89,10 @@ function buildPokestops(perms, defaults) {
if (avail.startsWith('i')) {
quests[avail] = new BaseFilter(defaults.allInvasions)
}
// Reward filters come only from the canonical available list. Building
// an `a` key for every masterfile form creates hidden siblings that the
// menu cannot switch off, while reward matching intentionally ignores
// form differences between scanner and masterfile data.
if (
avail.startsWith('a') &&
state.db.filterContext.Pokestop.hasConfirmedInvasions
Expand Down
45 changes: 42 additions & 3 deletions server/src/filters/fort/pokestop.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,43 @@ const { parseIdFormPair } = require('./parseIdForm')
// display_type 1-4 = rocket, 7 goldstop, 8 kecleon, 9 showcase.
const ROCKET_INCIDENT_DISPLAY_TYPES = [1, 2, 3, 4]

/**
* Expands enabled task-primary filters (`k<title>-<target>`) into their
* reward-primary equivalents, so the switch below - which only understands
* reward keys - picks them up automatically without needing its own clause
* type. A task narrowed via `.adv` to specific rewards (the reverse Advanced
* dialog) expands to only those; an unnarrowed task expands to every reward
* `taskConditions` has ever seen it grant.
*
* Presence of a key in `filters` already means enabled - `trimFilters`
* strips disabled entries and the `enabled` field itself before the client
* ever sends this - so no `.enabled` check is needed here, matching every
* other key in this file.
* @param {Record<string, any>} filters
* @param {Record<string, {rewards?: string[]}>} [taskConditions]
* @returns {Record<string, any>}
*/
function expandTaskFilters(filters, taskConditions) {
if (!taskConditions) return filters
const taskKeys = Object.keys(filters).filter((key) => key.startsWith('k'))
if (!taskKeys.length) return filters
const expanded = { ...filters }
taskKeys.forEach((taskKey) => {
const filter = filters[taskKey]
const rewards =
filter?.adv && !filter.all
? Array.isArray(filter.adv)
? filter.adv
: filter.adv.split(',')
: taskConditions[taskKey]?.rewards
if (!rewards) return
rewards.forEach((rewardKey) => {
if (!expanded[rewardKey]) expanded[rewardKey] = { all: false, adv: '' }
})
})
return expanded
}

/**
* Translate a pokestop's `args.filters` into ApiFortDnfFilter[] clauses.
*
Expand Down Expand Up @@ -38,12 +75,14 @@ const ROCKET_INCIDENT_DISPLAY_TYPES = [1, 2, 3, 4]
* from an optional invasion check that no Golbat clause can safely track);
* secondaryFilter confirms the specific reward.
*
* @param {Record<string, any>} filters args.filters
* @param {Record<string, any>} rawFilters args.filters
* @param {Record<string, any>} [eventInvasions] state.event.invasions (grunt→reward map, used for grunt-class exclusion)
* @param {Record<string, {rewards?: string[]}>} [taskConditions] state.db.taskConditions, used to expand task-primary keys into reward keys
* @returns {object[]}
*/
function buildPokestopDnfFilters(filters, eventInvasions) {
if (!filters || typeof filters !== 'object') return []
function buildPokestopDnfFilters(rawFilters, eventInvasions, taskConditions) {
if (!rawFilters || typeof rawFilters !== 'object') return []
const filters = expandTaskFilters(rawFilters, taskConditions)
const {
onlyAllPokestops,
onlyArEligible,
Expand Down
Loading
Loading