Reworked command and quest api 'traindisc' methods

zone/command.cpp

@@ -6407,9 +6407,6 @@ void command_beardcolor(Client *c, const Seperator *sep)
 
 void command_scribespells(Client *c, const Seperator *sep)
 {
-	// rewrote this command to test for possible type conversion issues
-	// most of the redundant checks can be removed if proven successful
-
 	Client *t = c;
 	if (c->GetTarget() && c->GetTarget()->IsClient() && c->GetGM())
 		t = c->GetTarget()->CastToClient();
@@ -6421,14 +6418,13 @@ void command_scribespells(Client *c, const Seperator *sep)
 
 	uint8 max_level = (uint8)atol(sep->arg[1]);
 	if (!c->GetGM() && max_level > (uint8)RuleI(Character, MaxLevel))
-		max_level = (uint8)RuleI(Character, MaxLevel);	//default to Character:MaxLevel if we're not a GM & it's higher than the max level
+		max_level = (uint8)RuleI(Character, MaxLevel); // default to Character:MaxLevel if we're not a GM & it's higher than the max level
 
-	uint8 min_level = (sep->IsNumber(2) ? (uint8)atol(sep->arg[2]) : 1);	//default to 1 if there isn't a 2nd argument
+	uint8 min_level = (sep->IsNumber(2) ? (uint8)atol(sep->arg[2]) : 1); // default to 1 if there isn't a 2nd argument
 	if (!c->GetGM() && min_level > (uint8)RuleI(Character, MaxLevel))
-		min_level = (uint8)RuleI(Character, MaxLevel);	//default to Character:MaxLevel if we're not a GM & it's higher than the max level
+		min_level = (uint8)RuleI(Character, MaxLevel); // default to Character:MaxLevel if we're not a GM & it's higher than the max level
 
-	if(max_level < 1 || min_level < 1)
+	if(max_level < 1 || min_level < 1) {
-	{
 		c->Message(0, "ERROR: Level must be greater than 1.");
 		return;
 	}
@@ -6486,7 +6482,7 @@ void command_scribespells(Client *c, const Seperator *sep)
 
 			uint16 spell_id_ = (uint16)spell_id;
 			if ((spell_id_ != spell_id) || (spell_id != spell_id_)) {
-				c->Message(13, "FATAL ERROR: Type conversion data loss with spell_id (%u != %i)", spell_id, spell_id_);
+				c->Message(13, "FATAL ERROR: Type conversion data loss with spell_id (%i != %u)", spell_id, spell_id_);
 				return;
 			}
 
@@ -8762,28 +8758,24 @@ void command_reloadtitles(Client *c, const Seperator *sep)
 
 void command_traindisc(Client *c, const Seperator *sep)
 {
-	uint8 max_level, min_level;
+	Client *t = c;
-	uint16 curspell, count;
+	if (c->GetTarget() && c->GetTarget()->IsClient() && c->GetGM())
-	Client *t=c;
+		t = c->GetTarget()->CastToClient();
 
-	if(c->GetTarget() && c->GetTarget()->IsClient() && c->GetGM())
+	if (sep->argnum < 1 || !sep->IsNumber(1)) {
-		t=c->GetTarget()->CastToClient();
-
-	if(!sep->arg[1][0])
-	{
 		c->Message(0, "FORMAT: #traindisc <max level> <min level>");
 		return;
 	}
 
-	max_level = (uint8)atoi(sep->arg[1]);
+	uint8 max_level = (uint8)atol(sep->arg[1]);
-	if (!c->GetGM() && max_level > RuleI(Character, MaxLevel))
+	if (!c->GetGM() && max_level >(uint8)RuleI(Character, MaxLevel))
-		max_level = RuleI(Character, MaxLevel);	//default to Character:MaxLevel if we're not a GM & it's higher than the max level
+		max_level = (uint8)RuleI(Character, MaxLevel); // default to Character:MaxLevel if we're not a GM & it's higher than the max level
-	min_level = sep->arg[2][0] ? (uint8)atoi(sep->arg[2]) : 1;	//default to 1 if there isn't a 2nd argument
-	if (!c->GetGM() && min_level > RuleI(Character, MaxLevel))
-		min_level = RuleI(Character, MaxLevel);	//default to Character:MaxLevel if we're not a GM & it's higher than the max level
 
-	if(max_level < 1 || min_level < 1)
+	uint8 min_level = (sep->IsNumber(2) ? (uint8)atol(sep->arg[2]) : 1); // default to 1 if there isn't a 2nd argument
-	{
+	if (!c->GetGM() && min_level > (uint8)RuleI(Character, MaxLevel))
+		min_level = (uint8)RuleI(Character, MaxLevel); // default to Character:MaxLevel if we're not a GM & it's higher than the max level
+
+	if(max_level < 1 || min_level < 1) {
 		c->Message(0, "ERROR: Level must be greater than 1.");
 		return;
 	}
@@ -8797,35 +8789,58 @@ void command_traindisc(Client *c, const Seperator *sep)
 		c->Message(0, "Training disciplines for %s.",  t->GetName());
 	Log(Logs::General, Logs::Normal, "Train disciplines request for %s from %s, levels: %u -> %u",  t->GetName(), c->GetName(), min_level, max_level);
 
-	for(curspell = 0, count = 0; curspell < SPDAT_RECORDS; curspell++)
+	int spell_id = 0;
-	{
+	int count = 0;
-		if
+
-		(
+	bool change = false;
-			spells[curspell].classes[WARRIOR] != 0 && // check if spell exists
+
-			spells[curspell].classes[t->GetPP().class_-1] <= max_level &&	//maximum level
+	for( ; spell_id < SPDAT_RECORDS; ++spell_id) {
-			spells[curspell].classes[t->GetPP().class_-1] >= min_level &&	//minimum level
+		if (spell_id < 0 || spell_id >= SPDAT_RECORDS) {
-			spells[curspell].skill != 52
+			c->Message(13, "FATAL ERROR: Spell id out-of-range (id: %i, min: 0, max: %i)", spell_id, SPDAT_RECORDS);
-		)
+			return;
-		{
+		}
-			if(IsDiscipline(curspell)){
+
-				//we may want to come up with a function like Client::GetNextAvailableSpellBookSlot() to help speed this up a little
+		while (true) {
-				for(int r = 0; r < MAX_PP_DISCIPLINES; r++) {
+			if (spells[spell_id].classes[WARRIOR] == 0) // check if spell exists
-					if(t->GetPP().disciplines.values[r] == curspell) {
+				break;
-						t->Message(13, "You already know this discipline.");
+			if (spells[spell_id].classes[t->GetPP().class_ - 1] > max_level) // maximum level
-						break;	//continue the 1st loop
+				break;
-					} else if(t->GetPP().disciplines.values[r] == 0) {
+			if (spells[spell_id].classes[t->GetPP().class_ - 1] < min_level) // minimum level
-						t->GetPP().disciplines.values[r] = curspell;
+				break;
-						database.SaveCharacterDisc(t->CharacterID(), r, curspell);
+			if (spells[spell_id].skill == 52)
-						t->SendDisciplineUpdate();
+				break;
-						t->Message(0, "You have learned a new discipline!");
+			
-						count++;	//success counter
+			uint16 spell_id_ = (uint16)spell_id;
-						break;	//continue the 1st loop
+			if ((spell_id_ != spell_id) || (spell_id != spell_id_)) {
-					}	//if we get to this point, there's already a discipline in this slot, so we continue onto the next slot
+				c->Message(13, "FATAL ERROR: Type conversion data loss with spell_id (%i != %u)", spell_id, spell_id_);
-				}
+				return;
 			}
+
+			if (!IsDiscipline(spell_id_))
+				break;
+
+			for (uint32 r = 0; r < MAX_PP_DISCIPLINES; ++r) {
+				if (t->GetPP().disciplines.values[r] == spell_id_) {
+					t->Message(13, "You already know this discipline.");
+					break; // continue the 1st loop
+				}
+				else if (t->GetPP().disciplines.values[r] == 0) {
+					t->GetPP().disciplines.values[r] = spell_id_;
+					database.SaveCharacterDisc(t->CharacterID(), r, spell_id_);
+					change = true;
+					t->Message(0, "You have learned a new discipline!");
+					++count; // success counter
+					break; // continue the 1st loop
+				} // if we get to this point, there's already a discipline in this slot, so we continue onto the next slot
+			}
+
+			break;
 		}
 	}
 
+	if (change)
+		t->SendDisciplineUpdate();
+
 	if (count > 0) {
 		t->Message(0, "Successfully trained %u disciplines.",  count);
 		if (t != c)

zone/questmgr.cpp

@@ -974,9 +974,6 @@ void QuestManager::permagender(int gender_id) {
 }
 
 uint16 QuestManager::scribespells(uint8 max_level, uint8 min_level) {
-	// rewrote this handler to test for possible type conversion issues
-	// most of the redundant checks can be removed if proven successful
-
 	QuestManagerCurrentQuestVars();
 	int book_slot = initiator->GetNextAvailableSpellBookSlot();
 	int spell_id = 0;
@@ -1022,29 +1019,30 @@ uint16 QuestManager::scribespells(uint8 max_level, uint8 min_level) {
 
 			uint16 spell_id_ = (uint16)spell_id;
 			if ((spell_id_ != spell_id) || (spell_id != spell_id_)) {
-				initiator->Message(13, "FATAL ERROR: Type conversion data loss with spell_id (%u != %i)", spell_id, spell_id_);
+				initiator->Message(13, "FATAL ERROR: Type conversion data loss with spell_id (%i != %u)", spell_id, spell_id_);
 				return count;
 			}
 
-			if (!IsDiscipline(spell_id_) && !initiator->HasSpellScribed(spell_id)) { //isn't a discipline & we don't already have it scribed
+			if (!IsDiscipline(spell_id_) && !initiator->HasSpellScribed(spell_id)) { // isn't a discipline & we don't already have it scribed
 				if (SpellGlobalRule) {
-					// Bool to see if the character has the required QGlobal to scribe it if one exists in the Spell_Globals table
+					// bool to see if the character has the required QGlobal to scribe it if one exists in the Spell_Globals table
 					SpellGlobalCheckResult = initiator->SpellGlobalCheck(spell_id_, char_id);
 					if (SpellGlobalCheckResult) {
 						initiator->ScribeSpell(spell_id_, book_slot);
-						count++;
+						++count;
 					}
 				}
 				else if (SpellBucketRule) {
+					// bool to see if the character has the required bucket to train it if one exists in the spell_buckets table
 					SpellBucketCheckResult = initiator->SpellBucketCheck(spell_id_, char_id);
 					if (SpellBucketCheckResult) {
 						initiator->ScribeSpell(spell_id_, book_slot);
-						count++;
+						++count;
 					}
 				}
 				else {
 					initiator->ScribeSpell(spell_id_, book_slot);
-					count++;
+					++count;
 				}
 			}
 
@@ -1054,82 +1052,104 @@ uint16 QuestManager::scribespells(uint8 max_level, uint8 min_level) {
 		book_slot = initiator->GetNextAvailableSpellBookSlot(book_slot);
 	}
 
-	return count; //how many spells were scribed successfully
+	return count; // how many spells were scribed successfully
 }
 
 uint16 QuestManager::traindiscs(uint8 max_level, uint8 min_level) {
 	QuestManagerCurrentQuestVars();
-	uint16 count;
+	int spell_id = 0;
-	uint16 spell_id;
+	int count = 0;
 
 	uint32 char_id = initiator->CharacterID();
 	bool SpellGlobalRule = RuleB(Spells, EnableSpellGlobals);
 	bool SpellBucketRule = RuleB(Spells, EnableSpellBuckets);
-	bool SpellGlobalCheckResult = 0;
+	bool SpellGlobalCheckResult = false;
-	bool SpellBucketCheckResult = 0;
+	bool SpellBucketCheckResult = false;
 
-	for(spell_id = 0, count = 0; spell_id < SPDAT_RECORDS; spell_id++)
+	bool change = false;
-	{
+
-		if
+	for( ; spell_id < SPDAT_RECORDS; ++spell_id) {
-		(
+		if (spell_id < 0 || spell_id >= SPDAT_RECORDS) {
-			spells[spell_id].classes[WARRIOR] != 0 &&	//check if spell exists
+			initiator->Message(13, "FATAL ERROR: Spell id out-of-range (id: %i, min: 0, max: %i)", spell_id, SPDAT_RECORDS);
-			spells[spell_id].classes[initiator->GetPP().class_-1] <= max_level &&	//maximum level
+			return count;
-			spells[spell_id].classes[initiator->GetPP().class_-1] >= min_level &&	//minimum level
+		}
-			spells[spell_id].skill != 52 &&
+
-			( !RuleB(Spells, UseCHAScribeHack) || spells[spell_id].effectid[EFFECT_COUNT - 1] != 10 )
+		while (true) {
-		)
+			if (spells[spell_id].classes[WARRIOR] == 0) // check if spell exists
-		{
+				break;
-			if(IsDiscipline(spell_id)){
+			if (spells[spell_id].classes[initiator->GetPP().class_ - 1] > max_level) // maximum level
-				//we may want to come up with a function like Client::GetNextAvailableSpellBookSlot() to help speed this up a little
+				break;
-				for(uint32 r = 0; r < MAX_PP_DISCIPLINES; r++) {
+			if (spells[spell_id].classes[initiator->GetPP().class_ - 1] < min_level) // minimum level
-					if(initiator->GetPP().disciplines.values[r] == spell_id) {
+				break;
-						initiator->Message(13, "You already know this discipline.");
+			if (spells[spell_id].skill == 52)
-						break;	//continue the 1st loop
+				break;
-					}
+			if (RuleB(Spells, UseCHAScribeHack) && spells[spell_id].effectid[EFFECT_COUNT - 1] == 10)
-					else if(initiator->GetPP().disciplines.values[r] == 0) {
+				break;
-						if (SpellGlobalRule) {
+
-							// Bool to see if the character has the required QGlobal to train it if one exists in the Spell_Globals table
+			uint16 spell_id_ = (uint16)spell_id;
-							SpellGlobalCheckResult = initiator->SpellGlobalCheck(spell_id, char_id);
+			if ((spell_id_ != spell_id) || (spell_id != spell_id_)) {
-							if (SpellGlobalCheckResult) {
+				initiator->Message(13, "FATAL ERROR: Type conversion data loss with spell_id (%i != %u)", spell_id, spell_id_);
-								initiator->GetPP().disciplines.values[r] = spell_id;
+				return count;
-								database.SaveCharacterDisc(char_id, r, spell_id);
+			}
-								initiator->SendDisciplineUpdate();
+
-								initiator->Message(0, "You have learned a new discipline!");
+			if (!IsDiscipline(spell_id_))
-								count++;	//success counter
+				break;
-							}
+
-							break;	//continue the 1st loop
+			for (uint32 r = 0; r < MAX_PP_DISCIPLINES; r++) {
-						} else if (SpellBucketRule) {
+				if (initiator->GetPP().disciplines.values[r] == spell_id_) {
-							// Bool to see if the character has the required bucket to train it if one exists in the spell_buckets table
+					initiator->Message(13, "You already know this discipline.");
-							SpellBucketCheckResult = initiator->SpellBucketCheck(spell_id, char_id);
+					break; // continue the 1st loop
-							if (SpellBucketCheckResult) {
+				}
-								initiator->GetPP().disciplines.values[r] = spell_id;
+				else if (initiator->GetPP().disciplines.values[r] == 0) {
-								database.SaveCharacterDisc(char_id, r, spell_id);
+					if (SpellGlobalRule) {
-								initiator->SendDisciplineUpdate();
+						// bool to see if the character has the required QGlobal to train it if one exists in the Spell_Globals table
-								initiator->Message(0, "You have learned a new discipline!");
+						SpellGlobalCheckResult = initiator->SpellGlobalCheck(spell_id_, char_id);
-								count++;
+						if (SpellGlobalCheckResult) {
-							}
+							initiator->GetPP().disciplines.values[r] = spell_id_;
-							break;
+							database.SaveCharacterDisc(char_id, r, spell_id_);
-						}
+							change = true;
-						else {
-							initiator->GetPP().disciplines.values[r] = spell_id;
-							database.SaveCharacterDisc(char_id, r, spell_id);
-							initiator->SendDisciplineUpdate();
 							initiator->Message(0, "You have learned a new discipline!");
-							count++;	//success counter
+							++count; // success counter
-							break;	//continue the 1st loop
 						}
-					}	//if we get to this point, there's already a discipline in this slot, so we skip it
+						break; // continue the 1st loop
+					}
+					else if (SpellBucketRule) {
+						// bool to see if the character has the required bucket to train it if one exists in the spell_buckets table
+						SpellBucketCheckResult = initiator->SpellBucketCheck(spell_id_, char_id);
+						if (SpellBucketCheckResult) {
+							initiator->GetPP().disciplines.values[r] = spell_id_;
+							database.SaveCharacterDisc(char_id, r, spell_id_);
+							change = true;
+							initiator->Message(0, "You have learned a new discipline!");
+							++count;
+						}
+						break;
+					}
+					else {
+						initiator->GetPP().disciplines.values[r] = spell_id_;
+						database.SaveCharacterDisc(char_id, r, spell_id_);
+						change = true;;
+						initiator->Message(0, "You have learned a new discipline!");
+						++count; // success counter
+						break; // continue the 1st loop
+					}
 				}
 			}
+
+			break;
 		}
 	}
-	return count;	//how many disciplines were learned successfully
+
+	if (change)
+		initiator->SendDisciplineUpdate();
+
+	return count; // how many disciplines were learned successfully
 }
 
 void QuestManager::unscribespells() {
 	QuestManagerCurrentQuestVars();
 	initiator->UnscribeSpellAll();
-	}
+}
 
 void QuestManager::untraindiscs() {
 	QuestManagerCurrentQuestVars();