Recipes dropped by the scan, and the non-retail recipe counts - #13
Open
uga wants to merge 3 commits into
Open
Conversation
ScanRecipes_NonRetail() classified every line of the trade skill window
through SkillTypeToColor, and stored it only if the lookup succeeded:
local color = SkillTypeToColor[skillType]
if color then
...
crafts[i] = format("%s|%s", color, craftInfo)
end
The table knew header, optimal, medium, easy and trivial. GetTradeSkillInfo()
returns two more. Blizzard's own TradeSkillTypeColor lists "nodifficulty",
drawn in white, which is what the client uses for a recipe that can never
grant a skill up - in practice mostly the epic patterns and formulas. And
"subheader", which the same function already expects a few lines above, when
it tests the first line of the list:
or (skillType ~= "header" and skillType ~= "subheader") then
Neither had a color, so neither was stored, and the recipe simply never
existed as far as Altoholic was concerned.
Dropping an entry does more than lose it. Crafts is indexed by the position
of the line in the window, so a skipped entry leaves a hole, and the length
of a table with a hole is any border Lua cares to return. Both readers walk
it with "for i = 1, #crafts", so a single white recipe can also truncate
every recipe after it. _IterateRecipes() carries a "Somehow the scan can set
an item to nil" guard which is that symptom, worked around at the far end.
Unknown types now fall back to grey rather than being dropped, so a type
added by a future patch costs at worst a wrong color, never a missing recipe
and never a hole. Grey is also the honest bucket for the two being named
here: a recipe the client refuses to color is one that grants no skill up.
This is not reproduced. It is the shape of a report of epic recipes missing
from tailoring and enchanting on Mists, where a maxed profession has them,
and it is a defect on its own terms either way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two defects in the same few lines, both about what happens when a line of
the trade skill window does not yield the link the code expects.
recipeID is declared once, above the loop, and only ever assigned inside
"if recipeLink then". An entry whose link carries no enchant id therefore
kept the id of whatever entry was scanned before it, and the leftover was
used twice over: to key the crafted item
resultItemsDB[recipeID] = maxMade + bit64:LeftShift(itemID, 8)
and to store the recipe itself, as "craftInfo = (recipeLink and recipeID)
and recipeID or """. The result is a recipe recorded under another recipe's
id, which reads back as a duplicate of its neighbour. It is now reset at
the top of every iteration, so a missing id stays missing.
The id was also extracted in two steps, and the first one was unchecked:
local found, _, enchantString = string.find(recipeLink, "^|%x+|H(.+)|h%[.+%]")
recipeID = tonumber(enchantString:match("enchant:(%d+)"))
A link that does not match the pattern leaves enchantString nil and the next
line throws. The scan wipes Crafts before the loop, so an error part way
through does not just skip that entry, it leaves the profession holding
however much had been read before it - and the window has to be reopened to
try again. Matching the id directly against the link cannot throw, and
returns exactly the same value for the normal "|Henchant:12345|h[Name]|h"
form.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_GetNumRecipesByColor() was written for retail and registered for every flavour, but the two _IterateRecipes() implementations do not hand the same thing to their callback. Retail passes one bit packed number per recipe, non-retail passes the color index, the id and the position, and calls back for recipes only, never for headers. So on non-retail the color index went through _GetRecipeInfo(), which read it as packed data and returned its two low bits: 1 orange -> 1 3 green -> 3 2 yellow -> 2 4 grey -> 0 then the counters were read back in retail's order, counts[3] first as the orange one. Orange and green were reported swapped: the Summary tooltip of a leatherworker with 9 orange, 6 yellow, 4 green and 131 grey recipes announced 4 orange and 9 green. The total was right, which is presumably why this survived - it is the sum of the same four counters either way, and grey landed on counts[0] by the same accident that moved the others. The split this restores is the one the pre-merge DataStore_Crafts.lua still carries, as _GetNumRecipesByColor_Retail and _GetNumRecipesByColor_NonRetail; the merged file kept the retail one only. Verified in game against the same character: 150 recipes, 4 green, 6 yellow, 9 orange. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Three defects in
ScanRecipes_NonRetail()and its reader, found while chasing a report of learnt recipes missing from Altoholic. Independent commits, take or leave them separately.Recipes whose difficulty the client does not color are dropped
SkillTypeToColorknowsheader,optimal,medium,easyandtrivial, and an entry is stored only if the lookup succeeds.GetTradeSkillInfo()also returnsnodifficulty— Blizzard's ownTradeSkillTypeColorlists it, drawn in white, for recipes that can never grant a skill up, in practice mostly epic patterns — andsubheader, which the same function already expects when it tests the first line of the list.Dropping an entry also leaves a hole in
Crafts, which is indexed by list position, and both readers walk it withfor i = 1, #crafts. The length of a table with a hole is any border Lua cares to return, so one white recipe can truncate the rest of the list too. The-- Somehow the scan can set an item to nilguard in_IterateRecipes()is that symptom.Unknown types now fall back to grey instead of being dropped, so a type added by a future patch costs a wrong color rather than a missing recipe.
Not reproduced. It matches a report of epic recipes missing from tailoring and enchanting on Mists that I could not reproduce myself, and it is a defect on its own terms either way.
A recipe could inherit the previous recipe's id
recipeIDis declared above the loop and only assigned insideif recipeLink then, so an entry whose link carries no enchant id kept the previous entry's — used both to keyresultItemsDBand to store the recipe itself.The extraction was also unchecked:
string.find(...)thenenchantString:match(...)on the next line throws if the link does not match the pattern. SinceCraftsis wiped before the loop, an error part way through leaves the profession holding only what had been read so far. Matching the id directly against the link cannot throw and returns the same value for the normal form.Orange and green recipe counts are swapped on non-retail
_GetNumRecipesByColor()is written for retail and registered for every flavour, but the two_IterateRecipes()implementations do not pass the same thing to their callback: retail passes bit packed data, non-retail passes the color index. The index went through_GetRecipeInfo(), which read it as packed data, then the counters were read back in retail's order.A leatherworker with 9 orange, 6 yellow, 4 green and 131 grey recipes was announced as 4 orange and 9 green in the Summary tooltip. The total was right, which is presumably why it went unnoticed. The split restored here is the one
DataStore_Crafts.luastill carries as_GetNumRecipesByColor_Retail/_NonRetail; the merged file kept the retail one only.Verified in game on Classic Era: 150 recipes, 4 green, 6 yellow, 9 orange.
Tested on Classic Era and TBC Anniversary. The scan change is the only one not backed by a reproduction.