[NPC Handins] Fix MultiQuest Handins (#4651)

* [NPC Handins] Fix multi-quest handins

* Update linux-build.sh

common/eqemu_logsys.cpp

@@ -601,6 +601,8 @@ void EQEmuLogSys::SilenceConsoleLogging()
 		log_settings[log_index].is_category_enabled = 0;
 	}
 
+	log_settings[Logs::MySQLError].log_to_console = static_cast<uint8>(Logs::MySQLError);
+	log_settings[Logs::Error].log_to_console = static_cast<uint8>(Logs::Error);
 	log_settings[Logs::Crash].log_to_console = static_cast<uint8>(Logs::General);
 }
 

utils/scripts/build/linux-build.sh

@@ -55,6 +55,7 @@ echo "# Running shared_memory"
 
 echo "# Running NPC hand-in tests"
 ./bin/zone tests:npc-handins 2>&1 | tee test_output.log
+./bin/zone tests:npc-handins-multiquest 2>&1 | tee -a test_output.log
 
 if grep -E -q "QueryErr|Error" test_output.log; then
     echo "Error found in test output! Failing build."

zone/cli/npc_handins.cpp

@@ -22,6 +22,8 @@ void ZoneCLI::NpcHandins(int argc, char **argv, argh::parser &cmd, std::string &
 	LogInfo("Booting test zone for NPC handins");
 	LogInfo("{}", Strings::Repeat("-", break_length));
 
+	LogSys.SilenceConsoleLogging();
+
 	Zone::Bootup(ZoneID("qrg"), 0, false);
 	zone->StopShutdownTimer();
 
@@ -404,6 +406,8 @@ void ZoneCLI::NpcHandins(int argc, char **argv, argh::parser &cmd, std::string &
 		std::map<std::string, uint32>         required;
 		std::vector<EQ::ItemInstance *> items;
 
+		LogSys.EnableConsoleLogging();
+
 		// turn this on to see debugging output
 		LogSys.log_settings[Logs::NpcHandin].log_to_console = std::getenv("DEBUG") ? 3 : 0;
 

zone/cli/npc_handins_multiquest.cpp

@@ -0,0 +1,219 @@
+#include "../../common/http/httplib.h"
+#include "../../common/eqemu_logsys.h"
+#include "../../common/platform.h"
+#include "../zone.h"
+#include "../client.h"
+#include "../../common/net/eqstream.h"
+
+extern Zone *zone;
+
+void ZoneCLI::NpcHandinsMultiQuest(int argc, char **argv, argh::parser &cmd, std::string &description)
+{
+	if (cmd[{"-h", "--help"}]) {
+		return;
+	}
+
+	uint32 break_length = 50;
+	int    failed_count = 0;
+
+	RegisterExecutablePlatform(EQEmuExePlatform::ExePlatformZoneSidecar);
+
+	LogInfo("{}", Strings::Repeat("-", break_length));
+	LogInfo("Booting test zone for NPC handins (MultiQuest)");
+	LogInfo("{}", Strings::Repeat("-", break_length));
+
+	LogSys.SilenceConsoleLogging();
+
+	Zone::Bootup(ZoneID("qrg"), 0, false);
+	zone->StopShutdownTimer();
+
+	entity_list.Process();
+	entity_list.MobProcess();
+
+	LogInfo("{}", Strings::Repeat("-", break_length));
+	LogInfo("> Done booting test zone");
+	LogInfo("{}", Strings::Repeat("-", break_length));
+
+	Client *c       = new Client();
+	auto   npc_type = content_db.LoadNPCTypesData(754008);
+	if (npc_type) {
+		auto npc = new NPC(
+			npc_type,
+			nullptr,
+			glm::vec4(0, 0, 0, 0),
+			GravityBehavior::Water
+		);
+
+		entity_list.AddNPC(npc);
+		npc->MultiQuestEnable();
+
+		LogInfo("> Spawned NPC [{}]", npc->GetCleanName());
+		LogInfo("> Spawned client [{}]", c->GetCleanName());
+
+		struct HandinEntry {
+			std::string            item_id            = "0";
+			uint32                 count              = 0;
+			const EQ::ItemInstance *item              = nullptr;
+			bool                   is_multiquest_item = false; // state
+		};
+
+		struct HandinMoney {
+			uint32 platinum = 0;
+			uint32 gold     = 0;
+			uint32 silver   = 0;
+			uint32 copper   = 0;
+		};
+
+		struct Handin {
+			std::vector<HandinEntry> items = {}; // items can be removed from this set as successful handins are made
+			HandinMoney              money = {}; // money can be removed from this set as successful handins are made
+		};
+
+		struct TestCase {
+			std::string description = "";
+			Handin      hand_in;
+			Handin      required;
+			Handin      returned;
+			bool        handin_check_result;
+		};
+
+		std::vector<TestCase> test_cases = {
+			TestCase{
+				.description = "Journeyman's Boots",
+				.hand_in = {
+					.items = {
+						HandinEntry{.item_id = "12268", .count = 1},
+						HandinEntry{.item_id = "7100", .count = 1},
+					},
+					.money = {.platinum = 325},
+				},
+				.required = {
+					.items = {
+						HandinEntry{.item_id = "12268", .count = 1},
+						HandinEntry{.item_id = "7100", .count = 1},
+					},
+					.money = {.platinum = 325},
+				},
+				.returned = {},
+				.handin_check_result = true,
+			},
+		};
+
+		std::map<std::string, uint32>   hand_ins;
+		std::map<std::string, uint32>   required;
+		std::vector<EQ::ItemInstance *> items;
+
+		LogSys.EnableConsoleLogging();
+
+		// turn this on to see debugging output
+		LogSys.log_settings[Logs::NpcHandin].log_to_console = std::getenv("DEBUG") ? 3 : 0;
+
+		LogInfo("{}", Strings::Repeat("-", break_length));
+
+		for (auto &test_case: test_cases) {
+			required.clear();
+
+			for (auto &hand_in: test_case.hand_in.items) {
+				hand_ins.clear();
+				items.clear();
+
+				auto             item_id = Strings::ToInt(hand_in.item_id);
+				EQ::ItemInstance *inst   = database.CreateItem(item_id);
+				if (inst->IsStackable()) {
+					inst->SetCharges(hand_in.count);
+				}
+
+				if (inst->GetItem()->MaxCharges > 0) {
+					inst->SetCharges(inst->GetItem()->MaxCharges);
+				}
+
+				hand_ins[hand_in.item_id] = inst->GetCharges();
+				items.push_back(inst);
+
+				npc->CheckHandin(c, hand_ins, required, items);
+				npc->ResetHandin();
+			}
+
+			// money
+			if (test_case.hand_in.money.platinum > 0) {
+				hand_ins["platinum"] = test_case.hand_in.money.platinum;
+			}
+			if (test_case.hand_in.money.gold > 0) {
+				hand_ins["gold"] = test_case.hand_in.money.gold;
+			}
+			if (test_case.hand_in.money.silver > 0) {
+				hand_ins["silver"] = test_case.hand_in.money.silver;
+			}
+			if (test_case.hand_in.money.copper > 0) {
+				hand_ins["copper"] = test_case.hand_in.money.copper;
+			}
+
+			for (auto &req: test_case.required.items) {
+				required[req.item_id] = req.count;
+			}
+
+			// money
+			if (test_case.required.money.platinum > 0) {
+				required["platinum"] = test_case.required.money.platinum;
+			}
+			if (test_case.required.money.gold > 0) {
+				required["gold"] = test_case.required.money.gold;
+			}
+			if (test_case.required.money.silver > 0) {
+				required["silver"] = test_case.required.money.silver;
+			}
+			if (test_case.required.money.copper > 0) {
+				required["copper"] = test_case.required.money.copper;
+			}
+
+			auto result = npc->CheckHandin(c, hand_ins, required, items);
+			if (result != test_case.handin_check_result) {
+				failed_count++;
+				LogError("FAIL [{}]", test_case.description);
+				// print out the hand-ins
+				LogError("Hand-ins >");
+				for (auto &item: npc->GetHandin().items) {
+					LogError(" > Item [{}] count [{}]", item.item_id, item.count);
+				}
+				LogError("Required >");
+				for (auto &req: required) {
+					LogError(" > Item [{}] count [{}]", req.first, req.second);
+				}
+				LogError("Expected [{}] got [{}]", test_case.handin_check_result, result);
+			}
+			else {
+				LogInfo("PASS [{}]", test_case.description);
+			}
+
+			auto returned = npc->ReturnHandinItems(c);
+
+			// assert that returned items are expected
+			for (auto &item: test_case.returned.items) {
+				auto      found = false;
+				for (auto &ret: returned.items) {
+					if (ret.item_id == item.item_id) {
+						found = true;
+						break;
+					}
+				}
+				if (!found) {
+					LogError("Returned item [{}] not expected", item.item_id);
+				}
+			}
+
+			npc->ResetHandin();
+
+			if (LogSys.log_settings[Logs::NpcHandin].log_to_console > 0) {
+				std::cout << std::endl;
+			}
+		}
+	}
+
+	if (failed_count > 0) {
+		LogError("Failed [{}] tests", failed_count);
+		std::exit(1);
+	}
+	else {
+		LogInfo("All tests passed");
+	}
+}

zone/main.cpp

@@ -133,7 +133,7 @@ int main(int argc, char **argv)
 	set_exception_handler();
 
 	// silence logging if we ran a command
-	if (ZoneCLI::RanConsoleCommand(argc, argv)) {
+	if (ZoneCLI::RanConsoleCommand(argc, argv) || ZoneCLI::RanTestCommand(argc, argv)) {
 		LogSys.SilenceConsoleLogging();
 	}
 
@@ -298,7 +298,7 @@ int main(int argc, char **argv)
 		EQ::InitializeDynamicLookups();
 	}
 
-	// command handler
+	// command handler (no sidecar or test commands)
 	if (ZoneCLI::RanConsoleCommand(argc, argv) && !(ZoneCLI::RanSidecarCommand(argc, argv) || ZoneCLI::RanTestCommand(argc, argv))) {
 		LogSys.EnableConsoleLogging();
 		ZoneCLI::CommandHandler(argc, argv);
@@ -310,6 +310,10 @@ int main(int argc, char **argv)
 		->SetGMSayHandler(&Zone::GMSayHookCallBackProcess)
 		->StartFileLogs();
 
+	if (ZoneCLI::RanTestCommand(argc, argv)) {
+		LogSys.SilenceConsoleLogging();
+	}
+
 	player_event_logs.SetDatabase(&database)->Init();
 
 	skill_caps.SetContentDatabase(&content_db)->LoadSkillCaps();
@@ -491,6 +495,7 @@ int main(int argc, char **argv)
 	// sidecar command handler
 	if (ZoneCLI::RanConsoleCommand(argc, argv)
 		&& (ZoneCLI::RanSidecarCommand(argc, argv) || ZoneCLI::RanTestCommand(argc, argv))) {
+		LogSys.EnableConsoleLogging();
 		ZoneCLI::CommandHandler(argc, argv);
 	}
 

zone/npc.cpp

@@ -4431,6 +4431,17 @@ bool NPC::CheckHandin(
 	// remove items from the hand-in bucket that were used to fulfill the requirement
 	std::vector<HandinEntry> items_to_remove;
 
+	// multi-quest
+	if (IsMultiQuestEnabled()) {
+		for (auto &h_item: h.items) {
+			for (const auto &r_item: r.items) {
+				if (h_item.item_id == r_item.item_id && h_item.count == r_item.count) {
+					h_item.is_multiquest_item = true;
+				}
+			}
+		}
+	}
+
 	// check if the hand-in items fulfill the requirement
 	bool items_met = true;
 	if (!handin_items.empty() && !r.items.empty()) {
@@ -4448,7 +4459,10 @@ bool NPC::CheckHandin(
 
 				if (id_match) {
 					uint32 used_count = std::min(remaining_requirement, h_item.count);
-					h_item.count -= used_count;
+					// If the item is a multi-quest item, we don't want to consume it for the hand-in bucket
+					if (!IsMultiQuestEnabled()) {
+						h_item.count -= used_count;
+					}
 					remaining_requirement -= used_count;
 
 					LogNpcHandinDetail(
@@ -4499,17 +4513,6 @@ bool NPC::CheckHandin(
 
 	requirement_met = money_met && items_met;
 
-	// multi-quest
-	if (IsMultiQuestEnabled()) {
-		for (auto &h_item: h.items) {
-			for (const auto &r_item: r.items) {
-				if (h_item.item_id == r_item.item_id && h_item.count == r_item.count) {
-					h_item.is_multiquest_item = true;
-				}
-			}
-		}
-	}
-
 	// in-case we trigger CheckHand-in multiple times, only set these once
 	if (!m_handin_started) {
 		m_handin_started  = true;
@@ -4690,6 +4693,11 @@ bool NPC::CheckHandin(
 		}
 	}
 
+	// when we meet requirements under multi-quest, we want to reset the hand-in bucket
+	if (requirement_met && IsMultiQuestEnabled()) {
+		ResetMultiQuest();
+	}
+
 	return requirement_met;
 }
 
@@ -4817,6 +4825,12 @@ NPC::Handin NPC::ReturnHandinItems(Client *c)
 		return_money.silver   = m_hand_in.money.silver;
 		return_money.gold     = m_hand_in.money.gold;
 		return_money.platinum = m_hand_in.money.platinum;
+
+		// if multi-quest and we returned money, reset the hand-in bucket
+		if (IsMultiQuestEnabled()) {
+			m_hand_in.money = {};
+			m_hand_in.original_money = {};
+		}
 	}
 
 	if (money_returned_via_external_quest_methods) {
@@ -4872,6 +4886,7 @@ NPC::Handin NPC::ReturnHandinItems(Client *c)
 
 void NPC::ResetHandin()
 {
+	LogNpcHandin("Resetting hand-in bucket for [{}]", GetCleanName());
 	m_has_processed_handin_return = false;
 	m_handin_started              = false;
 	if (!IsMultiQuestEnabled()) {
@@ -4882,3 +4897,12 @@ void NPC::ResetHandin()
 		m_hand_in = {};
 	}
 }
+
+void NPC::ResetMultiQuest() {
+	LogNpcHandin("Resetting multi-quest hand-in bucket for [{}]", GetCleanName());
+	for (auto &i: m_hand_in.original_items) {
+		safe_delete(i.item);
+	}
+
+	m_hand_in = {};
+}

zone/npc.h

@@ -596,6 +596,7 @@ public:
 	);
 	Handin ReturnHandinItems(Client *c);
 	void ResetHandin();
+	void ResetMultiQuest();
 	bool HasProcessedHandinReturn() { return m_has_processed_handin_return; }
 	bool HandinStarted() { return m_handin_started; }
 
@@ -749,6 +750,8 @@ protected:
 	// items can be decremented from this as each successful
 	// check is ran in scripts, the remainder is what is returned
 	Handin m_hand_in = {};
+public:
+	const Handin GetHandin() { return m_hand_in; }
 
 private:
 	uint32              m_loottable_id;

zone/queryserv.cpp

@@ -56,7 +56,6 @@ bool QueryServ::SendPacket(ServerPacket *pack)
 		return true;
 	}
 
-	LogInfo("SendPacket request with QS Server Offline.");
 	return false;
 }
 

zone/zone_cli.cpp

@@ -31,9 +31,11 @@ void ZoneCLI::CommandHandler(int argc, char **argv)
 	// Register commands
 	function_map["sidecar:serve-http"] = &ZoneCLI::SidecarServeHttp;
 	function_map["tests:npc-handins"] = &ZoneCLI::NpcHandins;
+	function_map["tests:npc-handins-multiquest"] = &ZoneCLI::NpcHandinsMultiQuest;
 
 	EQEmuCommand::HandleMenu(function_map, cmd, argc, argv);
 }
 
 #include "cli/sidecar_serve_http.cpp"
 #include "cli/npc_handins.cpp"
+#include "cli/npc_handins_multiquest.cpp"

zone/zone_cli.h

@@ -11,6 +11,7 @@ public:
 	static bool RanSidecarCommand(int argc, char **argv);
 	static bool RanTestCommand(int argc, char **argv);
 	static void NpcHandins(int argc, char **argv, argh::parser &cmd, std::string &description);
+	static void NpcHandinsMultiQuest(int argc, char **argv, argh::parser &cmd, std::string &description);
 };