Add radius jewel finder#9869
Closed
mcagnion wants to merge 60 commits into
Closed
Conversation
…in compute detail In compute mode, regular radius jewels (Might of the Meek, etc.) had no detailText since computeSocketImpact doesn't evaluate radius nodes. Now looks up nodesInRadius for the jewel's radiusIndex and shows "N matches" (notable/keystone count) instead of the uninformative "jewel only" fallback. Falls back to blank when no radius index is defined or no notables are found. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
'WeightedScore' is not a real calc output field, so getImpactValue was returning 0 for it. This caused realBaseline=0, making all pct/%/Pt columns show 0, and breaking candidate selection in connectionless paths (where delta = value - baseValue = 0 - 0 = 0). Fix: detect isWeightedScore and compute an actual ratio score via WeightedScore.computeRatioScore(buildBaseOutput, output, weights)*1000. This gives a meaningful non-zero baseline and consistent deltas across all compute paths. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace the isWeightedScore-specific branch with a generic getValue callback check. Any powerStatList entry that cannot be read directly from calc output can now provide its own getValue(output, build) function, keeping getImpactValue decoupled from specific stat types. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Replace deep copies (copyTableSafe) of full calculator outputs with extractTooltipStats, which copies only scalar fields and the small tables needed by AddStatComparesToTooltip (FailLists, ReqItems, Minion). Full calc outputs contain heavy sub-tables (SkillDPS, env, modDB, etc.) that were deep-copied for every result row — with ~2400 rows this could reach 25 GB. The filtered snapshot is ~2-5 KB per output vs ~0.5-5 MB, reducing total memory from ~25 GB to ~25 MB. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ariant jewel flow Remove the special-case two-step compute (pick socket → rank variants) for The Light of Meaning. It now uses computeBestVariantSocketImpact like other multi-variant jewels (Stat Conversion, Dreams & Nightmares, etc.), iterating all sockets × 13 stat variants and showing the best variant per socket in the standard computeSocket result view. - Build LOM variants with per-variant rawText via getUniqueVariantRawText - Remove isLightOfMeaning flag, dedicated socket/LOM-variant dropdowns, computeVariantImpact method, computeVariant list mode - Adapt preview to use previewFromRawText when a variant is selected - Add GetVirtualScreenSize stub to HeadlessWrapper - Update tests to exercise computeBestVariantSocketImpact for LOM Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nts from item variantList Replace hardcoded variant tables for The Light of Meaning and Split Personality with a generic buildVariantsFromUniqueItem helper that reads the item's variantList dynamically. This makes variant discovery resilient to additions or reordering in the unique data. Other multi-variant jewel types (Stat Conversion, Attribute Conversion, Combat Focus, Dreams & Nightmares, Tempered & Transcendent) group distinct uniques rather than variants of a single item, so they remain as explicit lists. Thread of Hope variants carry additional radiusIndex data tied to tree geometry, so they also stay hand-built. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Derive Thread of Hope ring names from item variantList instead of hardcoded array - Extract shared scoreUnallocNotablesAndKeystones for Impossible Escape and Thread of Hope (identical inline lambdas) - Simplify Tempered & Transcendent score: call scoreRadiusAttributes directly instead of indirecting through getTemperedTranscendentVariants Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Replace hardcoded LOM rawTexts with dynamic buildVariantsFromUniqueItem calls so tests adapt automatically when game data changes. Expose buildVariantsFromUniqueItem as a class method for testability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove redundant ternary in compute dispatch, translate French comment, use cached radiusIndexByLabel instead of inline lookup. Make variant name tests data-driven (Tempered/Transcendent, Split Personality) and add occupiedMode coverage (safe >= free, all > free). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…to scoped local local lines was declared inside the else block, making it inaccessible to the return statement after end. The tooltip silently fell through, showing the previously hovered jewel's tooltip instead. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… variant discovery
Remove the hardcoded Foulborn checkbox, raw text constants, and variant
tables. Foulborn jewels are now discovered dynamically from
data.uniques.generated via discoverFoulbornVariants(), which returns {}
gracefully when no Foulborn data exists (feature branch). After merge
into integration/current, Foulborn variants appear automatically as
entries in the jewel variant dropdown.
- Add discoverFoulbornVariants() + enrichment functions for complex
jewels (Unnatural Instinct, Inspired Learning, Intuitive Leap)
- Rewrite buildJewelTypes to call discovery + appendFoulbornVariants
- Replace isSelectedFoulbornActive() with hasVariantFamilies()
- Rewrite computeIntuitiveLeapSocketImpact to use variant flags
- Simplify all preview functions (remove isFoulborn parameter)
- Delete 13 hardcoded FOULBORN raw text constants, buildJewelRawText,
UNNATURAL_INSTINCT_FOULBORN_VARIANTS, buildDualModFoulbornVariants,
DREAMS_NIGHTMARES_FOULBORN_VARIANTS
- Fix pre-existing test bugs: computeSocketImpact positional args
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
previewFromRawText filtered out mod lines with `extra` set, which excludes mods not recognized by the mod parser (e.g. Inspired Learning's trigger-on-kill condition). Remove the filter so all active mod lines appear in jewel preview tooltips. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…socket view Adds an "All jewels" entry at the top of the jewel type dropdown that computes every jewel type across all sockets in a single run. Results are shown in a new `computeSocketAll` column layout with a Jewel column. A "View" dropdown (All results / Best per socket) lets the user toggle between the full flat ranking and a deduplicated view showing only the single best jewel per socket. Toggling re-filters cached rows without recomputing. Also extracts `buildComputeRows` and `filterBestPerSocket` as reusable local functions shared by both the single-jewel and all-jewels paths. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Wraps the per-type child progress tracker to prefix every tick label with the current jewel type name, so the progress bar reads "Computing... 42% | Might of the Meek | Marauder Start" instead of just "Computing... 42% | Marauder Start". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…-type comparison Evaluate all jewel types × all sockets using static scoring (no calcFunc), with type-specific logic for Thread of Hope, Impossible Escape, Split Personality, variant types, and simple types. Reuses the All results / Best per socket view toggle and result cache from Compute All. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…odules Extract jewel type definitions, variants, scoring, and preview into RadiusJewelData.lua (1108 lines). Extract all compute methods and connectionless plan logic into RadiusJewelCompute.lua (862 lines). RadiusJewelFinder.lua retains UI, orchestration, and socket helpers (2238 lines). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nded socket Add an Apply button that creates the selected unique jewel and equips it in the recommended socket. The rawText (with correct variant baked in) is carried on every result row across Compute, Find, and All Jewels modes. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…est per socket - Display elapsed time (ms or seconds) in the status label after Find and Compute complete. - Sort Best per socket results by score descending instead of undefined hash table iteration order. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Show a highlight ring around the socket or detail node in the mini tree viewer popup when hovering result rows, making the recommended socket visually obvious. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…mpute - Skip Impossible Escape variants with 0 unallocated candidates - Skip Thread of Hope ring×socket combos with 0 candidates - Skip Intuitive Leap sockets with 0 candidates - Hoist Thread of Hope item creation out of the socket loop - Fix test helper to pick a keystone with actual candidates Reduces All Jewels Compute time by ~18% (22.1s -> 18.1s on test build). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ss compute In All Jewels mode, only the final delta matters — intermediate plan steps are not displayed. Skip per-node calcFunc calls and jump straight to the final evaluation for connectionless types (Thread of Hope, Impossible Escape, Intuitive Leap), reducing calcFunc calls from 1+N+K+1 to 1+N+1 per socket. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ts in fast plan For Thread of Hope, share the candidate delta cache across all sockets and all ring variants (key: TOH|field instead of TOH|field|socket|variant). For Intuitive Leap, share across sockets (key: IL|field|variant instead of IL|field|variant|socket). The marginal delta of a passive node is approximately socket/variant independent, so cached deltas can safely be reused for candidate selection. Final results are always recomputed with the correct context. Also make the jewel-only calcFunc call lazy: skip it entirely when all candidates already hit the shared cache. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Simplify Impossible Escape cache key to IE|field|variant (removing the free/occupied socket distinction). Candidates are per-keystone not per-socket, so deltas are reusable across groups. Occupied socket groups now benefit from deltas already cached by the free group. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ant jewels BuildRaw() outputs "Rarity: UNIQUE" (uppercase) but the gsub stripping the rarity header was matching "Rarity: Unique" (mixed case). The raw text kept the UNIQUE header, and Apply prepended a second "Rarity: Unique" line, producing a double-rarity item that parsed incorrectly. Also remove hardcoded limit=2 on Split Personality since the raw text already contains "Limited to: 2". Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… compute Two optimizations for connectionless jewel compute: 1. Early pruning: skip the final calcFunc when the sum of cached individual deltas cannot beat the current best variant for the socket. Applies to both ToH (ring variants) and IE (keystone variants). 2. Two-pass plan for single-jewel mode: pass 1 finds the best ring variant using skipPlanSteps (one calcFunc per non-pruned variant), pass 2 recomputes only the winner with full plan steps. Avoids computing expensive step-by-step breakdowns for losing variants. Also default Max pts to 20, which bounds plan step count and dramatically reduces calcFunc calls (2225 -> ~130 on a typical build). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ps, IE group dedup Major performance improvements for connectionless jewel compute: - Fast mode filters candidates to notables/keystones only for ToH and IE, dramatically reducing delta cache fill cost (~300 nodes -> ~30-50) - Plan steps (pass 2) computed only for top 5 results instead of all sockets, cutting calcFunc calls from ~336 to ~105 for ToH - IE: skip redundant free socket groups when budget covers all candidates - IE: two-pass plan steps like ToH (pass 1 skipPlanSteps, pass 2 best only) - Fix All Jewels progress label: recursive wrapProgress so nested children always prefix with jewel type name - IE marked as isSocketIndependent: Best per socket assigns socket-dependent jewels first, then IE to remaining cheapest sockets - Default compute method changed to Fast, default max pts to 20 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… filter condition - Remove duplicate final calcFunc call in plan steps path: the last loop iteration already computes the same output as the separate final call. Saves 1 calcFunc per socket in single-jewel mode. - Unify notableOrKeystoneOnly condition between ToH and IE to consistently use `skipPlanSteps or methodId == "fast"`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…d and add unit tests Extract the local filterBestPerSocket function to a class method so it can be tested directly. Add 11 unit tests covering one-per-socket dedup, score sorting, jewelLimit enforcement, socket-dependent vs socket-independent allocation priority, tiebreak by points, shared limits, empty input, and input immutability. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Remove the early-exit guard in updatePreview() that checked jewelTypeSelect.dropped. The DropDownControl fires its selFunc callback before setting self.dropped = false, so updatePreview() was always wiping the preview data and returning empty during type selection. Visibility while the dropdown is open is already handled by the previewList.shown function. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…yboard shortcuts - Apply button greyed out when no result is selected - Tooltip on Apply showing which jewel goes in which socket - Double-click a result row to apply directly - Escape cancels compute and closes popup - Ctrl+Enter triggers Compute Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…stead of math.huge When a socket costs 0 points, per-point sort key now uses the raw value (pct or score) so free results rank among themselves by impact instead of all tying at infinity. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… action colors When a "Limited to: 1" jewel is already equipped, temporarily remove it (and its connectionless dependent nodes) before computing so deltas reflect moving the jewel rather than adding a second copy. Color-code socket labels: green=new, blue=move, orange=replace, grey=keep. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…nnectivity check findConnectionlessDependentNodes was using treeData.nodes (spec.tree.nodes) for the linked field, but linked arrays are only populated on spec.nodes. The BFS never ran, so all allocated radius nodes were incorrectly flagged as jewel-dependent. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Cover findEquippedJewelSockets (limit logic, title matching), findConnectionlessDependentNodes (isolated vs connected BFS, IE path), and removeEquippedJewels/restoreEquippedJewels (round-trip, slot cleanup, dependent node removal and preservation). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ta for moves, movereplace action - findEquippedJewelSockets: always return equipped sockets with .atLimit flag instead of returning empty when jewel has no limit (fixes THoM/IL/MoM showing "replace" instead of "keep" for their own socket) - Only call removeEquippedJewels when .atLimit (limited jewels at capacity) - Only set existingSocketId (triggers "move" action) when .atLimit - Compute net delta for move rows: subtract keepDelta so the displayed gain reflects moving vs keeping, not adding vs nothing - Add "movereplace" action (purple) to distinguish move-to-occupied from move-to-free (blue) in both list colors and detail panel - Update tests: assert .atLimit on findEquippedJewelSockets results Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Shrink and anchor the Radius Jewel Finder layout so the popup fits within 1024px-wide displays, matching the Timeless Jewel popup style. Adds regression coverage for popup/control bounds plus bottom and header margins. Review: 20260512T114219Z-codex-9345dd606fdb
Replace local nid variables with nodeId in Radius Jewel Finder connectionless-node handling and tests. Validation: busted.bat --cpath=../runtime/?.dll --output=gtest ../spec/System/TestRadiusJewelFinder_spec.lua
Rename local abbreviations and the move-replace action key for readability, and convert the remaining Massive-radius comment to English. Validation: busted.bat --cpath=../runtime/?.dll --output=gtest ../spec/System/TestRadiusJewelFinder_spec.lua Validation: lexicon-diff on feature targets and modified ranges
Rename connectionless helpers to disconnected-passive wording, replace budget/access/cost vocabulary with point-based names, and expand test/helper abbreviations. Review: 20260512T160616Z-claude-approved. Test: Windows busted.bat targeted TestRadiusJewelFinder_spec.lua (70 passed). Manual: user checked PoB UI.
Tighten result detail text, use compact all-jewels preview layout, and hide empty-result headers to avoid overlap. Reviewed-by: claude 20260512T190630Z-claude-approved
Treat jewels stored in unallocated sockets as inactive for occupancy, limits, and compute baselines while warning when Apply would overwrite one.
All-jewel Find compared heuristic scores across unrelated jewel families, so hide it and route All jewels to stat-based Compute results only. Also keeps result hover handling testable via GetHoverInfo and updates the RadiusJewelFinder system spec. Reviewed by Claude in request 20260513T191129Z-codex-401e1ec183cb; manually verified in PoB.
Backports the integration/current manifest/hash correction so RadiusJewelCompute.lua and RadiusJewelData.lua are included before any upstream PR from this feature branch.
Update manifest.xml after rebasing the Radius Jewel Finder TreeTab entry onto current origin/dev. Reviewed in 20260523T090038Z-codex-9a8ecce5100e.
Drop the stale Radius Jewel Finder stub after rebasing onto origin/dev, where HeadlessWrapper already gets virtual screen sizing from the shared headless path. Reviewed in 20260523T092557Z-codex-0f35438f3607.
Rename the Radius Jewel Finder busted tag so the upstream cspell check does not treat it as an invented word.
Contributor
Author
|
Closing this draft for now so I can reopen it with a cleaner commit history, screenshots, and a clearer description. |
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.
Fixes # .
Description of the problem being solved:
Adds a Tree tab tool to compare radius and placement jewels across passive tree sockets and rank the results for the selected stat.
The draft currently covers:
Find Radius Jewelentry point on the Tree tabSteps taken to verify a working solution:
busted.bat --directory=src --lpath="../runtime/lua/?.lua;../runtime/lua/?/init.lua" --cpath="../runtime/?.dll;./csrc/?.dll;./csrc/?/?.dll" --helper=HeadlessWrapper.lua --exclude-tags=builds,benchmark --output=gtest ../spec/System/TestRadiusJewelFinder_spec.lua- 72 passedbusted.bat --run=default --cpath="../runtime/?.dll;./csrc/?.dll;./csrc/?/?.dll" --exclude-tags=builds,benchmark --output=gtest- 295 passed / 1 unrelated Windows-local failure inTestTradeQueryCurrency_spec.lua:67("5 chaos, 10 div"vs"10 div, 5 chaos"); this branch does not modify TradeQuery filesgit diff --check origin/dev...HEAD- passedspellcheckandcheck_modcachepass on the draft PR; GitHubrun_testscurrently fails withluajit: not enough memoryafter loading the 3.13 passive tree, before a test assertion failureLink to a build that showcases this PR:
Draft: local build link pending.
Before screenshot:
Draft: pending Tree tab screenshot before this feature.
After screenshot:
Draft: pending Tree tab screenshot with
Find Radius Jeweland finder popup results.