[Commands] Cleanup #scribespell and #scribespells Commands. (#2534)

- Cleanup messages and logic.

zone/command.cpp

@@ -271,8 +271,8 @@ int command_init(void)
 		command_add("rules", "(subcommand) - Manage server rules", AccountStatus::GMImpossible, command_rules) ||
 		command_add("save", "Force your player or player corpse target to be saved to the database", AccountStatus::Guide, command_save) ||
 		command_add("scale", "Handles npc scaling", AccountStatus::GMLeadAdmin, command_scale) ||
-		command_add("scribespell", "[spellid] - Scribe specified spell in your target's spell book.", AccountStatus::GMCoder, command_scribespell) ||
+		command_add("scribespell", "[Spell ID] - Scribe a spell by ID to your or your target's spell book.", AccountStatus::GMCoder, command_scribespell) ||
-		command_add("scribespells", "[max level] [min level] - Scribe all spells for you or your player target that are usable by them, up to level specified. (may freeze client for a few seconds)", AccountStatus::GMLeadAdmin, command_scribespells) ||
+		command_add("scribespells", "[Max level] [Min level] - Scribe all spells for you or your player target that are usable by them, up to level specified. (may freeze client for a few seconds)", AccountStatus::GMLeadAdmin, command_scribespells) ||
 		command_add("sendzonespawns", "Refresh spawn list for all clients in zone", AccountStatus::GMLeadAdmin, command_sendzonespawns) ||
 		command_add("sensetrap",  "Analog for ldon sense trap for the newer clients since we still don't have it working.", AccountStatus::Player, command_sensetrap) ||
 		command_add("serverinfo", "Get CPU, Operating System, and Process Information about the server", AccountStatus::GMMgmt, command_serverinfo) ||

zone/gm_commands/scribespell.cpp

@@ -2,65 +2,92 @@
 
 void command_scribespell(Client *c, const Seperator *sep)
 {
-	uint16 spell_id  = 0;
+	auto arguments = sep->argnum;
-	uint16 book_slot = -1;
+	if (!arguments || !sep->IsNumber(1)) {
-	Client *t        = c;
+		c->Message(Chat::White, "Usage: #scribespell [Spell ID]");
+		return;
+	}
 
+	auto t = c;
 	if (c->GetTarget() && c->GetTarget()->IsClient() && c->GetGM()) {
 		t = c->GetTarget()->CastToClient();
 	}
 
-	if (!sep->arg[1][0]) {
+	const auto spell_id = std::stoul(sep->arg[1]);
-		c->Message(Chat::White, "FORMAT: #scribespell <spellid>");
-		return;
-	}
-
-	spell_id = atoi(sep->arg[1]);
 
 	if (IsValidSpell(spell_id)) {
-		t->Message(Chat::White, "Scribing spell: %s (%i) to spellbook.", spells[spell_id].name, spell_id);
+		t->Message(
+			Chat::White,
+			fmt::format(
+				"Scribing {} ({}) to spellbook.",
+				spells[spell_id].name,
+				spell_id
+			).c_str()
+		);
 
 		if (t != c) {
-			c->Message(Chat::White, "Scribing spell: %s (%i) for %s.", spells[spell_id].name, spell_id, t->GetName());
+			c->Message(
+				Chat::White,
+				fmt::format(
+					"Scribing {} ({}) for {}.",
+					spells[spell_id].name,
+					spell_id,
+					t->GetName()
+				).c_str()
+			);
 		}
 
-		LogInfo("Scribe spell: [{}] ([{}]) request for [{}] from [{}]",
+		LogInfo(
-				spells[spell_id].name,
+			"Scribe spell: [{}] ([{}]) request for [{}] from [{}]",
-				spell_id,
+			spells[spell_id].name,
-				t->GetName(),
+			spell_id,
-				c->GetName());
+			t->GetName(),
+			c->GetName()
+		);
 
-		if (spells[spell_id].classes[WARRIOR] != 0 && spells[spell_id].skill != 52 &&
+		if (
-			spells[spell_id].classes[t->GetPP().class_ - 1] > 0 && !IsDiscipline(spell_id)) {
+			spells[spell_id].classes[WARRIOR] != 0 &&
-			book_slot = t->GetNextAvailableSpellBookSlot();
+			spells[spell_id].skill != EQ::skills::SkillTigerClaw &&
+			spells[spell_id].classes[t->GetPP().class_ - 1] > 0 &&
+			!IsDiscipline(spell_id)
+		) {
+			auto book_slot = t->GetNextAvailableSpellBookSlot();
 
 			if (book_slot >= 0 && t->FindSpellBookSlotBySpellID(spell_id) < 0) {
 				t->ScribeSpell(spell_id, book_slot);
-			}
+			} else {
-			else {
 				t->Message(
-					Chat::Red,
+					Chat::White,
-					"Unable to scribe spell: %s (%i) to your spellbook.",
+					fmt::format(
-					spells[spell_id].name,
+						"Unable to scribe {} ({}) to your spellbook.",
-					spell_id
+						spells[spell_id].name,
+						spell_id
+					).c_str()
 				);
 
 				if (t != c) {
 					c->Message(
-						Chat::Red,
+						Chat::White,
-						"Unable to scribe spell: %s (%i) for %s.",
+						fmt::format(
-						spells[spell_id].name,
+							"Unable to scribe {} ({}) for {}.",
-						spell_id,
+							spells[spell_id].name,
-						t->GetName());
+							spell_id,
+							t->GetName()
+						).c_str()
+					);
 				}
 			}
+		} else {
+			c->Message(Chat::White, "Your target cannot scribe this spell.");
 		}
-		else {
+	} else {
-			c->Message(Chat::Red, "Your target can not scribe this spell.");
+		c->Message(
-		}
+			Chat::White,
-	}
+			fmt::format(
-	else {
+				"Spell ID {} is an unknown spell and cannot be scribed.",
-		c->Message(Chat::Red, "Spell ID: %i is an unknown spell and cannot be scribed.", spell_id);
+				spell_id
+			).c_str()
+		);
 	}
 }
 

