[Quest API] Send delivered task items in trade events (#2518)

This restores sending items to EVENT_TRADE that are updated by source
controlled delivery tasks which was removed in 7cf96ca2d8.

That patch filtered out items consumed by task updates to fix a few bugs
with items being returned despite incrementing a task:

  - If an npc without a quest trade event handler was the target of a
    delivery task for a NoDrop/non-Quest item, the npc would auto return
    it due to the `ReturnNonQuestNoDropItems` rule.

  - If an npc without a quest trade event handler was the target of a
    delivery task for a non-NoDrop item, the item would be added to the
    npc's loot.

  - If an npc with an EVENT_ITEM/EVENT_TRADE quest handler used the Lua
    or Perl trade plugins, the plugins would return task items unless
    specific checks for the turned in slots existed.

The quest plugin item returns are problematic for this since they just
summon to return items not handled by the script

  e.g. For a task to deliver N Large Fruit Bat Wings (item id 19616),
  if a player turned in 1 Wing in slot 1 and a stack of 20 Wings in slot
  2, the task would be incremented 21 times and the following Lua trade
  handler would return the stack of 20 from the 2nd trade slot:

  ```lua
    function event_trade(e)
      local item_lib = require("items")
      if item_lib.check_turn_in(e.trade, { item1 = 19616 }) then
        eq.debug("Lua consumed 1 slot and will return other slots")
      end
      item_lib.return_items(e.self, e.other, e.trade)
    end
  ```

  This also occured with the perl plugin though slightly differently
  since that plugin returns all slots unless the exact handin slot count
  matches (requiring check_handin conditions for all slots):

  ```perl
    sub EVENT_ITEM {
      if (plugin::check_handin(\%itemcount, 19616 => 1)) {
        # No issue if only one slot used for trade (item not returned)
      }
      # Perl fails handin check if multiple slots not checked and returns all
      plugin::return_items(\%itemcount);
    }
  ```

While that patch solved the issue, it's inconvenient and wrong to not
receive items in trade events used in a source task update. It breaks
existing trade scripts for tasks that aren't quest controlled and it
forces tasks to be changed to quest controlled and manually updated to
script any extra behavior.

This patch stores the task update count on the item instance before
dispatching it to quests. The burden is now on quests and plugins to
use that value in order to prevent returning items consumed by tasks.

`ItemInstance::RemoveTaskDeliveredItems` has been added to simplify
handling this in plugins which is also used for non-quest item returns.

common/item_instance.cpp

@@ -1721,6 +1721,18 @@ int EQ::ItemInstance::GetItemHaste(bool augments) const
 	return total;
 }
 
+int EQ::ItemInstance::RemoveTaskDeliveredItems()
+{
+	int count = IsStackable() ? GetCharges() : 1;
+	count -= GetTaskDeliveredCount();
+	if (IsStackable())
+	{
+		SetCharges(count);
+	}
+	SetTaskDeliveredCount(0);
+	return count;
+}
+
 //
 // class EvolveInfo
 //

common/item_instance.h

@@ -229,6 +229,13 @@ namespace EQ
 		void StopTimer(std::string name);
 		void ClearTimers();
 
+		int GetTaskDeliveredCount() const { return m_task_delivered_count; }
+		void SetTaskDeliveredCount(int count) { m_task_delivered_count = count; }
+		// This function should only be used by trade return apis
+		// Removes delivered task items from stack count and returns remaining count
+		// Return value should be used to determine if an item still exists (for stackable and non-stackable)
+		int RemoveTaskDeliveredItems();
+
 		// Get a total of a stat, including augs
 		// These functions should be used in place of other code manually totaling
 		// to centralize where it is done to make future changes easier (ex. whenever powersources come around)
@@ -313,6 +320,7 @@ namespace EQ
 		uint32				m_new_id_file;
 		uint32				m_ornament_hero_model;
 		uint32				m_recast_timestamp;
+		int                 m_task_delivered_count = 0;
 
 		//
 		// Items inside of this item (augs or contents);

zone/embparser.cpp

@@ -809,6 +809,22 @@ void PerlembParser::ExportVar(const char *pkgprefix, const char *varname, const
 	}
 }
 
