Questie

Questie

122M Downloads

Double quest tooltip with extraObjective

BreakBB opened this issue ยท 16 comments

commented

Bug description

Screenshots

grafik

Questie & WoW version

v8.4.2

commented

To clarify... one tooltip is for the Quest and the other is for the Objective being complete? Who wrote that? :D

commented

It's the bug where the extraobjectives are not cleared when quest is marked complete. #3466

commented

But neither of these quests have extraObjectives...

image

commented

image
image

commented

Doh... forgot to check corrections. Okie... I get it. I think I have a simple fix for that.

commented

Simple, difficult, complicated, just give a fix <3

commented

or all the things? (anything that can add quest tooltip info)

commented

only questie? or dugi too?

commented

1 tooltip is for the objective of the quest (talk to keristrasza) and the 2nd is the tooltip that shows you you can turn the quest in at that NPC. It's only Questie 100%

commented

I think I figured out this bug... it might very well be a race condition.

We don't destroy or recycle our tootlip lookup tables we use to build tooltips until a quest is HANDED in.

Quest Accepted

When you first get this quest, it fires off:

QuestieTooltips:RegisterObjectiveTooltip() - which inserts tooltip data into these tables:
QuestieTooltips.lookupByKey[key][objective.Index] <--- we're pushing raw strings here (GetToolTip(key) pulls these lines later
QuestieTooltips.lookupKeysByQuestId

You have to talk to Keristrasza to get ported. This builds the NPC tooltip... or whenever a player mouseover the NPC.

...it fires QuestieTooltips:GetTooltip() which returns its own set of tooltip lines (from a local table "tooltipLines") it builds based on what is in QuestieTooltips.lookupKeysByQuestId and QuestieTooltips.lookupByKey. Since there is no NPC ID in QuestieTooltips.lookupByKey it inserts the Quest Title and the "Talk to Keristrasza" objective.

The tooltip will look like this:

[71] Saragosa's End (11957)
Talk to Keristrasza 

Quest Complete

Saragosa is dead...

The tooltip SHOULD update to this:

[71] Saragosa's End (11957) (Complete)
Talk to Keristrasza 

...Keristrasza turns into a Quest Finisher and fires off:
QuestieTooltips:RegisterQuestStartTooltip() which inserts tooltip data into both tables:
QuestieTooltips.lookupByKey[key][npc.name] <--- we're pushing raw strings here (GetToolTip(key) pulls these lines)
QuestieTooltips.lookupKeysByQuestId

This time when you mouseover Keristrasza it fires QuestieTooltips:GetTooltip() again but THIS time there is an NPC ID present it inserts just the Quest Title string into the "tooltipLines" table.

The tooltip SHOULD update to this:

[71] Saragosa's End (11957) (Complete)

Quest Turned In

Clears QuestieTooltips.lookupKeysByQuestId[questID] and QuestieTooltips.lookupByKey["m_" .. Id] tables.

At least this is how it's supposed to work and based on the code, shouldn't produce any duplicates because the local tables inside GetTooltip() ARE destroyed each time you run this function and _QuestieTooltips:AddUnitDataToTooltip() has a time throttle in it to prevent being called more than once if the hookscripts fire a lot.

What's the bug?

I think both the Quest Status Update & Quest Finisher work flows are firing at the same time and two sets of strings are getting inserted via _QuestieTooltips:AddUnitDataToTooltip() which just blindly adds tooltip lines without checking to see what is in the tooltip first. I.E. The "tooltipData" table that is passed from GetTooltip() isn't checked against the current tooltip for duplicate lines. It only passes valid data.

The tooltip turns into this:

[71] Saragosa's End (11957) (Complete)        <-- Tooltip line given to Quest Finishers
[71] Saragosa's End (11957) (Complete)        <-- Correct Tooltip Line
Talk to Keristrasza                                   <-- Correct Tooltip Line

Bug Fix

It is also my belief that this can happen with Objects too.

           -- Certain race conditions can occur when the NPC/Objects are both the Quest Starter and Quest Finisher
           -- which can result in duplicate Quest Title tooltips appearing. DrawAvailableQuest() would have already
           -- registered this NPC/Object so, the appropiate tooltip lines are already present. If an NPC/Object
           -- isn't startedBy and finishedBy the same ID or the entry is not already present, then create it.
           
           if quest.startedBy ~= quest.finishedBy or (not QuestieTooltips.lookupByKey[key]) then
               QuestieTooltips:RegisterQuestStartTooltip(questId, finisher)
           end
commented

After a nights sleep... I re-read what I wrote above and corrected a few things. I don't normally post my notes but didn't want to wake up and wrap my head around all this again this morning.

The bug fix for this is to insert...

            if quest.startedBy ~= quest.finishedBy then
               QuestieTooltips:RegisterQuestStartTooltip(questId, finisher)
           end

...into QuestieQuest:AddFinisher(). No reason to re-register the tooltip if the tooltip date for this NPC is already present. I've verified that it doesn't affect map notes. We're just preventing another tooltip registration and it still works fine for all the other Quests I checked. This should prevent the above race condition.

commented

Macro? Tell me more...

commented

...I created a macro to repro the bugged tool tip on the NPC to make sure that running the above function did in fact clear the tooltip and rebuild it properly.

