[Commands] Cleanup #unscribespell Command. (#1998)

* [Commands] Cleanup #unscribespell Command.
- Cleanup messages and logic.
- Add Client::UnscribeSpellBySpellID(spell_id, update_client).
- Add $client->UnscribeSpellBySpellID(spell_id, update_client) to Perl.
- Add client:UnscribeSpellBySpellID(spell_id, update_client) to Lua.

* Update unscribespell.cpp

zone/client.h

@@ -818,6 +818,7 @@ public:
 	// defer save used when bulk saving
 	void UnscribeSpell(int slot, bool update_client = true, bool defer_save = false);
 	void UnscribeSpellAll(bool update_client = true);
+	void UnscribeSpellBySpellID(uint16 spell_id, bool update_client = true);
 	void UntrainDisc(int slot, bool update_client = true, bool defer_save = false);
 	void UntrainDiscAll(bool update_client = true);
 	void UntrainDiscBySpellID(uint16 spell_id, bool update_client = true);

zone/command.cpp

@@ -384,7 +384,7 @@ int command_init(void)
 		command_add("unlock", "- Unlock the worldserver", AccountStatus::GMLeadAdmin, command_unlock) ||
 		command_add("unmemspell", "[Spell ID] - Unmemorize a Spell by ID for you or your target", AccountStatus::Guide, command_unmemspell) ||
 		command_add("unmemspells", " - Unmemorize all spells for you or your target", AccountStatus::Guide, command_unmemspells) ||
-		command_add("unscribespell", "[spellid] - Unscribe specified spell from your target's spell book.", AccountStatus::GMCoder, command_unscribespell) ||
+		command_add("unscribespell", "[Spell ID] - Unscribe a spell from your or your target's spell book by Spell ID", AccountStatus::GMCoder, command_unscribespell) ||
 		command_add("unscribespells", "- Clear out your or your player target's spell book.", AccountStatus::GMCoder, command_unscribespells) ||
 		command_add("untraindisc", "[Spell ID] - Untrain your or your target's discipline by Spell ID", AccountStatus::GMCoder, command_untraindisc) ||
 		command_add("untraindiscs", "- Untrains all disciplines from your target.", AccountStatus::GMCoder, command_untraindiscs) ||

zone/gm_commands/unscribespell.cpp

@@ -1,62 +1,68 @@
 #include "../client.h"
