Fix/fixes 11/30 #647
Fix/fixes 11/30 #647
Conversation
RyanYappert
force-pushed
the
fix/fixes-11-30
branch
from
1010e4b to
a6bb65b
Compare
| Logger.Error(client, $"Unable to fetch the quest state manager for ${questScheduleId}"); | ||
| continue; | ||
| } | ||
|
|
||
| var questState = questStateManager.GetQuestState(questScheduleId); |
There was a problem hiding this comment.
Should the function that gets the quest state do all these checks instead? Looking at the fixes, it looks like we do the same steps in multiple handlers.
There was a problem hiding this comment.
At some point you need to pass the check back to the handler; that means either throwing an error or returning a null, and I've spoken before about how much I dislike silently returning null to indicate a failure state. Should we throw a MissingQuestException or something and then catch it in the handler?
EDIT: I guess there's also the option of a wrapped object like how the PartyManager does it, but that honestly just seems like an awkward way of avoiding a legitimate try/catch.
| @@ -38,6 +38,7 @@ public override void Handle(GameClient client, IPacket packet) | |||
| foreach (var questScheduleId in activeQuests) | |||
| { | |||
| var quest = QuestManager.GetQuestByScheduleId(questScheduleId); | |||
| if (quest is null) { continue; } | |||
There was a problem hiding this comment.
if (quest is null)
{
continue;
}| @@ -8,8 +8,11 @@ | |||
| "minimum_item_rank": 0, | |||
| "discoverable": true, | |||
| "order_conditions": [ | |||
| {"type": "SoloWithPawns"}, | |||
| {"type": "MainQuestCompleted", "Param1": 4} | |||
| { "type": "SoloWithPawns" }, | |||
There was a problem hiding this comment.
Can you format this file back to the way it was?
…ill doesn't work it at least appears in the log.
…plicate recipeId is present.
… people have reported.
…ResponseErrorExceptions to not work properly.
…ntryBoardItemEntryHandler to use a PacketQueue. Fix QuestStateManager::GetProcessState to not invent bad quests.
RyanYappert
force-pushed
the
fix/fixes-11-30
branch
from
61faead to
b42d06a
Compare
Another stack of general tweaks and fixes.
Checklist:
developbranch