commented

OK, so my first attempt at fixing this bug only fixed it for Quest Starters who are also Quest Finishers. I finally experienced this bug yesterday on a Quest Finisher who was a Quest Starter on a different quest in my log. The following logic should cover all use cases. The nuts and bolts of the logic is... if an NPC is registered, clear out all the lines and let QuestieTooltips:RegisterQuestStartTooltip(questId, finisher) run again to refresh the entire tooltip for this NPC.

        if (finisher ~= nil and finisher.spawns ~= nil) then
            -- Certain race conditions can occur when the NPC/Objects are both the Quest Starter and Quest Finisher
            -- which can result in duplicate Quest Title tooltips appearing. DrawAvailableQuest() would have already
            -- registered this NPC/Object so, the appropiate tooltip lines are already present. If an NPC/Object
            -- isn't startedBy and finishedBy the same ID or the entry is not already present, then create it.

            -- Clear the key if it exsists
            if QuestieTooltips.lookupByKey[key] then
                QuestieTooltips.lookupByKey[key] = {}
            end

            QuestieTooltips:RegisterQuestStartTooltip(questId, finisher)```
commented

FYI: The changed code is happening in QuestieQuest:AddFinisher(). The duplicate text is added when the Quest transitions from incomplete to complete and QuestieQuest:AddFinisher() is supposed to fire fairly soon after. In some race conditions it fires way too late.

So, I guess third times a charm. Sometimes a Quest Finisher can have more than one quest in its tooltip. The previous fixed code would have wiped out the duplicate AND the valid second quest that is supposed to be referenced. So, I went back and rethought out all the logic. Basically, the change is that it checks to see if there is more than one line in the tooltip. Then it crawls the tootip looking for duplicates. If found, it removes the duplicate Quest Title and any Special Objectives text. Then it does what it has always done. It runs QuestieTooltips:RegisterQuestStartTooltip() and since the correct flags are in the tooltip table that GetTooltip uses to build the tooltip, it only registers the Quest Title. Not both the Quest Title and Special Objective.

-- Certain race conditions can occur when the NPC/Objects are both the Quest Starter and Quest Finisher
-- which can result in duplicate Quest Title tooltips appearing. DrawAvailableQuest() would have already
-- registered this NPC/Object so, the appropiate tooltip lines are already present. This checks and clears
-- any duplicate keys before registering the Quest Finisher.

-- Clear duplicate keys if they exsist
if QuestieTooltips.lookupByKey[key] then
    if #QuestieTooltips:GetTooltip(key) > 1 then
        for ttline = 1, #QuestieTooltips:GetTooltip(key) do
            for index, line in pairs(QuestieTooltips:GetTooltip(key)) do
                if (ttline == index) then
                    Questie:Debug(Questie.DEBUG_DEVELOP, "[QuestieQuest] AddFinisher - Removing duplicate Quest Title!")

                    -- Remove duplicate Quest Title
                    QuestieTooltips.lookupByKey[key][tostring(questId) .. " " .. finisher.name] = nil

                    -- Now check to see if the dup has a Special Objective
                    local objText = string.match(line, ".*|cFFcbcbcb.*")

                    if objText then
                        local objIndex

                        -- Grab the Special Objective index
                        if quest.SpecialObjectives[1] then
                            objIndex = quest.SpecialObjectives[1].Index
                        end

                        if objIndex then
                            Questie:Debug(Questie.DEBUG_DEVELOP, "[QuestieQuest] AddFinisher - Removing Special Objective!")

                            -- Remove Special Objective Text
                            QuestieTooltips.lookupByKey[key][tostring(questId) .. " " .. objIndex] = nil
                        end
                    end
                end
            end
        end
    end
end

And as promised... this is the marco I used to "break" the NPC tooltips.

/script tt={} qId=10518 k="m_21158" o=QuestieDB:GetQuest(qId).SpecialObjectives[1] tt.questId=qId tt.objective=o
/script QuestieTooltips.lookupByKey[k][tostring(qId) .. " " .. o.Index]=tt
/script table.insert(QuestieTooltips.lookupKeysByQuestId[qId],k)

This only works for Quests with a Special Objective and once the Quest is in a completed state. Turn on DEBUG with spam level. Hover over the NPC (quest finisher) to get the tooltip ID. Change the value of k in the macro to this text (quotes must be present). Change the value of qId to the Quest ID.

  1. Run Macro - re-hover over NPC - tooltip will be "broken"
  2. Run /dump QuestieQuest:AddFinisher(QuestieDB:GetQuest(10518)) <--- make sure qId and this quest id match.

Hover over NPC. Tooltip should be in a correct state. For NPC's with more than one valid Quest Title in the tooltip... they should both be present. The above code ONLY removes duplicates for the quest table that is passed to it QuestieQuest:AddFinisher(quest) leaving behind any other valid Quest Titles. If you run the macro against the other valid quest listed in the tooltip and if it too has a Special Objective text, that quest will get duplicated. If it doesn't not have a Special Objective or if for some reason the tooltip has duplicate Quest Titles, they will be trimmed by the new code in QuestieQuest:AddFinisher().

commented

I have to reopen this, as it is present again
image