+#include "../../common/data_verification.h"
 
 void command_unscribespell(Client *c, const Seperator *sep)
 {
-	uint16 spell_id  = 0;
+	int arguments = sep->argnum;
-	uint16 book_slot = -1;
+	if (!arguments || !sep->IsNumber(1)) {
-	Client *t        = c;
+		c->Message(Chat::White, "Usage: #unscribespell [Spell ID] - Unscribe a spell from your or your target's spell book by Spell ID");
-
-	if (c->GetTarget() && c->GetTarget()->IsClient() && c->GetGM()) {
-		t = c->GetTarget()->CastToClient();
-	}
-
-	if (!sep->arg[1][0]) {
-		c->Message(Chat::White, "FORMAT: #unscribespell <spellid>");
 		return;
 	}
 
-	spell_id = atoi(sep->arg[1]);
+	auto target = c;
+	if (c->GetTarget() && c->GetTarget()->IsClient() && c->GetGM()) {
+		target = c->GetTarget()->CastToClient();
+	}
 
-	if (IsValidSpell(spell_id)) {
+	uint16 spell_id = EQ::Clamp(std::stoi(sep->arg[1]), 0, 65535);
-		book_slot = t->FindSpellBookSlotBySpellID(spell_id);
 
-		if (book_slot >= 0) {
+	if (!IsValidSpell(spell_id)) {
-			t->UnscribeSpell(book_slot);
+		c->Message(
-
+			Chat::White,
-			t->Message(Chat::White, "Unscribing spell: %s (%i) from spellbook.", spells[spell_id].name, spell_id);
+			fmt::format(
-
+				"Spell ID {} could not be found.",
-			if (t != c) {
-				c->Message(
-					Chat::White,
-					"Unscribing spell: %s (%i) for %s.",
-					spells[spell_id].name,
-					spell_id,
-					t->GetName());
-			}
-
-			LogInfo("Unscribe spell: [{}] ([{}]) request for [{}] from [{}]",
-					spells[spell_id].name,
-					spell_id,
-					t->GetName(),
-					c->GetName());
-		}
-		else {
-			t->Message(
-				Chat::Red,
-				"Unable to unscribe spell: %s (%i) from your spellbook. This spell is not scribed.",
-				spells[spell_id].name,
 				spell_id
-			);
+			).c_str()
+		);
+		return;
+	}
 
-			if (t != c) {
+	auto spell_name = GetSpellName(spell_id);
-				c->Message(
+	
-					Chat::Red,
+	if (target->HasSpellScribed(spell_id)) {
-					"Unable to unscribe spell: %s (%i) for %s due to spell not scribed.",
+		target->UnscribeSpellBySpellID(spell_id);
-					spells[spell_id].name,
+
-					spell_id,
+		c->Message(
-					t->GetName());
+			Chat::White,
-			}
+			fmt::format(
-		}
+				"Unscribing {} ({}) for {}.",
+				spell_name,
+				spell_id,
+				c == target ?
+				"yourself" :
+				fmt::format(
+					"{} ({})",
+					target->GetCleanName(),
+					target->GetID()
+				)
+			).c_str()
+		);
+	} else {
+		c->Message(
+			Chat::White,
+			fmt::format(
+				"{} not have {} ({}) scribed.",
+				c == target ?
+				"You do" :
+				fmt::format(
+					"{} ({}) does",
+					target->GetCleanName(),
+					target->GetID()
+				),
+				spell_name,
+				spell_id
+			).c_str()
+		);
 	}
 }
 

zone/lua_client.cpp

@@ -2336,6 +2336,16 @@ void Lua_Client::ResetCastbarCooldownBySpellID(uint32 spell_id) {
 	self->ResetCastbarCooldownBySpellID(spell_id);
 }
 
+void Lua_Client::UnscribeSpellBySpellID(uint16 spell_id) {
+	Lua_Safe_Call_Void();
+	self->UnscribeSpellBySpellID(spell_id);
+}
+
+void Lua_Client::UnscribeSpellBySpellID(uint16 spell_id, bool update_client) {
+	Lua_Safe_Call_Void();
+	self->UnscribeSpellBySpellID(spell_id, update_client);
+}
+
 luabind::scope lua_register_client() {
 	return luabind::class_<Lua_Client, Lua_Mob>("Client")
 	.def(luabind::constructor<>())
@@ -2717,6 +2727,8 @@ luabind::scope lua_register_client() {
 	.def("UnscribeSpell", (void(Lua_Client::*)(int,bool))&Lua_Client::UnscribeSpell)
 	.def("UnscribeSpellAll", (void(Lua_Client::*)(bool))&Lua_Client::UnscribeSpellAll)
 	.def("UnscribeSpellAll", (void(Lua_Client::*)(void))&Lua_Client::UnscribeSpellAll)
+	.def("UnscribeSpellBySpellID", (void(Lua_Client::*)(uint16))&Lua_Client::UnscribeSpellBySpellID)
+	.def("UnscribeSpellBySpellID", (void(Lua_Client::*)(uint16,bool))&Lua_Client::UnscribeSpellBySpellID)
 	.def("UntrainDisc", (void(Lua_Client::*)(int))&Lua_Client::UntrainDisc)
 	.def("UntrainDisc", (void(Lua_Client::*)(int,bool))&Lua_Client::UntrainDisc)
 	.def("UntrainDiscAll", (void(Lua_Client::*)(bool))&Lua_Client::UntrainDiscAll)

zone/lua_client.h

@@ -183,6 +183,8 @@ public:
 	void UnscribeSpell(int slot, bool update_client);
 	void UnscribeSpellAll();
 	void UnscribeSpellAll(bool update_client);
+	void UnscribeSpellBySpellID(uint16 spell_id);
+	void UnscribeSpellBySpellID(uint16 spell_id, bool update_client);
 	void TrainDisc(int itemid);
 	uint16 LearnDisciplines(uint8 min_level, uint8 max_level);
 	void TrainDiscBySpellID(int32 spell_id);

zone/perl_client.cpp

@@ -5976,6 +5976,26 @@ XS(XS_Client_ResetCastbarCooldownBySpellID) {
 	XSRETURN_EMPTY;
 }
 
+XS(XS_Client_UnscribeSpellBySpellID);
+XS(XS_Client_UnscribeSpellBySpellID) {
+	dXSARGS;
+	if (items != 2 && items != 3)
+		Perl_croak(aTHX_ "Usage: Client::UnscribeSpellBySpellID(THIS, uint16 spell_id, [bool update_client = true])");
+	{
+		Client* THIS;
+		uint16 spell_id = (uint16) SvUV(ST(1));
+		bool update_client = true;
+		VALIDATE_THIS_IS_CLIENT;
+
+		if (items == 3) {
+			update_client = (bool) SvTRUE(ST(2));
+		}
+
+		THIS->UnscribeSpellBySpellID(spell_id, update_client);
+	}
+	XSRETURN_EMPTY;
+}
+
 #ifdef __cplusplus
 extern "C"
 #endif
@@ -6292,6 +6312,7 @@ XS(boot_Client) {
 	newXSproto(strcpy(buf, "UnmemSpellBySpellID"), XS_Client_UnmemSpellBySpellID, file, "$$");
 	newXSproto(strcpy(buf, "UnscribeSpell"), XS_Client_UnscribeSpell, file, "$$;$");
 	newXSproto(strcpy(buf, "UnscribeSpellAll"), XS_Client_UnscribeSpellAll, file, "$;$");
+	newXSproto(strcpy(buf, "UnscribeSpellBySpellID"), XS_Client_UnscribeSpellBySpellID, file, "$$;$");
 	newXSproto(strcpy(buf, "UntrainDisc"), XS_Client_UntrainDisc, file, "$$;$");
 	newXSproto(strcpy(buf, "UntrainDiscAll"), XS_Client_UntrainDiscAll, file, "$;$");
 	newXSproto(strcpy(buf, "UntrainDiscBySpellID"), XS_Client_UntrainDiscBySpellID, file, "$$;$");

zone/spells.cpp

@@ -5345,14 +5345,14 @@ void Client::UnscribeSpell(int slot, bool update_client, bool defer_save)
 	m_pp.spell_book[slot] = 0xFFFFFFFF;
 
 	if (!defer_save) {
-		database.DeleteCharacterSpell(this->CharacterID(), m_pp.spell_book[slot], slot);
+		database.DeleteCharacterSpell(CharacterID(), m_pp.spell_book[slot], slot);
 	}
 
 	if (update_client && slot < EQ::spells::DynamicLookup(ClientVersion(), GetGM())->SpellbookSize) {
-		auto               outapp = new EQApplicationPacket(OP_DeleteSpell, sizeof(DeleteSpell_Struct));
+		auto outapp = new EQApplicationPacket(OP_DeleteSpell, sizeof(DeleteSpell_Struct));
-		DeleteSpell_Struct *del   = (DeleteSpell_Struct *) outapp->pBuffer;
+		DeleteSpell_Struct *del = (DeleteSpell_Struct *) outapp->pBuffer;
 		del->spell_slot = slot;
-		del->success    = 1;
+		del->success = 1;
 		QueuePacket(outapp);
 		safe_delete(outapp);
 	}
@@ -5370,6 +5370,16 @@ void Client::UnscribeSpellAll(bool update_client)
 	SaveSpells();
 }
 
+void Client::UnscribeSpellBySpellID(uint16 spell_id, bool update_client)
+{
+	for (int index = 0; index < EQ::spells::SPELLBOOK_SIZE; index++) {
+		if (IsValidSpell(m_pp.spell_book[index]) && m_pp.spell_book[index] == spell_id) {
+			UnscribeSpell(index, update_client, true);
+			break;
+		}
+	}
+}
+
 void Client::UntrainDisc(int slot, bool update_client, bool defer_save)
 {
 	if (slot >= MAX_PP_DISCIPLINES || slot < 0) {