Double quest tooltip with extraObjective
BreakBB opened this issue ยท 16 comments
To clarify... one tooltip is for the Quest and the other is for the Objective being complete? Who wrote that? :D
It's the bug where the extraobjectives are not cleared when quest is marked complete. #3466
Doh... forgot to check corrections. Okie... I get it. I think I have a simple fix for that.
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%
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
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.
...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.
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)```
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.
- Run Macro - re-hover over NPC - tooltip will be "broken"
- 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()
.