+
+void PerlembParser::ExportVar(const char* pkgprefix, const char* varname, const char* classname, void* value)
+{
+	if (!perl) {
+		return;
+	}
+
+	// todo: try/catch shouldn't be necessary here (called perl apis don't throw)
+	try {
+		perl->setptr(std::string(pkgprefix).append("::").append(varname).c_str(), classname, value);
+	}
+	catch (std::string e) {
+		AddError(fmt::format("Error exporting Perl variable [{}]", e));
+	}
+}
+
 int PerlembParser::SendCommands(
 	const char *pkgprefix,
 	const char *event,
@@ -1350,6 +1366,10 @@ void PerlembParser::ExportEventVariables(
 						temp_var_name = var_name;
 						temp_var_name += "_attuned";
 						ExportVar(package_name.c_str(), temp_var_name.c_str(), inst->IsAttuned());
+
+						temp_var_name = var_name;
+						temp_var_name += "_inst";
+						ExportVar(package_name.c_str(), temp_var_name.c_str(), "QuestItem", inst);
 					}
 					else {
 						ExportVar(package_name.c_str(), var_name.c_str(), 0);
@@ -1361,6 +1381,10 @@ void PerlembParser::ExportEventVariables(
 						temp_var_name = var_name;
 						temp_var_name += "_attuned";
 						ExportVar(package_name.c_str(), temp_var_name.c_str(), 0);
+
+						temp_var_name = var_name;
+						temp_var_name += "_inst";
+						ExportVar(package_name.c_str(), temp_var_name.c_str(), 0);
 					}
 				}
 			}

zone/embparser.h

@@ -88,6 +88,7 @@ private:
 	void ExportVar(const char *pkgprefix, const char *varname, int32 value);
 	void ExportVar(const char *pkgprefix, const char *varname, uint32 value);
 	void ExportVar(const char *pkgprefix, const char *varname, float value);
+	void ExportVar(const char* pkgprefix, const char* varname, const char* classname, void* value);
 	void ExportVarComplex(const char *pkgprefix, const char *varname, const char *value);
 
 	int EventCommon(QuestEventID event, uint32 objid, const char * data, NPC* npcmob, EQ::ItemInstance* item_inst, const SPDat_Spell_Struct* spell, Mob* mob, 

zone/embperl.h

@@ -129,6 +129,11 @@ public:
 		SV *t = get_sv(varname, true);
 		sv_setpv(t, val);
 	}
+	// put a pointer into a blessed perl variable
+	void setptr(const char* varname, const char* classname, void* val) const {
+		SV* t = get_sv(varname, GV_ADD);
+		sv_setref_pv(t, classname, val);
+	}
 
 	// put key-value pairs in hash
 	void sethash(const char *varname, std::map<std::string,std::string> &vals)

zone/lua_iteminst.cpp

@@ -274,6 +274,16 @@ int Lua_ItemInst::CountAugmentByID(uint32 item_id) {
 	return self->CountAugmentByID(item_id);
 }
 
