b-morgan / Skillet

World of Warcraft addon
GNU General Public License v3.0
15 stars 6 forks source link

For JS scraping wowhead #20

Closed matthewhively closed 3 years ago

matthewhively commented 3 years ago

What format of data do you want? Same as tbc-classic? It should go into

local skillLevels = {
    [0] = "orange/yellow/green/gray"
}

right?

formatted as: [itemID] = "a/b/c/d", and [itemID] = { [spellID] = "a/b/c/d" },

No segregation needed for "alchemy of outland" vs "alchemy of pandary" vs "shadowlands alchemy" right?

b-morgan commented 3 years ago

Yes, same format as tbc-classic. If the segregation is easy, then it can't hurt. If the segregation is hard, then it probably isn't necessary.

The separation by profession in Skillet-Classic was just comments so I imagine the segregation by expansion would just be comments as well.

matthewhively commented 3 years ago

Seems like it would be a pain segregate by expansion, so I'll just glob them all together by profession unless there is a code requirement that they be separated.

b-morgan commented 3 years ago

Retail SkillLevelData.lua has been copied from Classic and updated (the updates have been copied back to Classic). I should have a Skillet alpha build out later today.

matthewhively commented 3 years ago

Does archeology need skill level data?

b-morgan commented 3 years ago

No. Runeforging and Archeology are not supported by Skillet.

matthewhively commented 3 years ago

Looks like all recipes in retail only have 3 numbers: yellow/green/gray Should I force it to have a 4th number as a copy of the yellow?

CORRECTION: almost everything only has 3 numbers. Some old-world cookie recipes still have 4

matthewhively commented 3 years ago

This may not work the way we want it to. Apparently the live version of wowhead only rarely displays the orange level on the profession table view. It also displays an incorrect training level. If I click through into the actual spell page it will most likely display the orange level and the trainable level (which should be the same). Obviously I've only tried a few examples because its very time consuming to check spells 1-by-1 see: https://www.wowhead.com/alchemy#alchemy;100 look at the 5 earliest items at the bottom of the table. the 3 starter items, have no orange listed even in the spell page. Troll's blood and minor mana do list an orange level when on the spell page, but there is no way to view that from the table.

matthewhively commented 3 years ago

I submitted a feedback request asking wowhead to fix this... but I doubt they will.

matthewhively commented 3 years ago

I have some javascript that will read everything in the retail wowhead professions tables (or at least I squashed all the bugs I know of), but I don't have much reason to continue testing it unless we can accurately extract orange skill levels for most recipes :(

b-morgan commented 3 years ago

When a recipe is orange isn't really that important. What a user wants to know is when is it going to turn yellow, turn green, and turn gray. Can you tell when there is an orange value and when there isn't? If so, then when there isn't just set it to 1.

I rearranged and added debugs to the updated SkillLevelData.lua so that I could verify that TradeSkillInfo (I had to change the .toc) and LibPeriodicTable were getting results. They try both positive and negative itemID.

I probably should add a debugging option to gather all three sources and compare them (with debug output).

matthewhively commented 3 years ago

There does not appear to be a way to determine if there is an orange level (even if hidden) on the table view. But I can prepend "1" to all recipes that do not contain 4 numbers. If that is acceptable that should be very easy.

b-morgan commented 3 years ago

That probably works.

matthewhively commented 3 years ago

Hmmmm What should be done for these? https://www.wowhead.com/spell=310497/damage-retaliator

They seem work like enchantments, but are engineering. Should they be negative?

matthewhively commented 3 years ago

Similar for these: https://www.wowhead.com/spell=292320/blood-contract-bloodguard https://www.wowhead.com/spell=124561/draconic-leg-reinforcements https://www.wowhead.com/spell=279183/discreet-spellthread

b-morgan commented 3 years ago

My guess is that they are recipes that produce items so they should have a positive itemID (of the item produced).

I'm OK with 99+% correctness. There will be a few entries that are wrong and they can be fixed as needed if/when an issue is reported. The important thing is we will have the ability to make the fixes.

I don't understand how Wowhead processes the data that they collect. I just do my part in collecting it.

matthewhively commented 3 years ago

None of those items have an associated itemID, at least not that I can find. I can just skip them if necessary, but what does skillet do for them currently?

b-morgan commented 3 years ago

You can skip them (adding a comment would be icing on the cake). Its coming back to me... The ones that apply to your inventory act like enchants so there's never an "item". The inscription ones (Blood Contract: x) have items. If you search Wowhead for "Blood Contract:" you'll see both the spell and the item. Wowhead may not link them together like it should.

I'll look and see if any of my toons know any of those recipes. Skillet will function without entries it just won't know when they change color (but it will know what color they are at the moment because that information does come from the Blizzard API).

matthewhively commented 3 years ago

@b-morgan New PR is up. I have done zero testing in retail client, so I hope it just works without additional tweaks.

b-morgan commented 3 years ago

Skillet-4.34-alpha2 contains this code.