Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Quests] Cleanup Quest Parser Logic #4025

Merged
merged 11 commits into from Feb 6, 2024
Merged

Conversation

Kinglykrab
Copy link
Contributor

Notes

  • Consolidate duplicated code into loops to make it easier to read and easier to add on to.

Images

NPC EVENT_SAY Example

image

Global Player EVENT_SAY Example

image

Global Player EVENT_TARGET_CHANGE Example

image

Zone Player EVENT_EXP_GAIN Example

image

# Notes
- Consolidate duplicated code into loops to make it easier to read and easier to add on to.

# Images
@Kinglykrab Kinglykrab requested review from Akkadius and hgtw and removed request for Akkadius January 27, 2024 17:16
@Akkadius
Copy link
Member

Akkadius commented Feb 1, 2024

This ones gonna require some extra care and more detailed review

zone/embparser.cpp Outdated Show resolved Hide resolved
zone/embparser.cpp Outdated Show resolved Hide resolved
zone/embparser.cpp Outdated Show resolved Hide resolved
zone/embparser.cpp Outdated Show resolved Hide resolved
zone/embparser.cpp Outdated Show resolved Hide resolved
zone/embparser.cpp Outdated Show resolved Hide resolved
@Kinglykrab Kinglykrab merged commit 331e04f into master Feb 6, 2024
2 checks passed
@Kinglykrab Kinglykrab deleted the quests/cleanup_quest_parser branch February 6, 2024 00:53
@Akkadius Akkadius mentioned this pull request Feb 10, 2024
hgtw added a commit to hgtw/Server that referenced this pull request Apr 1, 2024
If a mob has a target when a zone is shutdown it will crash while trying
to dispatch EVENT_TARGET_CHANGE when the Mob destructor cleans up hatelists
if a quest interface isn't loaded for the type (in this case no bot scripts):

 	zone.exe!fmt::v10::format<std::string const &,char const *>(fmt::v10::basic_format_string<char,std::string const &,char const *> fmt={...}, const std::string & <args_0>={...}, const char * && <args_1>=0x0000000000000000) Line 2835	C++
>	zone.exe!QuestParserCollection::GetQIByBotQuest(std::string & filename={...}) Line 1138	C++
 	zone.exe!QuestParserCollection::BotHasQuestSubLocal(QuestEventID event_id=EVENT_TARGET_CHANGE) Line 353	C++
 	zone.exe!QuestParserCollection::BotHasQuestSub(QuestEventID event_id=EVENT_TARGET_CHANGE) Line 389	C++
 	zone.exe!Mob::SetTarget(Mob * mob=0x0000000000000000) Line 5431	C++
 	zone.exe!NPC::SetTarget(Mob * mob=0x0000000000000000) Line 575	C++
 	zone.exe!Mob::RemoveFromHateList(Mob * mob=0x000001bfbdc66040) Line 4894	C++
 	zone.exe!EntityList::RemoveFromTargets(Mob * mob=0x000001bfbdc66040, bool RemoveFromXTargets=true) Line 1530	C++
 	zone.exe!Mob::~Mob() Line 547	C++
 	zone.exe!NPC::~NPC() Line 537	C++
 	zone.exe!NPC::`scalar deleting destructor'(unsigned int)	C++
 	zone.exe!EntityList::RemoveAllMobs() Line 2678	C++
 	zone.exe!EntityList::Clear() Line 3090	C++
 	zone.exe!Zone::~Zone() Line 1103	C++
 	zone.exe!Zone::`scalar deleting destructor'(unsigned int)	C++
 	zone.exe!Zone::Shutdown(bool quiet=false) Line 928	C++

This is caused by the Zone destructor deleting short_name before calling
entity_list.Clear(). With an unloaded quest interface BotHasQuestSubLocal
calls GetQIByBotQuest which gets a null zone->GetShortName() and crashes
while formatting strings.

The immediate regressing commit for this crash is because a check for
zone->IsLoaded() was removed in 74f1eac with others that were removed
to fix a regression by EQEmu#4025. GetQIByBotQuest and GetQIByPlayerQuest
always had this check and should have remained for them.

