🐛 [Bug] Profession quests are able to be completed multiple times when abandoning and retraining a trade skill

FlagFlayer commented 2 years ago

🐛 Bug report

If you turn in any profession quest, then abandon the profession that is required to pick up the quest, then retrain that profession to the required skill to pick up the quest, you will be able to pick up the quest again. While this may be intended behaviour for quests for profession specialisation (e.g. Tribal/Dragonscale/Elemental Leatherworking, Armorsmith/Weaponsmith Blacksmithing, etc.), this is not the intended behaviour for other profession quests. After extensive testing I can confirm that no profession quest should ever be repeatable, including profession specialisation, which is all handled with book "Soothsaying for Dummies". The outcome of that is that it is impossible to switch specialisation as a blacksmith, but that is just the blizz-like implementation.

Wild Leather Armor quests (which have dialogue that allows to relearn the recipes should they be missing) are implemented here: https://github.com/vmangos/core/pull/1680.

Expected behavior

Profession quests that are completed do not automatically become available again when abandoning and retraining the relevant profession

Examples from Classic:

[768] Gathering Leather: https://youtu.be/tXpA-r7msg4?t=60 (German localisation: Leder sammeln -> Gathering Leather, Kurschnerei -> Skinning)
[769] Kodo Hide Bag: https://youtu.be/SeGHEiQCrLU?t=70 (German localisation: Kodobalgtasche -> Kodo Hide Bag, Lederverarbeitung -> Leatherworking)

Steps to reproduce

For example, the simplest quest to complete (Gathering Leather):

.go creature Mooranta
Train skinning
.additem [Light Leather] 24
Pick up and turn in Gathering Leather from Veren Tallstrider
Abandon skinning
Retrain skinning
The quest Gathering Leather will now be available to pick up again
You can now pick up the quest again and complete it again (Specific behaviour: When picking up the quest you would not be able to turn it in. Upon relog the quest will be abandoned but it will still be available to be picked up. After a relog the quest is available to pick up and turn in with no issue)
...
Profit

Version & Environment

Client Version: 1.12.1.5875

OS Client: Win 10 x64 OS Server: Win 10 x64

Crashlog

None

FlagFlayer commented 2 years ago

Bug was previously encountered at https://github.com/vmangos/core/issues/1639

FlagFlayer commented 2 years ago

Current behvaiour implies that when a profession is abandoned all quest completion data relating to that profession gets wiped. While the data for some quests should be wiped (e.g. the cases I mentioned above), it's definitely not correct for other quests (as tests on Classic have shown).

FlagFlayer commented 2 years ago

Testing with .lookup quest certainly implies much the same: 20221101TEST

FlagFlayer commented 2 years ago

This behaviour is seems accurate for vanilla too, not just classic. It also applies for quests that require more than just Apprentice developmental skill:

By Gimlina on 2005/11/01 (Patch 1.8.1)
Subject: "Arg, what the heck do I do now..."I have a problem. I had Blacksmithing and completed this quest already. After a patch, I had lost Blacksmithing (at the time I thought I screwed up somehow). I have since gone back and now can't get these plans. According to a GM the are other ways to get them (ither than the quest, that is). Does anyone know how to get these plans without completing the quest? Any help would be greatly appreciated.

Thanks!

FlagFlayer commented 2 years ago

Possible solution to the edge case I presented: Jangdor Swiftrider (for horde) and Pratt McGrubben (for alliance) are implied have a dialogue option that allows to learn the recipes once the relevant quests are completed. Needs testing in Classic.

By Lisse on 2005/08/25 (Patch 1.6.1)
Subject: "Bug/Exploit"There's a slight bug/exploit with this quest.

I completed it, received the Pattern: Wild Leather Helmet but couldn't learn it as my leatherworking was only 215 at the time.

I then spoke to the Quest giver again, and there was a new selection: "Jangdor, teach me again how to make the Wild Leather Helmet". I clicked on it, and he taught me the pattern and it appears in my leatherworking pattern list!

And I still have the unused pattern which I'm guessing I can auction off - it's not soulbound or anything.

EDIT: Confirmed this behaviour https://www.youtube.com/watch?v=NRkVuzSY7VQ&t=150s Quests are also behave differently than current vmangos behvaior. I'll implement all of this in my profession quest-related PR.

FlagFlayer commented 2 years ago

Okay there are no edge cases for this: https://www.wowhead.com/classic/object=177226/book-soothsaying-for-dummies

I need to check if this is properly implemented in vmangos. If it is then just making all profession quests not repeatable and that'd correct vmangos behaviour.

EDIT: Clearly can't be implemented until profession quests are correctly only available to be completed once. The quest completion is likely the flag the book uses to allow you to regain past specialisations, but as long as quest data gets completely wiped when you abandon and retrain a profession this can't be implemented or tested.

FlagFlayer commented 2 years ago

For the case with Blacksmithing: https://old.reddit.com/r/classicwow/comments/bh6k0f/changing_primary_profession_specializations/elqmoiw/

ZanathKariashi 4 points 3 years ago

Blacksmithing can never drop their specialization in Vanilla. Even getting rid of the profession and reacquiring doesn't work. (BSing didn't get the ability to do so until 2.1.3 in TBC, though by then every prof spec could be dropped simply by talking to your spec's trainer and paying 100 gold).

It does work for LWing and Engineering though. (added in 1.10).

FlagFlayer commented 1 year ago

~~I honestly have no idea when it comes to src so what I'm saying might be completely wrong, but I believe this might be related to this chunk of code in Player.cpp:~~

            for (auto& itr : mQuestStatus)
            { 
                if (Quest const* quest = sObjectMgr.GetQuestTemplate(itr.first))
                {
                    if (quest->GetRequiredSkill() == id)
                    {
                        // remove all quest entries for 'entry' from quest log
                        for (uint8 slot = 0; slot < MAX_QUEST_LOG_SIZE; ++slot)
                        {
                            if (GetQuestSlotQuestId(slot) == itr.first)
                            {
                                SetQuestSlot(slot, 0);
                                TakeOrReplaceQuestStartItems(itr.first, false, false);
                                break;
                            }
                        }

                        // set quest status to not started (will updated in DB at next save)
                        SetQuestStatus(itr.first, QUEST_STATUS_NONE); // Does not invalidate the iterator
                        itr.second.uState = QUEST_DELETED;
                    }
                }
            }
        }
    }

Lines 5987-6011. I believe the intention here is to abandon all quests in the quest log and set their status to QUEST_STATUS_NONE. But it accidentally does this to all profession quests of the abandoned profession resulting in all of them being redoable when retaking the profession.

Much later edit: I don't have the C++ knowledge to know what the issue actually is. Help would be appreciated.

vmangos / core