From 3144ac1a2839350a686216359e08facd84131789 Mon Sep 17 00:00:00 2001
From: Alex King <89047260+Kinglykrab@users.noreply.github.com>
Date: Wed, 24 May 2023 23:36:38 -0400
Subject: [PATCH] [Cleanup] Cleanup #setskill and #setskillall Commands (#3367)

# Notes
- No need to cap at 400 for max skill.
- When setting skill/skills, if we go over cap, set to cap.
---
 zone/gm_commands/setskill.cpp    | 75 ++++++++++++++------------------
 zone/gm_commands/setskillall.cpp | 55 +++++++++++------------
 2 files changed, 59 insertions(+), 71 deletions(-)

diff --git a/zone/gm_commands/setskill.cpp b/zone/gm_commands/setskill.cpp
index 297b366883..428a9cc486 100755
--- a/zone/gm_commands/setskill.cpp
+++ b/zone/gm_commands/setskill.cpp
@@ -2,54 +2,45 @@
 
 void command_setskill(Client *c, const Seperator *sep)
 {
-	Client *target = c;
+	const auto arguments = sep->argnum;
+
+	if (arguments < 2 || !sep->IsNumber(1) || !sep->IsNumber(2)) {
+		c->Message(Chat::White, "Usage: #setskill [Skill ID] [Skill Value]");
+		return;
+	}
+
+	auto t = c;
 	if (c->GetTarget() && c->GetTarget()->IsClient()) {
-		target = c->GetTarget()->CastToClient();
+		t = c->GetTarget()->CastToClient();
 	}
 
-	auto skill_id    = sep->IsNumber(1) ? Strings::ToInt(sep->arg[1]) : -1;
-	auto skill_value = sep->IsNumber(2) ? Strings::ToInt(sep->arg[2]) : -1;
-	if (
-		skill_id < 0 ||
-		skill_id > EQ::skills::HIGHEST_SKILL ||
-		skill_value < 0 ||
-		skill_value > HIGHEST_CAN_SET_SKILL
-		) {
+	const auto skill_id    = Strings::ToInt(sep->arg[1]);
+	const auto skill_value = Strings::ToInt(sep->arg[2]);
+
+	if (!EQ::ValueWithin(skill_id, EQ::skills::Skill1HBlunt, EQ::skills::HIGHEST_SKILL)) {
 		c->Message(Chat::White, "Usage: #setskill [Skill ID] [Skill Value]");
-		c->Message(Chat::White, fmt::format("Skill ID = 0 to {}", EQ::skills::HIGHEST_SKILL).c_str());
-		c->Message(Chat::White, fmt::format("Skill Value = 0 to {}", HIGHEST_CAN_SET_SKILL).c_str());
+		c->Message(Chat::White, fmt::format("Skill ID: 0 to {}", EQ::skills::HIGHEST_SKILL).c_str());
+		return;
 	}
-	else {
-		LogInfo(
-			"Set skill request from [{}], Target: [{}] Skill ID: [{}] Skill Value: [{}]",
-			c->GetCleanName(),
-			c->GetTargetDescription(target),
-			skill_id,
-			skill_value
-		);
 
-		if (
-			skill_id >= EQ::skills::Skill1HBlunt &&
-			skill_id <= EQ::skills::HIGHEST_SKILL
-			) {
-			target->SetSkill(
-				(EQ::skills::SkillType) skill_id,
-				skill_value
-			);
-
-			if (c != target) {
-				c->Message(
-					Chat::White,
-					fmt::format(
-						"Set {} ({}) to {} for {}.",
-						EQ::skills::GetSkillName((EQ::skills::SkillType) skill_id),
-						skill_id,
-						skill_value,
-						c->GetTargetDescription(target)
-					).c_str()
-				);
-			}
-		}
+	const auto skill_type = static_cast<EQ::skills::SkillType>(skill_id);
+
+	t->SetSkill(
+		skill_type,
+		skill_value > t->MaxSkill(skill_type) ? t->MaxSkill(skill_type) : skill_value
+	);
+
+	if (c != t) {
+		c->Message(
+			Chat::White,
+			fmt::format(
+				"Set {} ({}) to {} for {}.",
+				EQ::skills::GetSkillName(skill_type),
+				skill_id,
+				skill_value > t->MaxSkill(skill_type) ? t->MaxSkill(skill_type) : skill_value,
+				c->GetTargetDescription(t)
+			).c_str()
+		);
 	}
 }
 
diff --git a/zone/gm_commands/setskillall.cpp b/zone/gm_commands/setskillall.cpp
index e3652aef01..16f38a5ef3 100755
--- a/zone/gm_commands/setskillall.cpp
+++ b/zone/gm_commands/setskillall.cpp
@@ -2,45 +2,42 @@
 
 void command_setskillall(Client *c, const Seperator *sep)
 {
-	auto target = c;
+	const auto arguments = sep->argnum;
+
+	if (!arguments || !sep->IsNumber(1)) {
+		c->Message(Chat::White, "Usage: #setskillall [Skill Level] - Set all of your or your target's skills to the specified skill level");
+		return;
+	}
+
+	auto t = c;
 	if (c->GetTarget() && c->GetTarget()->IsClient()) {
-		target = c->GetTarget()->CastToClient();
+		t = c->GetTarget()->CastToClient();
 	}
 
-	if (!sep->IsNumber(1)) {
-		c->Message(Chat::White, "Usage: #setskillall [Skill Level] - Set all of your or your target's skills to the specified skill level");
-		c->Message(
-			Chat::White,
-			fmt::format(
-				"Note: Skill Level ranges from 0 to {}",
-				HIGHEST_CAN_SET_SKILL
-			).c_str()
-		);
-	} else {
-		if (c->Admin() >= commandSetSkillsOther || c->GetTarget() == c) {
-			LogInfo(
-				"Set ALL skill request from [{}], target:[{}]",
-				c->GetCleanName(),
-				c->GetTargetDescription(target)
-			);
+	if (c->Admin() < commandSetSkillsOther && t != c) {
+		c->Message(Chat::White, "Your status is not high enough to set another player's skills.");
+		return;
+	}
 
-			auto skill_level = static_cast<uint16>(Strings::ToUnsignedInt(sep->arg[1]));
+	auto skill_level = static_cast<uint16>(Strings::ToUnsignedInt(sep->arg[1]));
 
+	for (const auto& s : EQ::skills::GetSkillTypeMap()) {
+		if (c != t) {
 			c->Message(
 				Chat::White,
 				fmt::format(
-					"Setting all skills to {} for {}.",
-					skill_level,
-					c->GetTargetDescription(target)
+					"Setting {} ({}) to {} for {}.",
+					s.second,
+					s.first,
+					skill_level > t->MaxSkill(s.first) ? t->MaxSkill(s.first) : skill_level,
+					c->GetTargetDescription(t)
 				).c_str()
 			);
-
-			for (EQ::skills::SkillType skill_num = EQ::skills::Skill1HBlunt; skill_num <= EQ::skills::HIGHEST_SKILL; skill_num = (EQ::skills::SkillType) (skill_num + 1)) {
-				target->SetSkill(skill_num, skill_level);
-			}
-		} else {
-			c->Message(Chat::White, "Your status is not high enough to set another player's skills.");
 		}
+
+		t->SetSkill(
+			s.first,
+			skill_level > t->MaxSkill(s.first) ? t->MaxSkill(s.first) : skill_level
+		);
 	}
 }
-