This restores the zone->IsLoaded() checks for GetQIByBotQuest/PlayerQuest.
The other functions cannot have that check added until the other issues
mentioned in EQEmu#4149 are addressed.
Akkadius pushed a commit that referenced this pull request Apr 1, 2024
If a mob has a target when a zone is shutdown it will crash while trying
to dispatch EVENT_TARGET_CHANGE when the Mob destructor cleans up hatelists
if a quest interface isn't loaded for the type (in this case no bot scripts):

 	zone.exe!fmt::v10::format<std::string const &,char const *>(fmt::v10::basic_format_string<char,std::string const &,char const *> fmt={...}, const std::string & <args_0>={...}, const char * && <args_1>=0x0000000000000000) Line 2835	C++
>	zone.exe!QuestParserCollection::GetQIByBotQuest(std::string & filename={...}) Line 1138	C++
 	zone.exe!QuestParserCollection::BotHasQuestSubLocal(QuestEventID event_id=EVENT_TARGET_CHANGE) Line 353	C++
 	zone.exe!QuestParserCollection::BotHasQuestSub(QuestEventID event_id=EVENT_TARGET_CHANGE) Line 389	C++
 	zone.exe!Mob::SetTarget(Mob * mob=0x0000000000000000) Line 5431	C++
 	zone.exe!NPC::SetTarget(Mob * mob=0x0000000000000000) Line 575	C++
 	zone.exe!Mob::RemoveFromHateList(Mob * mob=0x000001bfbdc66040) Line 4894	C++
 	zone.exe!EntityList::RemoveFromTargets(Mob * mob=0x000001bfbdc66040, bool RemoveFromXTargets=true) Line 1530	C++
 	zone.exe!Mob::~Mob() Line 547	C++
 	zone.exe!NPC::~NPC() Line 537	C++
 	zone.exe!NPC::`scalar deleting destructor'(unsigned int)	C++
 	zone.exe!EntityList::RemoveAllMobs() Line 2678	C++
 	zone.exe!EntityList::Clear() Line 3090	C++
 	zone.exe!Zone::~Zone() Line 1103	C++
 	zone.exe!Zone::`scalar deleting destructor'(unsigned int)	C++
 	zone.exe!Zone::Shutdown(bool quiet=false) Line 928	C++

This is caused by the Zone destructor deleting short_name before calling
entity_list.Clear(). With an unloaded quest interface BotHasQuestSubLocal
calls GetQIByBotQuest which gets a null zone->GetShortName() and crashes
while formatting strings.

The immediate regressing commit for this crash is because a check for
zone->IsLoaded() was removed in 74f1eac with others that were removed
to fix a regression by #4025. GetQIByBotQuest and GetQIByPlayerQuest
always had this check and should have remained for them.

This restores the zone->IsLoaded() checks for GetQIByBotQuest/PlayerQuest.
The other functions cannot have that check added until the other issues
mentioned in #4149 are addressed.
MortimerGreenwald pushed a commit to MortimerGreenwald/Server that referenced this pull request Apr 5, 2024
If a mob has a target when a zone is shutdown it will crash while trying
to dispatch EVENT_TARGET_CHANGE when the Mob destructor cleans up hatelists
if a quest interface isn't loaded for the type (in this case no bot scripts):

 	zone.exe!fmt::v10::format<std::string const &,char const *>(fmt::v10::basic_format_string<char,std::string const &,char const *> fmt={...}, const std::string & <args_0>={...}, const char * && <args_1>=0x0000000000000000) Line 2835	C++
>	zone.exe!QuestParserCollection::GetQIByBotQuest(std::string & filename={...}) Line 1138	C++
 	zone.exe!QuestParserCollection::BotHasQuestSubLocal(QuestEventID event_id=EVENT_TARGET_CHANGE) Line 353	C++
 	zone.exe!QuestParserCollection::BotHasQuestSub(QuestEventID event_id=EVENT_TARGET_CHANGE) Line 389	C++
 	zone.exe!Mob::SetTarget(Mob * mob=0x0000000000000000) Line 5431	C++
 	zone.exe!NPC::SetTarget(Mob * mob=0x0000000000000000) Line 575	C++
 	zone.exe!Mob::RemoveFromHateList(Mob * mob=0x000001bfbdc66040) Line 4894	C++
 	zone.exe!EntityList::RemoveFromTargets(Mob * mob=0x000001bfbdc66040, bool RemoveFromXTargets=true) Line 1530	C++
 	zone.exe!Mob::~Mob() Line 547	C++
 	zone.exe!NPC::~NPC() Line 537	C++
 	zone.exe!NPC::`scalar deleting destructor'(unsigned int)	C++
 	zone.exe!EntityList::RemoveAllMobs() Line 2678	C++
 	zone.exe!EntityList::Clear() Line 3090	C++
 	zone.exe!Zone::~Zone() Line 1103	C++
 	zone.exe!Zone::`scalar deleting destructor'(unsigned int)	C++
 	zone.exe!Zone::Shutdown(bool quiet=false) Line 928	C++

