QuestieQuest.lua's tooltip registration logic seems wasteful CPU-wise
Arcitec opened this issue ยท 3 comments
_RegisterObjectiveTooltips = function(objective, questId)
Questie:Debug(DEBUG_INFO, "Registering objective tooltips for " .. objective.Description)
if objective.spawnList then
for id, spawnData in pairs(objective.spawnList) do
if spawnData.TooltipKey and (not objective.AlreadySpawned[id]) and (not objective.hasRegisteredTooltips) then
QuestieTooltips:RegisterObjectiveTooltip(questId, spawnData.TooltipKey, objective)
end
end
else
Questie:Error("[QuestieQuest]: [Tooltips] ".. l10n("There was an error populating objectives for %s %s %s %s", objective.Description or "No objective text", questId or "No quest id", 0 or "No objective", "No error"));
end
objective.hasRegisteredTooltips = true
end
Step by step, what it does:
- Loop through the objective's list of spawns.
- For every spawn, check if it has a designated tooltip key, and if
not objective.hasRegisteredTooltips
. - If so (key exists, and the objective has not been processed yet), register that sub-objective-key (spawn key) in the QuestieTooltips.lookupByKey table.
- Finally, set
objective.hasRegisteredTooltips = true
Here is what it probably should be doing instead:
- Check
if not objective.hasRegisteredTooltips
then do the work, else do nothing at all.
To clarify: Right now, the code is looping through a bunch of data and sub-tables, and taking a code path that contains an if-statement which ensures that the final code inside the loop never runs if objective.hasRegisteredTooltips = true
. So I don't see any reason whatsoever to keep the old code in this function. There's no reason to begin a loop at all, if that value is already true.
I totally agree, this looks absolutely wasteful.
The objective code of Questie in particular is in pretty bad shape and fragile as well. That is one of the main reasons we are very careful touching it and didn't start the required full rewrite already. There were other way more important topics as well towards the release of TBC.
But simply moveing the check a layer up looks quite easy to achieve. Would you mind creating a PR for this and test the change? ๐๐ป
@BreakBB I apologize for not getting involved after reporting a bunch of Questie issues. I quit WoW around the same time.
Everything came from me reverse engineering Questie to make a Plater nameplate addon that replaces Questie's nameplate code:
I hope the issue reports are still a form of contribution even though I didn't have time to rewrite anything for Questie...