zone/gm_commands/scribespells.cpp

@@ -2,21 +2,22 @@
 
 void command_scribespells(Client *c, const Seperator *sep)
 {
-	Client *target = c;
+	auto arguments = sep->argnum;
-	if (c->GetTarget() && c->GetTarget()->IsClient() && c->GetGM()) {
+	if (!arguments || !sep->IsNumber(1)) {
-		target = c->GetTarget()->CastToClient();
-	}
-
-	if (sep->argnum < 1 || !sep->IsNumber(1)) {
 		c->Message(Chat::White, "Usage: #scribespells [Max Level] [Min Level]");
 		return;
 	}
 
-	uint8 rule_max_level = (uint8) RuleI(Character, MaxLevel);
+	auto t = c;
-	uint8 max_level = (uint8) std::stoi(sep->arg[1]);
+	if (c->GetTarget() && c->GetTarget()->IsClient() && c->GetGM()) {
+		t = c->GetTarget()->CastToClient();
+	}
+
+	uint8 rule_max_level = RuleI(Character, MaxLevel);
+	auto max_level = static_cast<uint8>(std::stoul(sep->arg[1]));
 	uint8 min_level = (
 		sep->IsNumber(2) ?
-		(uint8) std::stoi(sep->arg[2]) :
+		static_cast<uint8>(std::stoul(sep->arg[2])) :
 		1
 	); // Default to Level 1 if there isn't a 2nd argument
 
@@ -31,35 +32,38 @@ void command_scribespells(Client *c, const Seperator *sep)
 	}
 
 	if (max_level < 1 || min_level < 1) {
-		c->Message(Chat::White, "Level must be greater than or equal to 1.");
+		c->Message(Chat::White, "Maximum Level and Minimum Level must be greater than 0.");
 		return;
 	}
 
 	if (min_level > max_level) {
-		c->Message(Chat::White, "Minimum Level must be less than or equal to Maximum Level.");
+		c->Message(Chat::White, "Maximum Level must be greater than Minimum Level.");
 		return;
 	}
 
-	uint16 scribed_spells = target->ScribeSpells(min_level, max_level);
+	const auto scribed_spells = t->ScribeSpells(min_level, max_level);
-	if (c != target) {
+
-		std::string spell_message = (
+	if (c != t) {
-			scribed_spells > 0 ?
+		const auto target_description = c->GetTargetDescription(t);
-			(
+
-				scribed_spells == 1 ?
+		if (!scribed_spells) {
-				"A new spell" :
+			c->Message(
+				Chat::White,
 				fmt::format(
-					"{} New spells",
+					"No new spells scribed for {}.",
-					scribed_spells
+					target_description
-				)
+				).c_str()
-			) :
+			);
-			"No new spells"
+			return;
-		);
+		}
+
 		c->Message(
 			Chat::White,
 			fmt::format(
-				"{} scribed for {}.",
+				"{} New spell{} scribed for {}.",
-				spell_message,
+				scribed_spells,
-				c->GetTargetDescription(target)
+				scribed_spells != 1 ? "s" : "",
+				target_description
 			).c_str()
 		);
 	}