[Cleanup] Cleanup spell and max level bucket logic. (#2181)

* [Cleanup] Cleanup spell and max level bucket logic.
- Spell buckets will now allow new mob->SetBucket() buckets since most people use these now.
- Max level bucket will now allow new mob->SetBucket() bucket since most people use these now.
- Clean up GetScribeableSpells() and GetLearnableDisciplines() logic and magic numbers.
- Make GetClientMaxLevel() uint8 instead of int since it can only be 0-255.

* Fix typo from other commit.

* Lua setter.

* Update client.cpp

zone/client.cpp

@@ -282,11 +282,9 @@ Client::Client(EQStreamInterface* ieqs)
 	if (!RuleB(Character, PerCharacterQglobalMaxLevel) && !RuleB(Character, PerCharacterBucketMaxLevel)) {
 		SetClientMaxLevel(0);
 	} else if (RuleB(Character, PerCharacterQglobalMaxLevel)) {
-		int client_max_level = GetCharMaxLevelFromQGlobal();
+		SetClientMaxLevel(GetCharMaxLevelFromQGlobal());
-		SetClientMaxLevel(client_max_level);
 	} else if (RuleB(Character, PerCharacterBucketMaxLevel)) {
-		int client_max_level = GetCharMaxLevelFromBucket();
+		SetClientMaxLevel(GetCharMaxLevelFromBucket());
-		SetClientMaxLevel(client_max_level);
 	}
 
 	KarmaUpdateTimer = new Timer(RuleI(Chat, KarmaUpdateIntervalMS));
@@ -10295,40 +10293,45 @@ void Client::Fling(float value, float target_x, float target_y, float target_z,
 }
 
 std::vector<int> Client::GetLearnableDisciplines(uint8 min_level, uint8 max_level) {
-	bool SpellGlobalRule = RuleB(Spells, EnableSpellGlobals);
-	bool SpellBucketRule = RuleB(Spells, EnableSpellBuckets);
-	bool SpellGlobalCheckResult = false;
-	bool SpellBucketCheckResult = false;
 	std::vector<int> learnable_disciplines;
-	for (int spell_id = 0; spell_id < SPDAT_RECORDS; ++spell_id) {
+	for (uint16 spell_id = 0; spell_id < SPDAT_RECORDS; ++spell_id) {
 		bool learnable = false;
-		if (!IsValidSpell(spell_id))
+		if (!IsValidSpell(spell_id)) {
-			continue;
-		if (!IsDiscipline(spell_id))
-			continue;
-		if (spells[spell_id].classes[WARRIOR] == 0)
-			continue;
-		if (max_level > 0 && spells[spell_id].classes[m_pp.class_ - 1] > max_level)
-			continue;
-		if (min_level > 1 && spells[spell_id].classes[m_pp.class_ - 1] < min_level)
-			continue;
-		if (spells[spell_id].skill == 52)
-			continue;
-		if (RuleB(Spells, UseCHAScribeHack) && spells[spell_id].effect_id[EFFECT_COUNT - 1] == 10)
-			continue;
-		if (HasDisciplineLearned(spell_id))
 			continue;
+		}
 
-		if (SpellGlobalRule) {
+		if (!IsDiscipline(spell_id)) {
-			SpellGlobalCheckResult = SpellGlobalCheck(spell_id, CharacterID());
+			continue;
-			if (SpellGlobalCheckResult) {
-				learnable = true;
 		}
-		} else if (SpellBucketRule) {
+
-			SpellBucketCheckResult = SpellBucketCheck(spell_id, CharacterID());
+		if (spells[spell_id].classes[WARRIOR] == 0) {
-			if (SpellBucketCheckResult) {
+			continue;
-				learnable = true;
 		}
+
+		if (max_level && spells[spell_id].classes[m_pp.class_ - 1] > max_level) {
+			continue;
+		}
+
+		if (min_level > 1 && spells[spell_id].classes[m_pp.class_ - 1] < min_level) {
+			continue;
+		}
+
+		if (spells[spell_id].skill == EQ::skills::SkillTigerClaw) {
+			continue;
+		}
+
+		if (RuleB(Spells, UseCHAScribeHack) && spells[spell_id].effect_id[EFFECT_COUNT - 1] == SE_CHA) {
+			continue;
+		}
+
+		if (HasDisciplineLearned(spell_id)) {
+			continue;
+		}
+
+		if (RuleB(Spells, EnableSpellGlobals) && SpellGlobalCheck(spell_id, CharacterID())) {
+			learnable = true;
+		} else if (RuleB(Spells, EnableSpellBuckets) && SpellBucketCheck(spell_id, CharacterID())) {
+			learnable = true;
 		} else {
 			learnable = true;
 		}
@@ -10361,40 +10364,45 @@ std::vector<int> Client::GetMemmedSpells() {
 }
 
 std::vector<int> Client::GetScribeableSpells(uint8 min_level, uint8 max_level) {
-	bool SpellGlobalRule = RuleB(Spells, EnableSpellGlobals);
-	bool SpellBucketRule = RuleB(Spells, EnableSpellBuckets);
-	bool SpellGlobalCheckResult = false;
-	bool SpellBucketCheckResult = false;
 	std::vector<int> scribeable_spells;
-	for (int spell_id = 0; spell_id < SPDAT_RECORDS; ++spell_id) {
+	for (uint16 spell_id = 0; spell_id < SPDAT_RECORDS; ++spell_id) {
 		bool scribeable = false;
-		if (!IsValidSpell(spell_id))
+		if (!IsValidSpell(spell_id)) {
-			continue;
-		if (IsDiscipline(spell_id))
-			continue;
-		if (spells[spell_id].classes[WARRIOR] == 0)
-			continue;
-		if (max_level > 0 && spells[spell_id].classes[m_pp.class_ - 1] > max_level)
-			continue;
-		if (min_level > 1 && spells[spell_id].classes[m_pp.class_ - 1] < min_level)
-			continue;
-		if (spells[spell_id].skill == 52)
-			continue;
-		if (RuleB(Spells, UseCHAScribeHack) && spells[spell_id].effect_id[EFFECT_COUNT - 1] == 10)
-			continue;
-		if (HasSpellScribed(spell_id))
 			continue;
+		}
 
-		if (SpellGlobalRule) {
+		if (IsDiscipline(spell_id)) {
-			SpellGlobalCheckResult = SpellGlobalCheck(spell_id, CharacterID());
+			continue;
-			if (SpellGlobalCheckResult) {
-				scribeable = true;
 		}
-		} else if (SpellBucketRule) {
+
-			SpellBucketCheckResult = SpellBucketCheck(spell_id, CharacterID());
+		if (spells[spell_id].classes[WARRIOR] == 0) {
-			if (SpellBucketCheckResult) {
+			continue;
-				scribeable = true;
 		}
+
+		if (max_level && spells[spell_id].classes[m_pp.class_ - 1] > max_level) {
+			continue;
+		}
+
+		if (min_level > 1 && spells[spell_id].classes[m_pp.class_ - 1] < min_level) {
+			continue;
+		}
+
+		if (spells[spell_id].skill == EQ::skills::SkillTigerClaw) {
+			continue;
+		}
+
+		if (RuleB(Spells, UseCHAScribeHack) && spells[spell_id].effect_id[EFFECT_COUNT - 1] == SE_CHA) {
+			continue;
+		}
+
+		if (HasSpellScribed(spell_id)) {
+			continue;
+		}
+
+		if (RuleB(Spells, EnableSpellGlobals) && SpellGlobalCheck(spell_id, CharacterID())) {
+			scribeable = true;
+		} else if (RuleB(Spells, EnableSpellBuckets) && SpellBucketCheck(spell_id, CharacterID())) {
+			scribeable = true;
 		} else {
 			scribeable = true;
 		}

zone/client.h

@@ -720,8 +720,8 @@ public:
 	void SendGuildJoin(GuildJoin_Struct* gj);
 	void RefreshGuildInfo();
 
-	int GetClientMaxLevel() const { return client_max_level; }
+	uint8 GetClientMaxLevel() const { return client_max_level; }
-	void SetClientMaxLevel(int max_level) { client_max_level = max_level; }
+	void SetClientMaxLevel(uint8 max_level) { client_max_level = max_level; }
 
 	void CheckManaEndUpdate();
 	void SendManaUpdate();
@@ -828,8 +828,8 @@ public:
 	void UntrainDiscBySpellID(uint16 spell_id, bool update_client = true);
 	bool SpellGlobalCheck(uint16 spell_id, uint32 char_id);
 	bool SpellBucketCheck(uint16 spell_id, uint32 char_id);
-	uint32 GetCharMaxLevelFromQGlobal();
+	uint8 GetCharMaxLevelFromQGlobal();
-	uint32 GetCharMaxLevelFromBucket();
+	uint8 GetCharMaxLevelFromBucket();
 
 	void Fling(float value, float target_x, float target_y, float target_z, bool ignore_los = false, bool clipping = false);
 
@@ -2008,7 +2008,7 @@ private:
 	void InterrogateInventory_(bool errorcheck, Client* requester, int16 head, int16 index, const EQ::ItemInstance* inst, const EQ::ItemInstance* parent, bool log, bool silent, bool &error, int depth);
 	bool InterrogateInventory_error(int16 head, int16 index, const EQ::ItemInstance* inst, const EQ::ItemInstance* parent, int depth);
 
-	int client_max_level;
+	uint8 client_max_level;
 
 	uint32 m_expedition_id = 0;
 	ExpeditionInvite m_pending_expedition_invite { 0 };

zone/client_packet.cpp

@@ -1318,7 +1318,7 @@ void Client::Handle_Connect_OP_ZoneEntry(const EQApplicationPacket *app)
 	drakkin_details = m_pp.drakkin_details;
 
 	// Max Level for Character:PerCharacterQglobalMaxLevel and Character:PerCharacterBucketMaxLevel
-	int client_max_level = 0;
+	uint8 client_max_level = 0;
 	if (RuleB(Character, PerCharacterQglobalMaxLevel)) {
 		client_max_level = GetCharMaxLevelFromQGlobal();
 	} else if (RuleB(Character, PerCharacterBucketMaxLevel)) {

zone/embparser_api.cpp

@@ -8351,7 +8351,7 @@ XS(XS__checknamefilter);
 XS(XS__checknamefilter) {
 	dXSARGS;
 	if (items != 1) {
-		Perl_croak(aTHX_ "Usage: quest::checknamefilter(std::string name)");
+		Perl_croak(aTHX_ "Usage: quest::checknamefilter(string name)");
 	}
 
 	dXSTARG;

zone/exp.cpp

@@ -22,6 +22,7 @@
 #include "../common/string_util.h"
 
 #include "client.h"
+#include "data_bucket.h"
 #include "groups.h"
 #include "mob.h"
 #include "raids.h"
@@ -722,12 +723,12 @@ void Client::SetEXP(uint32 set_exp, uint32 set_aaxp, bool isrezzexp) {
 		}
 	}
 
-	if (GetClientMaxLevel() > 0) {
+	auto client_max_level = GetClientMaxLevel();
-		int client_max_level = GetClientMaxLevel();
+	if (client_max_level) {
 		if (GetLevel() >= client_max_level) {
-			uint32 expneeded = GetEXPForLevel(client_max_level);
+			auto exp_needed = GetEXPForLevel(client_max_level);
-			if(set_exp > expneeded) {
+			if (set_exp > exp_needed) {
-				set_exp = expneeded;
+				set_exp = exp_needed;
 			}
 		}
 	}
@@ -1148,44 +1149,52 @@ void Client::SendLeadershipEXPUpdate() {
 	FastQueuePacket(&outapp);
 }
 
-uint32 Client::GetCharMaxLevelFromQGlobal() {
+uint8 Client::GetCharMaxLevelFromQGlobal() {
-	QGlobalCache *char_c = nullptr;
+	auto char_cache = GetQGlobals();
-	char_c = GetQGlobals();
 
-	std::list<QGlobal> globalMap;
+	std::list<QGlobal> global_map;
-	uint32 ntype = 0;
 
-	if(char_c) {
+	if (char_cache) {
-		QGlobalCache::Combine(globalMap, char_c->GetBucket(), ntype, CharacterID(), zone->GetZoneID());
+		QGlobalCache::Combine(global_map, char_cache->GetBucket(), 0, CharacterID(), zone->GetZoneID());
 	}
 
-	auto iter = globalMap.begin();
+	for (const auto& global : global_map) {
-	uint32 gcount = 0;
+		if (global.name == "CharMaxLevel") {
-	while(iter != globalMap.end()) {
+			if (StringIsNumber(global.value)) {
-		if((*iter).name.compare("CharMaxLevel") == 0){
+				return static_cast<uint8>(std::stoul(global.value));
-			return atoi((*iter).value.c_str());
+			}
 		}
-		++iter;
-		++gcount;
 	}
 
 	return 0;
 }
 
-uint32 Client::GetCharMaxLevelFromBucket()
+uint8 Client::GetCharMaxLevelFromBucket()
 {
-	uint32      char_id = CharacterID();
+	auto new_bucket_name = fmt::format(
-	std::string query   = StringFormat("SELECT value FROM data_buckets WHERE `key` = '%i-CharMaxLevel'", char_id);
+		"{}-CharMaxLevel",
-	auto        results = database.QueryDatabase(query);
+		GetBucketKey()
-	if (!results.Success()) {
+	);
-		LogError("Data bucket for CharMaxLevel for char ID [{}] failed", char_id);
+
-		return 0;
+	auto bucket_value = DataBucket::GetData(new_bucket_name);
+	if (!bucket_value.empty()) {
+		if (StringIsNumber(bucket_value)) {
+			return static_cast<uint8>(std::stoul(bucket_value));
+		}
 	}
 
-	if (results.RowCount() > 0) {
+	auto old_bucket_name = fmt::format(
-		auto row = results.begin();
+		"{}-CharMaxLevel",
-		return atoi(row[0]);
+		CharacterID()
+	);
+
+	bucket_value = DataBucket::GetData(old_bucket_name);
+	if (!bucket_value.empty()) {
+		if (StringIsNumber(bucket_value)) {
+			return static_cast<uint8>(std::stoul(bucket_value));
 		}
+	}
+
 	return 0;
 }
 

zone/lua_client.cpp

@@ -1844,12 +1844,12 @@ void Lua_Client::SetSecondaryWeaponOrnamentation(uint32 model_id) {
 	self->SetSecondaryWeaponOrnamentation(model_id);
 }
 
-void Lua_Client::SetClientMaxLevel(int value) {
+void Lua_Client::SetClientMaxLevel(uint8 max_level) {
 	Lua_Safe_Call_Void();
-	self->SetClientMaxLevel(value);
+	self->SetClientMaxLevel(max_level);
 }
 
-int Lua_Client::GetClientMaxLevel() {
+uint8 Lua_Client::GetClientMaxLevel() {
 	Lua_Safe_Call_Int();
 	return self->GetClientMaxLevel();
 }

zone/lua_client.h

@@ -423,8 +423,8 @@ public:
 	void SetSecondaryWeaponOrnamentation(uint32 model_id);
 	void TaskSelector(luabind::adl::object table);
 
-	void SetClientMaxLevel(int value);
+	void SetClientMaxLevel(uint8 max_level);
-	int GetClientMaxLevel();
+	uint8 GetClientMaxLevel();
 
 	void DialogueWindow(std::string markdown);
 

zone/perl_client.cpp

@@ -4759,12 +4759,12 @@ XS(XS_Client_SetClientMaxLevel); /* prototype to pass -Wmissing-prototypes */
 XS(XS_Client_SetClientMaxLevel) {
 	dXSARGS;
 	if (items != 2)
-		Perl_croak(aTHX_ "Usage: Client::SetClientMaxLevel(THIS, int in_level)");
+		Perl_croak(aTHX_ "Usage: Client::SetClientMaxLevel(THIS, uint8 max_level)");
 	{
 		Client* THIS;
-		int in_level = (int)SvUV(ST(1));
+		uint8 max_level = (uint8) SvUV(ST(1));
 		VALIDATE_THIS_IS_CLIENT;
-		THIS->SetClientMaxLevel(in_level);
+		THIS->SetClientMaxLevel(max_level);
 	}
 	XSRETURN_EMPTY;
 }
@@ -4776,7 +4776,7 @@ XS(XS_Client_GetClientMaxLevel) {
 		Perl_croak(aTHX_ "Usage: Client::GetClientMaxLevel(THIS)");
 	{
 		Client* THIS;
-		int RETVAL;
+		uint8 RETVAL;
 		dXSTARG;
 		VALIDATE_THIS_IS_CLIENT;
 		RETVAL = THIS->GetClientMaxLevel();

zone/spells.cpp

@@ -78,6 +78,7 @@ Copyright (C) 2001-2002 EQEMu Development Team (http://eqemu.org)
 #include "../common/data_verification.h"
 #include "../common/misc_functions.h"
 
+#include "data_bucket.h"
 #include "quest_parser_collection.h"
 #include "string_ids.h"
 #include "worldserver.h"
@@ -5511,7 +5512,7 @@ uint32 Client::GetHighestScribedSpellinSpellGroup(uint32 spell_group)
 	return highest_spell_id;
 }
 
-bool Client::SpellGlobalCheck(uint16 spell_id, uint32 char_id) {
+bool Client::SpellGlobalCheck(uint16 spell_id, uint32 character_id) {
 	std::string query = fmt::format(
 		"SELECT qglobal, value FROM spell_globals WHERE spellid = {}",
 		spell_id
@@ -5536,7 +5537,7 @@ bool Client::SpellGlobalCheck(uint16 spell_id, uint32 char_id) {
 
 	query = fmt::format(
 		"SELECT value FROM quest_globals WHERE charid = {} AND name = '{}'",
-		char_id,
+		character_id,
 		EscapeString(spell_global_name)
 	);
 
@@ -5546,7 +5547,7 @@ bool Client::SpellGlobalCheck(uint16 spell_id, uint32 char_id) {
 			"Spell global [{}] for spell ID [{}] for character ID [{}] query failed.",
 			spell_global_name,
 			spell_id,
-			char_id
+			character_id
 		);
 
 		return false; // Query failed, do not allow scribing.
@@ -5557,7 +5558,7 @@ bool Client::SpellGlobalCheck(uint16 spell_id, uint32 char_id) {
 			"Spell global [{}] for spell ID [{}] for character ID [{}] does not exist.",
 			spell_global_name,
 			spell_id,
-			char_id
+			character_id
 		);
 
 		return false; // No rows found, do not allow scribing.
@@ -5580,7 +5581,7 @@ bool Client::SpellGlobalCheck(uint16 spell_id, uint32 char_id) {
 		"Spell global [{}] for spell ID [{}] for character ID [{}] did not match value [{}] value found was [{}].",
 		spell_global_name,
 		spell_id,
-		char_id,
+		character_id,
 		spell_global_value,
 		global_value
 	);
@@ -5588,7 +5589,7 @@ bool Client::SpellGlobalCheck(uint16 spell_id, uint32 char_id) {
 	return false;
 }
 
-bool Client::SpellBucketCheck(uint16 spell_id, uint32 char_id) {
+bool Client::SpellBucketCheck(uint16 spell_id, uint32 character_id) {
 	auto query = fmt::format(
 		"SELECT `key`, value FROM spell_buckets WHERE spellid = {}",
 		spell_id
@@ -5611,37 +5612,14 @@ bool Client::SpellBucketCheck(uint16 spell_id, uint32 char_id) {
 		return true; // If the entry in the spell_buckets table has nothing set for the qglobal name, allow scribing.
 	}
 
-	query = fmt::format(
+	auto new_bucket_name = fmt::format(
-		"SELECT value FROM data_buckets WHERE `key` = '{}-{}'",
+		"{}-{}",
-		char_id,
+		GetBucketKey(),
-		EscapeString(spell_bucket_name)
+		spell_bucket_name
 	);
 
-	results = database.QueryDatabase(query);
+	auto bucket_value = DataBucket::GetData(new_bucket_name);
-	if (!results.Success()) {
+	if (!bucket_value.empty()) {
-		LogError(
-			"Spell bucket [{}] for spell ID [{}] for character ID [{}] query failed.",
-			spell_bucket_name,
-			spell_id,
-			char_id
-		);
-
-		return false; // Query failed, do not allow scribing.
-	}
-
-	if (!results.RowCount()) {
-		LogError(
-			"Spell bucket [{}] for spell ID [{}] for character ID [{}] does not exist.",
-			spell_bucket_name,
-			spell_id,
-			char_id
-		);
-
-		return false; // No rows found, do not allow scribing.
-	}
-
-	row = results.begin();
-	std::string bucket_value = row[0];
 		if (StringIsNumber(bucket_value) && StringIsNumber(spell_bucket_value)) {
 			if (std::stoi(bucket_value) >= std::stoi(spell_bucket_value)) {
 				return true; // If value is greater than or equal to spell bucket value, allow scribing.
@@ -5651,17 +5629,27 @@ bool Client::SpellBucketCheck(uint16 spell_id, uint32 char_id) {
 				return true; // If value is equal to spell bucket value, allow scribing.
 			}
 		}
+	}
 
-	// If user's data bucket does not meet requirements, do not allow scribing.
+	auto old_bucket_name = fmt::format(
-	LogError(
+		"{}-{}",
-		"Spell bucket [{}] for spell ID [{}] for character ID [{}] did not match value [{}] value found was [{}].",
+		character_id,
-		spell_bucket_name,
+		spell_bucket_name
-		spell_id,
-		char_id,
-		spell_bucket_value,
-		bucket_value
 	);
 
+	bucket_value = DataBucket::GetData(old_bucket_name);
+	if (!bucket_value.empty()) {
+		if (StringIsNumber(bucket_value) && StringIsNumber(spell_bucket_value)) {
+			if (std::stoi(bucket_value) >= std::stoi(spell_bucket_value)) {
+				return true; // If value is greater than or equal to spell bucket value, allow scribing.
+			}
+		} else {				
+			if (bucket_value == spell_bucket_value) {
+				return true; // If value is equal to spell bucket value, allow scribing.
+			}
+		}
+	}
+
 	return false;
 }