problem with warriors skillid 26

[gtestdev]

Description:

Warriors can get a infinite rollback. I don't know how to reproduce it, but for some reason appear this in log:

2022-02-07_08:41:21 SQL(p): INSERT INTO character_skills (guid, skill, value, max) VALUES (1636, 26, 425, 425)
 [ERROR]: [1062] Duplicate entry '1636-26' for key 'PRIMARY'
2022-02-07_08:41:21 Transaction aborted. 121 queries not executed.
2022-02-07_08:44:21 SQL(p): INSERT INTO character_skills (guid, skill, value, max) VALUES (1636, 26, 425, 425)
 [ERROR]: [1062] Duplicate entry '1636-26' for key 'PRIMARY'
2022-02-07_08:44:21 Transaction aborted. 120 queries not executed.

and in DB appear:

  guid   skill   value     max  
------  ------  ------  --------
  1636      26     0       0

Seems when in Player::SetSkill:

            // update skill state
            if (itr->second.uState == SKILL_UNCHANGED)
            {
                if (currVal == 0)   // activated skill, mark as new to save into database
                    itr->second.uState = SKILL_NEW;
                else                // updated skill, mark as changed to save into database
                    itr->second.uState = SKILL_CHANGED;
            }

set to skill_new and do a insert and duplicated error problem appear, but I can't figure out how to reproduce it.

Steps to reproduce the problem:

I don't know

TC rev. hash/commit:

TrinityCore rev. https://github.com/The-Cataclysm-Preservation-Project/TrinityCore/commit/83456ed8533d59da5907558964a05b8cf09f1052 2022-01-14 21:40:27 +0100 (master branch) (Unix, RelWithDebInfo, Static)

[Ovahlord]

Obviously you had a crash somewhen between because that's skill is a warrior's "arms" specialization

[gtestdev]

It has happened on multiple occasions with different characters, all warriors, I have not perceived anything related to a crash. it seems that for some unknown reason, the core forces setskill 26 0 0 and once that happens they have constant rollback due to the transaction aborted caused by duplicate entry in insert into character_skill

[Ovahlord]

any news on how to reproduce it?

[gtestdev]

I can't find a way to reproduce it, seems to happen when the player logs out after doing some specific thing (unknown) I have seen in the code some SetSkill(spellLearnSkill->skill, 0, 0, 0); in Player::RemoveSpell, but I can't figure out how it can happen with skillid 26, It's the only one skillid that happens.

if you want to reproduce the rollback just get in DB any offline warrior and update in the character_skills table, value and max set to 0 for skillid 26 and log in with that warrior and do something and logout, you always rollback.

[Ovahlord]

But how on earth do you even change the skill? That skill is only being learned on character creation and then never again touched. There is no way that this skill ever gets re-inserted

[gtestdev]

as I said at the beginning of the issue, Player::SetSkill when currVal == 0 do itr->second.uState = SKILL_NEW; so when execute Player::_SaveSkills do CHAR_INS_CHAR_SKILLS due to case SKILL_NEW. so re-insert happens because currVal == 0. and as I mentioned a moment ago, I can't figure out how it can happen that setskill 0 0 to skillid 26, but when that unkown situation happens, player will be stuck in a continuous rollback forever unless in the DB is set to a value greater than 0 in skillid 26

[Ovahlord]

then add an assert when skill 26 is being changed and show me the crashlog so I can track down where it comes from.

[Ovahlord]

So did you do what I was asking for? Still need to know how to reproduce it

[AGandrup]

I just started experience this issue as well, but am also unsure how to reproduce it atm. Up untill last night, I have only had one single character on my server which has been a warrior. Last night I created another character, also warrior, and now close to 24 hours later I am randomly having this issue as well. I haven't had any crashes at all. I am trying to think back at what exactly I have been doing, and mostly it has just been swapping between different specs and attacking training dummys.

So far the issue is only with the first character I created, and the SQL error also only happens upon logging out of that character.

[AGandrup]

Alright, I think I found whats causing the issue and also how to reproduce it. It has to do with dual wield, oddly enough. But first, let me explain how to reproduce it:

1) Create a fresh Warrior. 2) Set your level to 10 or above for talent point(s). 3) Pick any talent in fury and hit learn. As part of the Fury spec, you will learn Dual Wield. 4) Relog. Notice the error in the worldserver.exe console saying: "Player has forbidden skill 118 for his race/class combination". 5) Once you are logged in again, unlearn your talents (or swap to a different spec if you got dual spec). Now, if you relog again, and then navigate into the character_skills table inside your characters database (might have to refresh it), you can see that skill id 26 now has its value set to 0 as well as its max value. These two values must be at least 1, though they are meant to be set to character level * 5, otherwise that character will be caught in a loop of rollbacks. 6) Logging out of that character will now cause the SQL error mentioned above, to appear in the console.

So its actually due to Dual Wield being a forbidden skill for warriors, and if you open SkillRaceClassInfo.dbc and set a filter on Field01 to be equal to 118 (which is the skill id for Dual Wield), you can look at Field03 and see that the class id for warrior is missing, which is 1. Shaman, Rogue, Hunter and Death Knights are the only ones that are there.

[AGandrup]

I am not sure what the preferred way is for fixing an issue like this but I suppose a check in the Player::_LoadSkills method, where it throws the error, could be made. Because looking at the code:

SkillRaceClassInfoEntry const* rcEntry = sDBCManager.GetSkillRaceClassInfo(skill, getRace(), getClass());
            if (!rcEntry)
            {
                TC_LOG_ERROR("entities.player", "Player::_LoadSkills: Player '%s' (%s, Race: %u, Class: %u) has forbidden skill %u for his race/class combination",
                    GetName().c_str(), GetGUID().ToString().c_str(), uint32(getRace()), uint32(getClass()), skill);

                mSkillStatus.insert(SkillStatusMap::value_type(skill, SkillStatusData(0, SKILL_DELETED)));
                continue;
            }

I suspect that it is this line mSkillStatus.insert(SkillStatusMap::value_type(skill, SkillStatusData(0, SKILL_DELETED))); which for some reason messes with skill 26, though I am not sure atm.

[Ovahlord]

Got a rough idea where this comes from. Pretty sure it's because the forbidden skill is just slapped into position 0 which is the same position where skill 26 goes into so they both are messing with each other. Gonna store the forbidden skill at a proper position this time and see what's gonna happen