Merge pull request #79 from Valorith/codex/task-delivery-handin-fix

[Bug Fix] Fix task delivery hand-ins on quest NPCs

CMakeLists.txt

@@ -71,7 +71,11 @@ if(UNIX)
 	endif()
 endif()
 
-find_package(Boost REQUIRED COMPONENTS dynamic_bitset foreach tuple)
+find_package(Boost REQUIRED)
+set(EQEMU_BOOST_HEADER_TARGET Boost::headers)
+if(NOT TARGET Boost::headers)
+	set(EQEMU_BOOST_HEADER_TARGET Boost::boost)
+endif()
 find_package(cereal CONFIG REQUIRED)
 find_package(fmt CONFIG REQUIRED)
 find_package(glm CONFIG REQUIRED)

common/item_instance.cpp

@@ -148,6 +148,7 @@ EQ::ItemInstance::ItemInstance(const ItemInstance& copy)
 
 	m_exp       = copy.m_exp;
 	m_evolveLvl = copy.m_evolveLvl;
+	m_task_delivered_count = copy.m_task_delivered_count;
 
 	if (copy.m_scaledItem) {
 		m_scaledItem = new ItemData(*copy.m_scaledItem);

libs/luabind/CMakeLists.txt

@@ -23,7 +23,7 @@ set(lb_sources
 add_library(luabind ${lb_sources})
 
 target_include_directories(luabind PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} ${LUAJIT_INCLUDE_DIR})
-target_link_libraries(luabind PUBLIC Boost::dynamic_bitset Boost::tuple Boost::foreach ${LUAJIT_LIBRARY})
+target_link_libraries(luabind PUBLIC ${EQEMU_BOOST_HEADER_TARGET} ${LUAJIT_LIBRARY})
 target_include_directories(luabind PRIVATE ..)
 
 if(UNIX)

zone/CMakeLists.txt

@@ -685,7 +685,7 @@ add_definitions(-DZONE)
 
 if(EQEMU_BUILD_LUA)
     target_compile_definitions(lua_zone PUBLIC LUA_EQEMU)
-    target_link_libraries(lua_zone PUBLIC luabind Boost::dynamic_bitset Boost::tuple Boost::foreach common)
+    target_link_libraries(lua_zone PUBLIC luabind ${EQEMU_BOOST_HEADER_TARGET} common)
     if (EQEMU_BUILD_STATIC AND LUA_LIBRARY)
         target_link_libraries(zone PRIVATE ${LUA_LIBRARY})
     endif()

zone/cli/tests/cli_npc_handins.cpp

@@ -20,6 +20,7 @@ void RunTest(const std::string& test_name, int expected, int actual);
 struct HandinEntry {
 	std::string            item_id            = "0";
 	uint32                 count              = 0;
+	int                    task_delivered_count = 0;
 	const EQ::ItemInstance *item              = nullptr;
 	bool                   is_multiquest_item = false; // state
 };
@@ -407,6 +408,32 @@ void ZoneCLI::TestNpcHandins(int argc, char **argv, argh::parser &cmd, std::stri
 				},
 				.handin_check_result = true,
 			},
+			TestCase{
+				.description = "Test task-delivered non-stack item is not returned",
+				.hand_in = {
+					.items = {
+						HandinEntry{.item_id = "1001", .count = 1, .task_delivered_count = 1},
+					},
+				},
+				.required = {},
+				.returned = {},
+				.handin_check_result = false,
+			},
+			TestCase{
+				.description = "Test task-delivered stack only returns undelivered charges",
+				.hand_in = {
+					.items = {
+						HandinEntry{.item_id = "13005", .count = 20, .task_delivered_count = 10},
+					},
+				},
+				.required = {},
+				.returned = {
+					.items = {
+						HandinEntry{.item_id = "13005", .count = 10},
+					},
+				},
+				.handin_check_result = false,
+			},
 			TestCase{
 				.description = "Test handing in Soulfire that has 5 charges and have it count as 1 item",
 				.hand_in = {
@@ -455,6 +482,8 @@ void ZoneCLI::TestNpcHandins(int argc, char **argv, argh::parser &cmd, std::stri
 					inst->SetCharges(inst->GetItem()->MaxCharges);
 				}
 
+				inst->SetTaskDeliveredCount(hand_in.task_delivered_count);
+
 				hand_ins[hand_in.item_id] = inst->GetCharges();
 				items.push_back(inst);
 			}

zone/npc.cpp

@@ -4630,7 +4630,7 @@ NPC::Handin NPC::ReturnHandinItems(Client *c)
 		}
 	}
 
