Repository navigation
Fix the Trader scoring every Megalomaniac as no change - #2564
Open
juddisjudd wants to merge 1 commit into
Open
juddisjudd wants to merge 1 commit into
juddisjudd wants to merge 1 commit into
Conversation
TradeQuery:GetResultEvaluation weighs a Megalomaniac by adding its notables
with calcFunc({ addNodes = ... }). It looks each one up in tree.notableMap
and keeps only a node whose `recipes` field is set. The tree gives a notable
a `recipe` field, so no node passed the check, nothing was added, and every
Megalomaniac listing scored the same as the build without it.
Check `recipe`, the field the tree sets.
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.
Description of the problem being solved:
The Trader scores every Megalomaniac listing as no change.
TradeQuery:GetResultEvaluationweighs a Megalomaniac by adding its notables withcalcFunc({ addNodes = addedNodes }). It looks each notable up intree.notableMapand keeps it only whennode.recipes ~= nil. The tree names that fieldrecipe(PassiveTree.luareadsnode.recipe), so the check never passes,addedNodesstays empty, and the result is the base output.The check came in with f2b103d, which stopped a crash when a notable was missing from the map on an old tree. The
node andpart of the check stays, so that case is still skipped.This change checks
node.recipe.Steps taken to verify a working solution:
Measured by driving
TradeQueryheadless at 2450f2f. The line is the same ondev.GetMiscCalculatorgives +15.8% Combined DPS.Link to a build that showcases this PR:
Any build with an allocated jewel socket. Open the Trader, run "Find best" on the Megalomaniac row and look at the score of each result.
Before screenshot:
Every Megalomaniac result: +0.0%.
After screenshot:
Results score by what their notables add, for example +41.5%.