Adding Skills/Professions

Hello,
I am currently trying to understand the process to add skills. I have been digging around in the code for the past few nights trying different things and wanted to clear up some assumptions that i had about the overall process for adding skills/professions. I believe the flow is supposed to work like this.

[ol][li]CMSG_TRAINER_BUY_SPELL Is sent to the server[/li]
[li]Trainer TeachSpell is called[/li]
[li]Player Learn Spell is called[/li]
[li]Player Add Spell is called[/li]
I believe Skills/Professions are considered Ranked Skills ( Apprentice, Journeyman, Expert, Artisan, etc)

	[/ol]

[li]Should Call Player SetSkill[/li]	
[li]Should Call Player _SaveSkills and save them to the database[/li]	

Are those assumptions correct so far? I am looking to fix the issue of learned professions not saving to the database. Based on my debugging, I think I am on the right path. Just looking for some more info from people that have worked in this code base for a while. I think I can solve this problem with a few more hours of digging.

Branch: Master
Sha: 9bb85965e10924554b087eb5ae94e6e4fa970ae9
Client Version: 8.1.5.30706

Thank you,
Chaz

You are already wrong at step 3. Profession spells are supposed to be inserted into trainer_spell table with spell like this https://www.wowhead.com/spell=264591 which has the LEARN_SPELL effect (not 264590)

This means the code goes into CastSpell case, not LearnSpell. This is a very important distinction because that spell has additional effects on top of simply teaching the spell

Ok, so the trainer will cast the spell on the player which then allows the player to learn the skill. Is that correct?

Ok, so after following that path suggested, it looks like the issue is that the skill’s state is not being set to SKILL_NEW, which is preventing the record from being inserted into the database in the _SetSkills function. Still need to do some further testing to verify that with some other skills. I kept buying the mining skill and removing it ( also had to delete the cache folder)

Looks like its failing at the line in SetSkill where it checks to see if the user has the skill. When adding a new skill the uState is always SKILL_CHANGED and is not at the end of the mSkillStatus list. I am not really sure how mSkillStatus is supposed to work yet. Is the intent of the mSkillStatus to only contain a list of skills that the user currently has with a uState of UnChanged or is the intent to contain all the skills with a status of deleted when not being used? I would think its only supposed to be a list of skills the user currently has.

I am guessing the issue is with the find function and that not returning the end of the list.

Does that sound about right? Any suggestions?

The intent is that player has all possible skills inside mSkillStatus, including ones he doesn’t know (this is important for client visuals)

So, I was able to get the skills added and removed. However, I noticed that the starter skills are not being learned correctly. I am guessing its a database issue that needs to be corrected. What is the best way to go about finding the spells. I was looking at the spell_dbc table for the spells 2657 ( smelt Copper on wowhead) and 2656 ( Mining Skills on wowhead) but didnt see them. Also looked for them on the character spells table but didn’t see them either. Do they just need to be added to a table? I am guessing they are already there some where. Just not sure where to look.

do you have solution fix that bug ?