Enchanting recipes that create an item, and the craft window icon - #12
Open
uga wants to merge 4 commits into
Open
Enchanting recipes that create an item, and the craft window icon#12uga wants to merge 4 commits into
uga wants to merge 4 commits into
Conversation
The enchanting scan read GetCraftItemLink() and kept only what matched
"enchant:(%d+)". A handful of entries create an actual item instead - the
runed rods, the oils, enchanted leather and thorium - and return an item link,
so the id came out nil and the recipe was stored as the unusable "1|nil".
It also indexed the link without checking it exists, which would error outright
on any entry without one.
Those crafts now fall back to the item id, which is what the trade skill path
already stores for the same case ("id = itemID in vanilla, recipeID in LK"),
and entries with no usable id are skipped rather than written as a broken
value. Reagent links are guarded the same way.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
For enchanting, RecipeRow deliberately skips the item lookup and takes the icon from GetSpellInfo(recipeID). That holds for real enchants, whose id is a spell, but not for the entries that create an item - the runed rods, the oils, enchanted leather - which are now stored with an item id, so the spell lookup returns nothing usable and the row draws a wrong icon. The reader cannot tell an item id from an enchant id by looking at it, but the craft window knows the icon of every entry, and the stored format already has a third field for it that nothing ever filled. The scan now writes GetCraftIcon() there, and the row prefers it over the spell lookup when present. Recipes that carry no stored icon, which is every other profession, are unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This was referenced Aug 1, 2026
The enchanting branch of ScanRecipes_NonRetail() fills Crafts by index over GetNumCrafts(), but it is not the first to write there: the tradeskill loop above it has already run over GetNumTradeSkills() entries. Crafts is wiped once, before that loop, so whatever the loop stored past the length of the craft list survives into the enchanting data. That the two can be live at once is not hypothetical - the line right above wipes Categories for exactly that reason, "it can erroneously get it from the tradeskill window". Whatever leaks the categories leaks the recipes too, there was just nothing to notice past the end of a shorter list. Storing the entries whose difficulty the client does not color makes that loop keep more lines than it used to, so it also makes this leak bigger. Wiping here, where the craft list takes over, bounds it: what the pane shows for enchanting is the craft list and nothing else. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A maxed enchanter was reported as "0 green / 0 yellow / 101 orange", while every other profession broke its recipes down correctly. Old school enchanting does not go through the tradeskill window, it has a craft window of its own, and the scan for it wrote the difficulty as a flat 1 with a comment saying it was not known where to get it. It is in the third return of GetCraftInfo, in the same words the tradeskill window uses - "optimal", "medium", "easy", "trivial" - so it maps through the same SkillTypeToColor table as everything else. Checked on a live Classic Era client: the window answers "optimal" for some entries and "medium" for others, so the information was there the whole time. The flat 1 stays as the fallback for anything that table has no entry for. Every character has to open their enchanting window once for the stored difficulties to be rewritten. 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.
Summary
Two fixes in
DataStore_Crafts, both about enchanting. Verified in game on Classic Era and TBC Anniversary.1. Store enchanting crafts that create an item rather than an enchant
Some enchanting recipes produce an item (rods, oils, wands) instead of applying an enchant. They were stored as if they were enchants, so the resulting item was lost.
2. Keep the craft window's icon for enchanting recipes
Enchanting has no recipe categories, and the code path that derives the icon assumed there was one, so the craft window lost its icon.
Applicability
Both are in the non-retail craft path, so they apply to every Classic flavour. The companion UI changes are in the
Altoholic_CataPR.Related PRs
This is one change set spread over the repos it touches. Account sharing needs the first two together; the others are independent of each other.