This is caused by the Zone destructor deleting short_name before calling
entity_list.Clear(). With an unloaded quest interface BotHasQuestSubLocal
calls GetQIByBotQuest which gets a null zone->GetShortName() and crashes
while formatting strings.

The immediate regressing commit for this crash is because a check for
zone->IsLoaded() was removed in 74f1eac with others that were removed
to fix a regression by EQEmu#4025. GetQIByBotQuest and GetQIByPlayerQuest
always had this check and should have remained for them.

This restores the zone->IsLoaded() checks for GetQIByBotQuest/PlayerQuest.
The other functions cannot have that check added until the other issues
mentioned in EQEmu#4149 are addressed.
MortimerGreenwald pushed a commit to MortimerGreenwald/Server that referenced this pull request Apr 15, 2024
If a mob has a target when a zone is shutdown it will crash while trying
to dispatch EVENT_TARGET_CHANGE when the Mob destructor cleans up hatelists
if a quest interface isn't loaded for the type (in this case no bot scripts):

 	zone.exe!fmt::v10::format<std::string const &,char const *>(fmt::v10::basic_format_string<char,std::string const &,char const *> fmt={...}, const std::string & <args_0>={...}, const char * && <args_1>=0x0000000000000000) Line 2835	C++
>	zone.exe!QuestParserCollection::GetQIByBotQuest(std::string & filename={...}) Line 1138	C++
 	zone.exe!QuestParserCollection::BotHasQuestSubLocal(QuestEventID event_id=EVENT_TARGET_CHANGE) Line 353	C++
 	zone.exe!QuestParserCollection::BotHasQuestSub(QuestEventID event_id=EVENT_TARGET_CHANGE) Line 389	C++
 	zone.exe!Mob::SetTarget(Mob * mob=0x0000000000000000) Line 5431	C++
 	zone.exe!NPC::SetTarget(Mob * mob=0x0000000000000000) Line 575	C++
 	zone.exe!Mob::RemoveFromHateList(Mob * mob=0x000001bfbdc66040) Line 4894	C++
 	zone.exe!EntityList::RemoveFromTargets(Mob * mob=0x000001bfbdc66040, bool RemoveFromXTargets=true) Line 1530	C++
 	zone.exe!Mob::~Mob() Line 547	C++
 	zone.exe!NPC::~NPC() Line 537	C++
 	zone.exe!NPC::`scalar deleting destructor'(unsigned int)	C++
 	zone.exe!EntityList::RemoveAllMobs() Line 2678	C++
 	zone.exe!EntityList::Clear() Line 3090	C++
 	zone.exe!Zone::~Zone() Line 1103	C++
 	zone.exe!Zone::`scalar deleting destructor'(unsigned int)	C++
 	zone.exe!Zone::Shutdown(bool quiet=false) Line 928	C++

This is caused by the Zone destructor deleting short_name before calling
entity_list.Clear(). With an unloaded quest interface BotHasQuestSubLocal
calls GetQIByBotQuest which gets a null zone->GetShortName() and crashes
while formatting strings.

The immediate regressing commit for this crash is because a check for
zone->IsLoaded() was removed in 74f1eac with others that were removed
to fix a regression by EQEmu#4025. GetQIByBotQuest and GetQIByPlayerQuest
always had this check and should have remained for them.

This restores the zone->IsLoaded() checks for GetQIByBotQuest/PlayerQuest.
The other functions cannot have that check added until the other issues
mentioned in EQEmu#4149 are addressed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants