Fix Rocket reward filter identity and add quest task filtering - #1237
Closed
DannyM300 wants to merge 13 commits into
Closed
Fix Rocket reward filter identity and add quest task filtering#1237DannyM300 wants to merge 13 commits into
DannyM300 wants to merge 13 commits into
Conversation
Rocket reward filters were built from two sources that disagree about a reward's form id: confirmed scanner rewards (`incident`, form commonly 0) and the masterfile encounter pool (a real form, e.g. Deino 2291). Both reached the available list, so a species produced two identical-looking menu entries - `a276-0` and `a276-3163` - and `hasRocketPokemonFilter` compared the form exactly, so only whichever entry matched the grunt in hand did anything. Ticking the other silently did nothing. Match on the Pokemon ID instead, and collapse the available list to one key per species, keeping the non-zero masterfile form that saved filters already hold and the icon renderer expects. Grunt rewards do not meaningfully vary by form, and the SQL stage already selected stops by ID alone, so this makes the two stages agree. Both key shapes stay accepted, so existing saved filters keep resolving. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
invasionPokemon defaulted to true, so every Rocket reward filter (~1,400+ species/form keys) started enabled for every user. A grunt is shown if ANY of its possible rewards is enabled - and an unconfirmed grunt's reward is often a pool of several species, not one - so with almost everything on by default, nearly every grunt matched via the reward path regardless of what the grunt-type filter said. Excluding a species did nothing observable unless every other species that could appear in the same reward pool was also excluded. Reward filtering is additive on top of grunt-type filtering (deliberate OR semantics), not a replacement for it. Defaulting it off lets grunt-type filters work on their own, with reward filters as an opt-in narrowing - ticking Deino adds Deino's grunts on top, rather than the whole reward list needing to be manually emptied out first. Existing saved filters were seeded from the old default and need a reset in the UI to pick this up. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Confirmed rewards built their available key from the scanner's raw slot form, while the unconfirmed fallback and the endpoint path built it from the masterfile encounter pool's form. The two disagree often enough (Wobbuffet has shown up as both 602 and 2328 for the same grunt type) that which key got minted depended on which source happened to answer a given poll - so the same species could produce a different key on a later poll than an earlier one did. dedupeRocketPokemonKeys only reconciles keys within a single poll's result, not across separate ones over time, so a user could switch off the key from poll 1 and have poll 2 mint a fresh, still-enabled twin they never see appear - the reward looks stuck on no matter what they toggle, and no reset fixes it, because the reset itself just rebuilds from whatever the next poll happens to report. Always resolve the form from the masterfile encounter pool - regardless of whether the reward came from a confirmed scanner slot or the unconfirmed fallback - so every poll agrees on the same key for a given species. There is nothing left to reconcile after the fact, because nothing can diverge in the first place. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two separate places built Rocket reward filter keys. buildPokestops built one `a<id>-<form>` key per species that's an actual current grunt reward, sourced from the canonical available list. buildPokemon separately, unconditionally built a bare `a<id>` key for every single Pokemon species in the masterfile - not just current rewards - then base.js merged both into the same filter object. The UI's Rocket Pokemon tab only renders tiles for the canonical available-list keys, so buildPokemon's bulk-generated bare keys were never shown, never reachable via Set All, and never touched by a reset - but hasRocketPokemonFilter still matched against them by species ID. A user could switch off every visible entry for a species and a hidden sibling from this second generator would keep the grunt showing, with no way to see or disable it from the UI. This also explains why the reward-filter list looked enormous: buildPokemon generated a key for every masterfile species regardless of availability, not just current rewards. Removing this generator leaves buildPokestops as the sole source of Rocket reward filter keys. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds a Tasks tab alongside Items/Pokemon in the Quests filter panel. Task filtering was previously only reachable indirectly: pick a reward, then optionally narrow it to specific task conditions via its Advanced dialog. This adds the reverse: pick a task, then optionally narrow it to specific reward keys via the same Advanced mechanism - `k<title>-<target>` is a task-primary filter key, matched the same additive way reward-primary filters already are. Server side, `addTaskCondition` builds the inverse of the existing `conditions[rewardKey]` map alongside it during the same available-list scan, so `taskConditions[taskKey]` accumulates every reward key seen for that task without a second pass over the data. `matchesAdvancedFilter` is shared between the reward-primary and task-primary checks - they were near-identical duplicates, now one tested function. Both flow through the existing conditions caching/dual-source-merge path (DbManager, disk cache, GraphQL) that questConditions already used. Client side, the new key prefix plugs into the same subCategory/tab pattern rocketPokemon and invasions already use, and reuses QuestConditionSelector's exact shape in TaskRewardSelector, just listing rewards instead of task conditions. Defaults to enabled (`tasks: true`), consistent with the other quest reward filters (items/pokemon/candy/etc), not the invasion reward default flip from earlier today. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Tasks tab rendered blank on any deployment sourcing quest data from the Golbat in-memory endpoint (/api/fort/available) rather than direct SQL - the same dual-source split that bit rocket rewards earlier. This scanner's raw pokestop table has zero rows with quest_reward_type set at all; quest data only ever came through the endpoint's own process() function in pokestopAvailableMapper.js, which never learned about task keys. Mirrors the SQL-side addTaskCondition logic inline (this mapper is dependency-free by design, so it can't require the shared module - see the earlier rocket-key fix for the same constraint), and threads taskConditions through the endpoint-path return in Pokestop.js the same way conditions already was. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Task-only filtering did nothing: buildPokestopDnfFilters translates args.filters into the SQL/Golbat pre-filter that decides which rows are even fetched from the database, and its switch only understands reward keys - a task key (k<title>-<target>) fell through to the default case, failed Number.isFinite on a non-numeric id, and was silently dropped. With nothing else enabled, zero clauses were emitted for quests, so no task-only request ever fetched a matching row - the (correct) matching logic added earlier in secondaryFilter never got the chance to run, because the rows never arrived. expandTaskFilters resolves each enabled task key against state.db.taskConditions (the same data source that already powers the Tasks tab and its reward picker) and synthesizes the corresponding reward-key entries before the existing switch runs, so it picks them up through its own already-correct, already-tested clause logic rather than needing a new Golbat field invented for this. A task narrowed via .adv to specific rewards expands to only those; .all bypasses narrowing the same way it does for reward filters already. An explicit reward filter already present in args.filters is never overwritten by the synthesized one. This only affects the endpoint-backed (Golbat /api/fort/available) path, the only place buildPokestopDnfFilters is called - the SQL path builds its query separately and was already correctly handled by the secondaryFilter change. RM_DEBUG_TASK logging from the previous commit is left in for one more deploy to directly confirm rows now reach the matching stage; it'll come out once verified. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
RM_DEBUG_TASK confirmed the DNF expansion fix works end-to-end against live data (enabled task correctly matched its expected reward, other tasks correctly didn't) - no longer needed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Virtuoso cannot reliably measure a grid inside a hidden tab or a closed drawer. Reopening the drawer directly onto a persisted tab left that grid mounted against the closed drawer's stale viewport until the user switched tabs away and back. Gate both the grid's mount and its restoreStateFrom snapshot on shouldPersistGridState (drawer open and tab visible), so the remount on becoming active picks up a fresh measurement instead of a stale one. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
useStorage.getState() was saved verbatim, including every filter key at its default value - the whole point of a profile backup is the handful a user actually changed, not the full menu. The task filter feature just added a large new set of possible keys on top of the existing rocket/invasion/quest ones, making this far more likely to hit the existing 413 "backup too large" handling than before. createBackupData() diffs the current filters against the server's live defaults (useMemory.filters) and keeps only what differs. Safe on load: the backup write already clears localStorage and reloads the page, so useMapData's existing deepMerge(serverDefaults, loadedState) fills back in anything omitted because it matched the default at save time. Also adds error feedback to the Update button, which previously failed silently on the same 413 this was written to reduce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Collaborator
|
Please ask your Claude to open each independent thing on its own PR. |
Author
|
Thanks — understood. I'll split the Rocket fix, profile payload compaction, and task filtering into focused PRs so each can be reviewed independently. |
Author
|
Split into focused draft PRs as requested:
|
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
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.
Summary
Hi! This started with a user report that some Rocket reward Pokémon appeared more than once in the filter menu and that only one of the identical-looking toggles affected the displayed invasion.
While investigating and testing that fix, I also added an optional task-first quest filter requested by users, plus a profile payload fix needed to save the additional dynamic filters reliably.
I have tried to keep the SQL and Golbat/API paths consistent throughout. I appreciate that the maintainers know this codebase better and may prefer a different implementation or may want the changes split differently.
Rocket reward filter bug
Root cause
ReactMap can build available Rocket reward filters from two sources:
Those sources can describe the same reward Pokémon with different form IDs. For example, Taillow could be represented as both:
The menu label only uses Pokémon ID
276, so both entries were displayed as Taillow. However, their internal keys remained different, and a JavaScriptSetcould not deduplicate the two strings.Filtering then treated the same identity inconsistently:
Consequently, one duplicate could match confirmed scanner data while the other matched the masterfile/unconfirmed path. Both looked identical to the user, but one could appear to do nothing for the currently displayed grunt.
Fix
This canonicalises and deduplicates Rocket reward filter keys across data sources:
/api/fort/availablepaths.The intended matching remains additive: an invasion is shown when either its grunt type or one of its possible reward Pokémon is enabled, and hidden when neither is enabled.
A longer explanation and reproduction example are included in
docs/rocket-reward-filter-bug.md.User-requested quest task filtering
This adds a Tasks tab alongside the existing Items and Pokémon quest reward filters. It allows users to select quests by task rather than only by reward.
Task filters use a stable
k<title>-<target>key and support:Task-to-reward mappings are generated for both SQL-backed available PokéStop data and Golbat
/api/fort/availabledata. For the Golbat DNF path, enabled task keys are expanded into the relevant reward keys before query translation; the SQL/in-memory path applies the equivalent task-and-reward match directly.Task filters default to off, making the feature opt-in and preserving the behaviour of existing reward filters.
Profile payload/save fix
Available filters are dynamically merged into the client filter tree. Serialising the entire merged tree into every profile includes a large number of entries that are identical to the server defaults; adding task filters made this large enough to trigger an oversized database insert in testing.
Profile creation and updating now:
This keeps users' actual selections, including task selections, while avoiding a MySQL/MariaDB schema change.
Validation
main(8ce212ff).Happy to adjust the implementation or split the Rocket fix, task filtering and profile compaction if that would be easier to review.