-	auto returned = m_hand_in;
+	auto returned = Handin{};
 
 	// check if any money was handed in
 	if (m_hand_in.original_money.platinum > 0 ||
@@ -4669,26 +4669,61 @@ NPC::Handin NPC::ReturnHandinItems(Client *c)
 			m_hand_in.items.end(),
 			[&](HandinEntry &i) {
 				if (i.item && i.item->GetItem() && !i.is_multiquest_item && !returned_items_already) {
+					const uint32 task_delivered_count = std::min<uint32>(
+						i.count,
+						std::max(i.item->GetTaskDeliveredCount(), 0)
+					);
+					const uint32 return_count = i.count - task_delivered_count;
+
+					if (task_delivered_count > 0) {
+						LogNpcHandin(
+							"Task delivery consumed item [{}] ({}) count [{}], remaining return count [{}]",
+							i.item->GetItem()->Name,
+							i.item_id,
+							task_delivered_count,
+							return_count
+						);
+					}
+
+					if (return_count == 0) {
+						return true; // Task delivery already consumed this hand-in item
+					}
+
+					const uint16 charges_to_return = i.item->IsStackable() ?
+						static_cast<uint16>(return_count) :
+						std::max(static_cast<uint16>(i.item->GetCharges()), static_cast<uint16>(1));
+
 					return_items.emplace_back(
 						PlayerEvent::HandinEntry{
 							.item_id = i.item->GetID(),
 							.item_name = i.item->GetItem()->Name,
 							.augment_ids = i.item->GetAugmentIDs(),
 							.augment_names = i.item->GetAugmentNames(),
-							.charges = std::max(static_cast<uint16>(i.item->GetCharges()), static_cast<uint16>(1))
+							.charges = charges_to_return
 						}
 					);
 
-					// If the item is stackable and the new charges don't match the original count
+					returned.items.emplace_back(
-					// set the charges to the original count
+						HandinEntry{
-					if (i.item->IsStackable() && i.item->GetCharges() != i.count) {
+							.item_id = i.item_id,
-						i.item->SetCharges(i.count);
+							.count = return_count,
+							.item = i.item,
+							.is_multiquest_item = i.is_multiquest_item
+						}
+					);
+
+					// Return only the remaining portion that was not consumed by either
+					// quest hand-ins or task delivery updates.
+					if (i.item->IsStackable() && i.item->GetCharges() != charges_to_return) {
+						i.item->SetCharges(charges_to_return);
 					}
+					i.item->SetTaskDeliveredCount(0);
 
 					c->PushItemOnCursor(*i.item, true);
 					LogNpcHandin(
-						"Hand-in failed, returning item [{}] i.is_multiquest_item [{}]",
+						"Hand-in failed, returning item [{}] count [{}] i.is_multiquest_item [{}]",
 						i.item->GetItem()->Name,
+						return_count,
 						i.is_multiquest_item
 					);
 
@@ -4737,6 +4772,7 @@ 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;
+		returned.money        = m_hand_in.money;
 
 		// if multi-quest and we returned money, reset the hand-in bucket
 		if (IsMultiQuestEnabled()) {

zone/object.cpp

@@ -679,9 +679,8 @@ bool Object::HandleClick(Client* sender, const ClickObject_Struct* click_object)
 				}
 			}
 
-			// Transfer item to client
+			// Transfer item to client using the normal cursor update path.
-			sender->PutItemInInventory(EQ::invslot::slotCursor, *m_inst, false);
+			sender->PutItemInInventory(EQ::invslot::slotCursor, *m_inst, true);
-			sender->SendItemPacket(EQ::invslot::slotCursor, m_inst, ItemPacketTrade);
 
 			sender->CheckItemDiscoverability(m_inst->GetID());
 

zone/zone.cpp

@@ -354,6 +354,16 @@ bool Zone::LoadGroundSpawns() {
 			const uint32 max_allowed = g.spawn[slot_id].max_allowed;
 
 			if (inst) {
+				const auto* item = inst->GetItem();
+				if (
+					item &&
+					!inst->IsStackable() &&
+					inst->GetCharges() == 0 &&
+					item->MaxCharges > 0
+				) {
+					inst->SetCharges(item->MaxCharges);
+				}
+
 				for (uint32 i = 0; i < max_allowed; i++) {
 					auto object = new Object(
 						inst,