Objectives of failed quests are redrawn on reload
BreakBB opened this issue · 8 comments
@BreakBB did you want me to make the code changes I outlined above? Or did you have something else in mind?
@BreakBB opps... I was playing with this yesterday and left in some modified code in by local build and it was checked in with this commit:
Soooo, technically this is fixed? But this would only handle failed quests upon login or reloadUI. It would fix your bug but it doesn't really address the underlying logic. I really don't think it would hurt to expand the scope and just run QuestieQuest:UpdateQuest(questId) for each questId iterated by QuestieQuest:GetAllQuestIds(). Unless there is some logic bomb I'm not considering here...
Oh, so the bug I reported was only on your PR not on master? 🤔
Then I guess you can close this issue and simply create a new one for the other bug/TODO.
There is a comment in the code from the person who originally wrote this module inside the QuestieQuest:GetAllQuestIds()
function:
--This prevents us from adding Failed Quests to the tracker - we want the ability to track all quests.
--elseif data.isComplete ~= -1 then -- TODO FIX LATER. Now currentQuestLog may have part of failed quests. Check what is needed? All or none of those?
While in game when a quest is updated it runs QuestieQuest:UpdateQuest(questId)
. This function has had its logic significantly improved over the last several months. This is why in game quest updates are perfectly fine.
When you either login or do a reloadUI it scans the players quest log and does a very rudimentary update by running ONLY QuestieQuest:GetAllQuestIds()
. This has so far worked out quite well for the most part, however I knew it would only be a matter of time before someone else saw this bug too. I've had my eye on it for quite some time and have been meaning to circle back around to it. Not entirely sure what the code comment is referring to specifically, but we'd want to update and show ALL quests... even failed ones.
I can add a quick check to GetAllQuestIds() to run UpdateQuest() for failed questIDs only... but I'm starting to think that maybe we should just run UpdateQuest() for each questID that GetAllQuestIds() iterates through since the logic is now pretty solid.
By the way... this isn't an actual tooltip issue. It's more or less a "Quest State" issue. When you first login it does the basic stuff. When a quest is in a failed state, there are some additional "clean up" routines that need to fire to bring the quest state back to a more or less "Available" state...
@BreakBB No... this is a bug that has been around for a long time. The chances of someone noticing it is slim since most players who fail a quest just go and pick it back up and ignore the state of the icons. Doing a reloadUI or logging out after failing a quest probably doesn't happen much... the tracker reports the state of the quest as being failed and that is generally the only thing a player pays attention to. Once the quest is picked backup, everything is corrected.
My comment above was just simply to say that I fixed the bug in my local build and was waiting for your feedback. Then I went on and fixed another bug that was unrelated to yours and then checked in the fix for your bug accidently along with the other fix. Make sense?
If you're good with it, then I'll leave it alone. If not, then I'll revert the change.