+int Lua_ItemInst::GetTaskDeliveredCount() {
+	Lua_Safe_Call_Int();
+	return self->GetTaskDeliveredCount();
+}
+
+int Lua_ItemInst::RemoveTaskDeliveredItems() {
+	Lua_Safe_Call_Int();
+	return self->RemoveTaskDeliveredItems();
+}
+
 luabind::scope lua_register_iteminst() {
 	return luabind::class_<Lua_ItemInst>("ItemInst")
 	.def(luabind::constructor<>())
@@ -303,6 +313,7 @@ luabind::scope lua_register_iteminst() {
 	.def("GetKillsNeeded", (uint32(Lua_ItemInst::*)(int))&Lua_ItemInst::GetKillsNeeded)
 	.def("GetMaxEvolveLvl", (int(Lua_ItemInst::*)(void))&Lua_ItemInst::GetMaxEvolveLvl)
 	.def("GetPrice", (uint32(Lua_ItemInst::*)(void))&Lua_ItemInst::GetPrice)
+	.def("GetTaskDeliveredCount", &Lua_ItemInst::GetTaskDeliveredCount)
 	.def("GetTotalItemCount", (int(Lua_ItemInst::*)(void))&Lua_ItemInst::GetTotalItemCount)
 	.def("GetUnscaledItem", (Lua_ItemInst(Lua_ItemInst::*)(int))&Lua_ItemInst::GetUnscaledItem)
 	.def("IsAmmo", (bool(Lua_ItemInst::*)(void))&Lua_ItemInst::IsAmmo)
@@ -315,6 +326,7 @@ luabind::scope lua_register_iteminst() {
 	.def("IsStackable", (bool(Lua_ItemInst::*)(void))&Lua_ItemInst::IsStackable)
 	.def("IsType", (bool(Lua_ItemInst::*)(int))&Lua_ItemInst::IsType)
 	.def("IsWeapon", (bool(Lua_ItemInst::*)(void))&Lua_ItemInst::IsWeapon)
+	.def("RemoveTaskDeliveredItems", &Lua_ItemInst::RemoveTaskDeliveredItems)
 	.def("SetCharges", (void(Lua_ItemInst::*)(int))&Lua_ItemInst::SetCharges)
 	.def("SetColor", (void(Lua_ItemInst::*)(uint32))&Lua_ItemInst::SetColor)
 	.def("SetCustomData", (void(Lua_ItemInst::*)(std::string,bool))&Lua_ItemInst::SetCustomData)

zone/lua_iteminst.h

@@ -82,6 +82,8 @@ public:
 	void ClearTimers();
 	bool ContainsAugmentByID(uint32 item_id);
 	int CountAugmentByID(uint32 item_id);
+	int GetTaskDeliveredCount();
+	int RemoveTaskDeliveredItems();
 
 private:
 	bool cloned_;

zone/perl_questitem.cpp

@@ -64,6 +64,26 @@ int Perl_QuestItem_CountAugmentByID(EQ::ItemInstance* self, uint32_t item_id) //
 	return self->CountAugmentByID(item_id);
 }
 
+bool Perl_QuestItem_IsStackable(EQ::ItemInstance* self)
+{
+	return self->IsStackable();
+}
+
+void Perl_QuestItem_SetCharges(EQ::ItemInstance* self, int16_t charges)
+{
+	self->SetCharges(charges);
+}
+
+int Perl_QuestItem_GetTaskDeliveredCount(EQ::ItemInstance* self)
+{
+	return self->GetTaskDeliveredCount();
+}
+
+int Perl_QuestItem_RemoveTaskDeliveredItems(EQ::ItemInstance* self)
+{
+	return self->RemoveTaskDeliveredItems();
+}
+
 void perl_register_questitem()
 {
 	perl::interpreter perl(PERL_GET_THX);
@@ -75,10 +95,14 @@ void perl_register_questitem()
 	package.add("GetCharges", &Perl_QuestItem_GetCharges);
 	package.add("GetID", &Perl_QuestItem_GetID);
 	package.add("GetName", &Perl_QuestItem_GetName);
+	package.add("GetTaskDeliveredCount", &Perl_QuestItem_GetTaskDeliveredCount);
 	package.add("IsAttuned", &Perl_QuestItem_IsAttuned);
+	package.add("IsStackable", &Perl_QuestItem_IsStackable);
 	package.add("IsType", &Perl_QuestItem_IsType);
 	package.add("ItemSay", (void(*)(EQ::ItemInstance*, const char*))&Perl_QuestItem_ItemSay);
 	package.add("ItemSay", (void(*)(EQ::ItemInstance*, const char*, int))&Perl_QuestItem_ItemSay);
+	package.add("RemoveTaskDeliveredItems", &Perl_QuestItem_RemoveTaskDeliveredItems);
+	package.add("SetCharges", &Perl_QuestItem_SetCharges);
 	package.add("SetScale", &Perl_QuestItem_SetScale);
 }
 

zone/task_client_state.cpp

@@ -755,12 +755,7 @@ bool ClientTaskState::UpdateTasksOnDeliver(Client* client, std::vector<EQ::ItemI
 		int updated_count = UpdateTasks(client, filter, count);
 		if (updated_count > 0)
 		{
-			// remove items used in updates
-			item->SetCharges(count - updated_count);
-			if (count == updated_count)
-			{
-				item = nullptr; // all items in trade slot consumed
-			}
+			item->SetTaskDeliveredCount(updated_count);
 			is_updated = true;
 		}
 	}

zone/trading.cpp

@@ -903,11 +903,20 @@ void Client::FinishTrade(Mob* tradingWith, bool finalizer, void* event_entry, st
 		// todo: rule or npc field to auto return normal items also
 		if (!quest_npc)
 		{
-			for (const EQ::ItemInstance* inst : items) {
+			for (EQ::ItemInstance* inst : items) {
 				if (!inst || !inst->GetItem()) {
 					continue;
 				}
 
+				// remove delivered task items
+				if (RuleB(TaskSystem, EnableTaskSystem) && inst->GetTaskDeliveredCount() > 0) {
+					int remaining = inst->RemoveTaskDeliveredItems();
+					if (remaining <= 0) {
+						inst = nullptr;
+						continue; // all items in trade slot consumed by task update
+					}
+				}
+
 				const EQ::ItemData* item = inst->GetItem();
 
 				bool isPetAndCanHaveNoDrop = (RuleB(Pets, CanTakeNoDrop) &&
@@ -981,7 +990,6 @@ void Client::FinishTrade(Mob* tradingWith, bool finalizer, void* event_entry, st
 			tradingWith->FaceTarget(this);
 		}
 
-		// quest items are filtered by task deliver updates
 		std::vector<std::any> item_list(items.begin(), items.end());
 		parse->EventNPC(EVENT_TRADE, tradingWith->CastToNPC(), this, "", 0, &item_list);