[Mangos Zero] .learn all_crafts is bugged

[Zilvereyes]

Hi yesterday when i was testing something on my newly build develop21 core and database,

i realized that the .learn all_crafts command is learning you a ton of skills that have nothing to do with crafts.

I have found that the dbc (skillLine.dbc) where the data is pulled from is the reason,

a lot of skills have the SkillLineCategoryId of 9 and 11 that cant be categorized as crafts,

I can gather some data on what ID's and SkillLineCategoryId's are for the crafts, but might need help fixing this.

I'll see what I can figure out my self and post back with what i have.

Skill Name		ID	SkillLineCategoryId
Engineering		101	11
Blacksmithing		164	11
Leatherworking		165	11
Alchemy			171	11
Herbalism		182	11
Mining			186	11
Tailoring		197	11
Enchanting		333	11
Skinning		393	11

First Aid		129	9
Cooking			185	9
Fishing			356	9
Riding

Not sure if riding should be learned here too, give me some feedback.

[Zilvereyes]

Here is my first thought on fixing this, not sure if it'll work, compiling now to test.

Level2.cpp

bool ChatHandler::HandleLearnAllCraftsCommand(char* /*args*/)
{
for (uint32 i = 0; i < sSkillLineStore.GetNumRows(); ++i)
{
	SkillLineEntry const* skillInfo = sSkillLineStore.LookupEntry(i);
	if (!skillInfo)
	{ continue; }

	if (skillInfo->categoryId == SKILL_CATEGORY_PROFESSION || skillInfo->categoryId == SKILL_CATEGORY_SECONDARY)
	{
+			// Learn only the crafts that actually are crafts (MaNGOS ZERO)
+			if (skillInfo->id == SKILL_ENGINEERING || skillInfo->id == SKILL_BLACKSMITHING ||
+				skillInfo->id == SKILL_LEATHERWORKING || skillInfo->id == SKILL_ALCHEMY ||
+				skillInfo->id == SKILL_HERBALISM || skillInfo->id == SKILL_MINING ||
+				skillInfo->id == SKILL_TAILORING || skillInfo->id == SKILL_ENCHANTING ||
+				skillInfo->id == SKILL_SKINNING || skillInfo->id == SKILL_FIRST_AID ||
+				skillInfo->id == SKILL_COOKING || skillInfo->id == SKILL_FISHING)
+			{
+				HandleLearnSkillRecipesHelper(m_session->GetPlayer(), skillInfo->id);
+			}
-			HandleLearnSkillRecipesHelper(m_session->GetPlayer(), skillInfo->id);
	}
}

SendSysMessage(LANG_COMMAND_LEARN_ALL_CRAFT);
return true;
}

If anyone have any ideas or thoughts feel free to reply to this post.

[antz]

that looks like a neat fix, would you be able to create a git pull request for that ?

[Zilvereyes]

If you can supply me with the git commands to do so i can, i'm new to using git.

I created a diff file like the How To Work on bugs (Wiki page) suggested.

[ATTACH]361[/ATTACH]

Edit: Managed to figure out how to do a pull, so have done that :-)

learn_all_crafts.diff

[antz]

Thank you for the PR - it's been merged

After merging it, it occurred to me that reversing the logic could make it simplier and also scale for the other cores.

I.e. rather than checking which ones to include, instead check which ones to exclude.

[Zilvereyes]

Thank you for the PR - it's been merged

After merging it, it occurred to me that reversing the logic could make it simplier and also scale for the other cores.

I.e. rather than checking which ones to include, instead check which ones to exclude.

I do understand your concern, but there is a lot of skill's/spell's that is sorted out not being part of the crafts pool,

making it a long list of stuff to filter out, but i have been thinking of how to implement this in a better way.

Anyway, i'm currently setting up all the MaNGOS servers with the intend of porting this change to them,

and try and find other stuff to help out fixing.

Might take a while setting it all up and such but when i got time irl, I do work on it.

[antz]

Great stuff, Thank you in advance

[Olion]

how to implement this in a better way.

A filtering of the spells to learn is done in the ChatHandler::HandleLearnSkillRecipesHelper() method. The present filter is incomplete, at least due to the following reason.

Some riding skills in vanilla are race- and class-dependent. The dependence is reflected not in SkillLineAbility.dbc, but in SkillRaceClassInfo.dbc, with few lines per skill. The later DBC is read in too, with convenient data structure created in the multimap SpellMgr::mSkillRaceClassInfoMap.

So, the present filter should be augmented (at the end) by checking the data from mSkillRaceClassInfoMap. It also must generalize the code properly for higher emulator flavours up to 6.x client, since the mentioned DBCs are present throughout all clients.