Reduce confusion between skill IDs and skill DB indexes (part 2)

Remove MAX_SKILL in favor of MAX_SKILL_DB, when referring to the
maximum skill DB index. This is opposed to MAX_SKILL_ID, the maximum
skill ID.

Signed-off-by: Haru <haru@dotalux.com>

14 files changed, 44 insertions, 42 deletions

diff --git a/src/char/char.c b/src/char/char.c
index 78b2d19fa..cf3fbb5e7 100644
--- a/src/char/char.c
+++ b/src/char/char.c
@@ -600,7 +600,7 @@ int char_mmo_char_tosql(int char_id, struct mmo_charstatus* p)
 		StrBuf->Clear(&buf);
 		StrBuf->Printf(&buf, "INSERT INTO `%s`(`char_id`,`id`,`lv`,`flag`) VALUES ", skill_db);
 		//insert here.
-		for( i = 0, count = 0; i < MAX_SKILL; ++i ) {
+		for (i = 0, count = 0; i < MAX_SKILL_DB; ++i) {
 			if( p->skill[i].id != 0 && p->skill[i].flag != SKILL_FLAG_TEMPORARY ) {
 				if( p->skill[i].lv == 0 && ( p->skill[i].flag == SKILL_FLAG_PERM_GRANTED || p->skill[i].flag == SKILL_FLAG_PERMANENT ) )
 					continue;
@@ -1301,7 +1301,7 @@ int char_mmo_char_fromsql(int char_id, struct mmo_charstatus* p, bool load_every
 	//read skill
 	//`skill` (`char_id`, `id`, `lv`)
 	memset(&tmp_skill, 0, sizeof(tmp_skill));
-	if (SQL_ERROR == SQL->StmtPrepare(stmt, "SELECT `id`, `lv`,`flag` FROM `%s` WHERE `char_id`=? LIMIT %d", skill_db, MAX_SKILL)
+	if (SQL_ERROR == SQL->StmtPrepare(stmt, "SELECT `id`, `lv`,`flag` FROM `%s` WHERE `char_id`=? LIMIT %d", skill_db, MAX_SKILL_DB)
 	 || SQL_ERROR == SQL->StmtBindParam(stmt, 0, SQLDT_INT, &char_id, 0)
 	 || SQL_ERROR == SQL->StmtExecute(stmt)
 	 || SQL_ERROR == SQL->StmtBindColumn(stmt, 0, SQLDT_USHORT, &tmp_skill.id  , 0, NULL, NULL)
@@ -1314,7 +1314,7 @@ int char_mmo_char_fromsql(int char_id, struct mmo_charstatus* p, bool load_every
 	if( tmp_skill.flag != SKILL_FLAG_PERM_GRANTED )
 		tmp_skill.flag = SKILL_FLAG_PERMANENT;
 
-	for( i = 0; i < MAX_SKILL && SQL_SUCCESS == SQL->StmtNextRow(stmt); ++i ) {
+	for (i = 0; i < MAX_SKILL_DB && SQL_SUCCESS == SQL->StmtNextRow(stmt); ++i) {
 		if( skillid2idx[tmp_skill.id] )
 			memcpy(&p->skill[skillid2idx[tmp_skill.id]], &tmp_skill, sizeof(tmp_skill));
 		else
@@ -2979,8 +2979,8 @@ void char_parse_frommap_skillid2idx(int fd)
 	if( j )
 		j /= 4;
 	for(i = 0; i < j; i++) {
-		if( RFIFOW(fd, 4 + (i*4)) > MAX_SKILL_ID ) {
-			ShowWarning("Error skillid2dx[%d] = %d failed, %d is higher than MAX_SKILL_ID (%d)\n",RFIFOW(fd, 4 + (i*4)), RFIFOW(fd, 6 + (i*4)),RFIFOW(fd, 4 + (i*4)),MAX_SKILL_ID);
+		if (RFIFOW(fd, 4 + (i*4)) >= MAX_SKILL_ID) {
+			ShowWarning("Error skillid2dx[%d] = %d failed, %d is higher than MAX_SKILL_ID (%d)\n", RFIFOW(fd, 4 + (i*4)), RFIFOW(fd, 6 + (i*4)), RFIFOW(fd, 4 + (i*4)), MAX_SKILL_ID);
 			continue;
 		}
 		skillid2idx[RFIFOW(fd, 4 + (i*4))] = RFIFOW(fd, 6 + (i*4));
diff --git a/src/common/mmo.h b/src/common/mmo.h
index 52f68b719..6a0e75ca5 100644
--- a/src/common/mmo.h
+++ b/src/common/mmo.h
@@ -139,8 +139,8 @@
 #endif
 #define MAX_FAME 1000000000
 #define MAX_CART 100
-#ifndef MAX_SKILL
-#define MAX_SKILL 1510
+#ifndef MAX_SKILL_DB
+#define MAX_SKILL_DB 1510 ///< Maximum number of skills in the skill DB (compacted array size)
 #endif
 #ifndef MAX_SKILL_ID
 #define MAX_SKILL_ID 10015   // [Ind/Hercules] max used skill ID
@@ -635,7 +635,7 @@ struct mmo_charstatus {
 
 	struct point last_point,save_point,memo_point[MAX_MEMOPOINTS];
 	struct item inventory[MAX_INVENTORY],cart[MAX_CART];
-	struct s_skill skill[MAX_SKILL];
+	struct s_skill skill[MAX_SKILL_DB];
 
 	struct s_friend friends[MAX_FRIENDS]; //New friend system [Skotlex]
 #ifdef HOTKEY_SAVING
@@ -1203,4 +1203,8 @@ enum hz_char_ask_name_answer {
 #error MAX_SLOTS it too small
 #endif
 
+#ifdef MAX_SKILL
+#error MAX_SKILL has been replaced by MAX_SKILL_DB. Please update your custom definitions.
+#endif
+
 #endif /* COMMON_MMO_H */
diff --git a/src/map/atcommand.c b/src/map/atcommand.c
index b92ca8c48..1590c3f0c 100644
--- a/src/map/atcommand.c
+++ b/src/map/atcommand.c
@@ -9395,7 +9395,7 @@ ACMD(cddebug) {
 	}
 
 	if (!cd || (*message && !strcmpi(message,"reset"))) {
-		for(i = 0; i < MAX_SKILL; i++) {
+		for (i = 0; i < MAX_SKILL_DB; i++) {
 			if( sd->blockskill[i] ) {
 				clif->messages(fd,"Found skill '%s', unblocking...",skill->dbs->db[i].name);
 				sd->blockskill[i] = false;
diff --git a/src/map/chrif.c b/src/map/chrif.c
index 39aa046d7..96f144523 100644
--- a/src/map/chrif.c
+++ b/src/map/chrif.c
@@ -1360,10 +1360,10 @@ void chrif_skillid2idx(int fd) {
 	if (!sockt->session_is_valid(fd))
 		return;
 
-	WFIFOHEAD(fd,4 + (MAX_SKILL * 4));
+	WFIFOHEAD(fd,4 + (MAX_SKILL_DB * 4));
 	WFIFOW(fd,0) = 0x2b0b;
-	for(i = 0; i < MAX_SKILL; i++) {
-		if( skill->dbs->db[i].nameid ) {
+	for (i = 0; i < MAX_SKILL_DB; i++) {
+		if (skill->dbs->db[i].nameid != 0) {
 			WFIFOW(fd, 4 + (count*4)) = skill->dbs->db[i].nameid;
 			WFIFOW(fd, 6 + (count*4)) = i;
 			count++;
diff --git a/src/map/clif.c b/src/map/clif.c
index 4dc36be6d..4bf3cdf94 100644
--- a/src/map/clif.c
+++ b/src/map/clif.c
@@ -4866,9 +4866,9 @@ void clif_skillinfoblock(struct map_session_data *sd)
 	fd=sd->fd;
 	if (!fd) return;
 
-	WFIFOHEAD(fd, MAX_SKILL * 37 + 4);
+	WFIFOHEAD(fd, MAX_SKILL_DB * 37 + 4);
 	WFIFOW(fd,0) = 0x10f;
-	for ( i = 0, len = 4; i < MAX_SKILL; i++) {
+	for ( i = 0, len = 4; i < MAX_SKILL_DB; i++) {
 		if( (id = sd->status.skill[i].id) != 0 ) {
 			int level;
 			// workaround for bugreport:5348
@@ -4899,7 +4899,7 @@ void clif_skillinfoblock(struct map_session_data *sd)
 	WFIFOSET(fd,len);
 
 	// workaround for bugreport:5348; send the remaining skills one by one to bypass packet size limit
-	for ( ; i < MAX_SKILL; i++) {
+	for ( ; i < MAX_SKILL_DB; i++) {
 		if( (id = sd->status.skill[i].id) != 0 ) {
 			clif->addskill(sd, id);
 			clif->skillinfo(sd, id, 0);
@@ -5001,7 +5001,7 @@ void clif_skillinfo(struct map_session_data *sd,int skill_id, int inf)
 	int skill_lv;
 
 	nullpo_retv(sd);
-	Assert_retv(idx >= 0 && idx < MAX_SKILL);
+	Assert_retv(idx >= 0 && idx < MAX_SKILL_DB);
 
 	skill_lv = sd->status.skill[idx].lv;
 
@@ -17642,7 +17642,7 @@ int clif_autoshadowspell_list(struct map_session_data *sd) {
 
 	WFIFOHEAD(fd, 2 * 6 + 4);
 	WFIFOW(fd,0) = 0x442;
-	for( i = 0, c = 0; i < MAX_SKILL; i++ )
+	for (i = 0, c = 0; i < MAX_SKILL_DB; i++)
 		if( sd->status.skill[i].flag == SKILL_FLAG_PLAGIARIZED && sd->status.skill[i].id > 0 &&
 				sd->status.skill[i].id < GS_GLITTERING && skill->get_type(sd->status.skill[i].id) == BF_MAGIC )
 		{ // Can't auto cast both Extended class and 3rd class skills.
diff --git a/src/map/homunculus.h b/src/map/homunculus.h
index a80392cf6..f3b10fc19 100644
--- a/src/map/homunculus.h
+++ b/src/map/homunculus.h
@@ -124,7 +124,7 @@ struct homun_data {
 	struct map_session_data *master;      //pointer back to its master
 	int hungry_timer;                     //[orn]
 	unsigned int exp_next;
-	char blockskill[MAX_SKILL];           // [orn]
+	char blockskill[MAX_SKILL_DB];        // [orn]
 
 	int64 masterteleport_timer;
 };
diff --git a/src/map/mercenary.h b/src/map/mercenary.h
index 0af75e8a2..f3773f191 100644
--- a/src/map/mercenary.h
+++ b/src/map/mercenary.h
@@ -92,7 +92,7 @@ struct mercenary_data {
 	struct regen_data regen;
 	struct s_mercenary_db *db;
 	struct s_mercenary mercenary;
-	char blockskill[MAX_SKILL];
+	char blockskill[MAX_SKILL_DB];
 
 	struct map_session_data *master;
 	int contract_timer;
diff --git a/src/map/pc.c b/src/map/pc.c
index f622b3d42..d5d359557 100644
--- a/src/map/pc.c
+++ b/src/map/pc.c
@@ -1560,7 +1560,7 @@ int pc_calc_skillpoint(struct map_session_data* sd) {
 
 	nullpo_ret(sd);
 
-	for (i = 1; i < MAX_SKILL; i++) {
+	for (i = 1; i < MAX_SKILL_DB; i++) {
 		int skill_lv = pc->checkskill2(sd,i);
 		if (skill_lv > 0) {
 			inf2 = skill->dbs->db[i].inf2;
@@ -1596,7 +1596,7 @@ int pc_calc_skilltree(struct map_session_data *sd)
 	}
 	classidx = pc->class2idx(class);
 
-	for( i = 0; i < MAX_SKILL; i++ ) {
+	for (i = 0; i < MAX_SKILL_DB; i++) {
 		if( sd->status.skill[i].flag != SKILL_FLAG_PLAGIARIZED && sd->status.skill[i].flag != SKILL_FLAG_PERM_GRANTED ) //Don't touch these
 			sd->status.skill[i].id = 0; //First clear skills.
 		/* permanent skills that must be re-checked */
@@ -1613,7 +1613,7 @@ int pc_calc_skilltree(struct map_session_data *sd)
 		}
 	}
 
-	for( i = 0; i < MAX_SKILL; i++ ) {
+	for (i = 0; i < MAX_SKILL_DB; i++) {
 		if( sd->status.skill[i].flag != SKILL_FLAG_PERMANENT && sd->status.skill[i].flag != SKILL_FLAG_PERM_GRANTED && sd->status.skill[i].flag != SKILL_FLAG_PLAGIARIZED )
 		{ // Restore original level of skills after deleting earned skills.
 			sd->status.skill[i].lv = (sd->status.skill[i].flag == SKILL_FLAG_TEMPORARY) ? 0 : sd->status.skill[i].flag - SKILL_FLAG_REPLACED_LV_0;
@@ -1649,7 +1649,7 @@ int pc_calc_skilltree(struct map_session_data *sd)
 	}
 
 	if( pc_has_permission(sd, PC_PERM_ALL_SKILL) ) {
-		for( i = 0; i < MAX_SKILL; i++ ) {
+		for (i = 0; i < MAX_SKILL_DB; i++) {
 			switch(skill->dbs->db[i].nameid) {
 				/**
 				 * Dummy skills must be added here otherwise they'll be displayed in the,
@@ -1843,7 +1843,7 @@ int pc_clean_skilltree(struct map_session_data *sd)
 {
 	int i;
 	nullpo_ret(sd);
-	for (i = 0; i < MAX_SKILL; i++){
+	for (i = 0; i < MAX_SKILL_DB; i++) {
 		if (sd->status.skill[i].flag == SKILL_FLAG_TEMPORARY || sd->status.skill[i].flag == SKILL_FLAG_PLAGIARIZED) {
 			sd->status.skill[i].id = 0;
 			sd->status.skill[i].lv = 0;
@@ -5895,8 +5895,9 @@ int pc_checkskill(struct map_session_data *sd,uint16 skill_id) {
 	return 0;
 }
 int pc_checkskill2(struct map_session_data *sd,uint16 index) {
-	if(sd == NULL) return 0;
-	if(index >= ARRAYLENGTH(sd->status.skill) ) {
+	if (sd == NULL)
+		return 0;
+	if (index >= MAX_SKILL_DB) {
 		ShowError("pc_checkskill: Invalid skill index %d (char_id=%d).\n", index, sd->status.char_id);
 		return 0;
 	}
@@ -7293,7 +7294,7 @@ int pc_allskillup(struct map_session_data *sd)
 
 	nullpo_ret(sd);
 
-	for(i=0;i<MAX_SKILL;i++){
+	for (i = 0; i < MAX_SKILL_DB; i++) {
 		if (sd->status.skill[i].flag != SKILL_FLAG_PERMANENT && sd->status.skill[i].flag != SKILL_FLAG_PERM_GRANTED && sd->status.skill[i].flag != SKILL_FLAG_PLAGIARIZED) {
 			sd->status.skill[i].lv = (sd->status.skill[i].flag == SKILL_FLAG_TEMPORARY) ? 0 : sd->status.skill[i].flag - SKILL_FLAG_REPLACED_LV_0;
 			sd->status.skill[i].flag = SKILL_FLAG_PERMANENT;
@@ -7304,7 +7305,7 @@ int pc_allskillup(struct map_session_data *sd)
 
 	if (pc_has_permission(sd, PC_PERM_ALL_SKILL)) { //Get ALL skills except npc/guild ones. [Skotlex]
 		//and except SG_DEVIL [Komurka] and MO_TRIPLEATTACK and RG_SNATCHER [ultramage]
-		for(i=0;i<MAX_SKILL;i++){
+		for (i = 0; i < MAX_SKILL_DB; i++) {
 			switch( skill->dbs->db[i].nameid ) {
 				case SG_DEVIL:
 				case MO_TRIPLEATTACK:
@@ -7544,9 +7545,7 @@ int pc_resetskill(struct map_session_data* sd, int flag)
 			status_change_end(&sd->bl, SC_SPRITEMABLE, INVALID_TIMER);
 	}
 
-	for( i = 1; i < MAX_SKILL; i++ ) {
-		// FIXME: We're looping on i = [1..MAX_SKILL-1] (which makes sense as index for sd->status.skill[]) but then we're using the
-		// same i to access skill->dbs->db[], and especially to check skill_ischangesex(). This is wrong.
+	for (i = 1; i < MAX_SKILL_DB; i++) {
 		uint16 skill_id = 0;
 		int lv = sd->status.skill[i].lv;
 		if (lv < 1) continue;
diff --git a/src/map/pc.h b/src/map/pc.h
index 50cb1b68e..df0df979d 100644
--- a/src/map/pc.h
+++ b/src/map/pc.h
@@ -292,7 +292,7 @@ struct map_session_data {
 	uint16 skill_id_old,skill_lv_old;
 	uint16 skill_id_dance,skill_lv_dance;
 	short cook_mastery; // range: [0,1999] [Inkfish]
-	bool blockskill[MAX_SKILL];
+	bool blockskill[MAX_SKILL_DB];
 	int cloneskill_id, reproduceskill_id;
 	int menuskill_id, menuskill_val, menuskill_val2;
 	int invincible_timer;
diff --git a/src/map/script.c b/src/map/script.c
index 5773457a7..f72b54a0d 100644
--- a/src/map/script.c
+++ b/src/map/script.c
@@ -14590,7 +14590,7 @@ BUILDIN(getskilllist)
 	int i,j=0;
 	if (sd == NULL)
 		return true;
-	for(i=0;i<MAX_SKILL;i++) {
+	for (i = 0; i < MAX_SKILL_DB; i++) {
 		if(sd->status.skill[i].id > 0 && sd->status.skill[i].lv > 0) {
 			pc->setreg(sd,reference_uid(script->add_str("@skilllist_id"), j),sd->status.skill[i].id);
 			pc->setreg(sd,reference_uid(script->add_str("@skilllist_lv"), j),sd->status.skill[i].lv);
diff --git a/src/map/skill.c b/src/map/skill.c
index d2db6c1bb..710b4d403 100644
--- a/src/map/skill.c
+++ b/src/map/skill.c
@@ -18813,7 +18813,7 @@ int skill_blockpc_end(int tid, int64 tick, int id, intptr_t data)
 	struct map_session_data *sd = map->id2sd(id);
 	struct skill_cd * cd = NULL;
 
-	if (data <= 0 || data >= MAX_SKILL)
+	if (data <= 0 || data >= MAX_SKILL_DB)
 		return 0;
 	if (!sd || !sd->blockskill[data])
 		return 0;
@@ -18949,7 +18949,7 @@ int skill_blockpc_start_(struct map_session_data *sd, uint16 skill_id, int tick)
 int skill_blockhomun_end(int tid, int64 tick, int id, intptr_t data)
 {
 	struct homun_data *hd = map->id2hd(id);
-	if (data <= 0 || data >= MAX_SKILL)
+	if (data <= 0 || data >= MAX_SKILL_DB)
 		return 0;
 	if (hd != NULL)
 		hd->blockskill[data] = 0;
@@ -18978,7 +18978,7 @@ int skill_blockhomun_start(struct homun_data *hd, uint16 skill_id, int tick)
 int skill_blockmerc_end(int tid, int64 tick, int id, intptr_t data)
 {
 	struct mercenary_data *md = map->id2mc(id);
-	if (data <= 0 || data >= MAX_SKILL)
+	if (data <= 0 || data >= MAX_SKILL_DB)
 		return 0;
 	if (md != NULL)
 		md->blockskill[data] = 0;
@@ -20855,7 +20855,7 @@ bool skill_read_skilldb(const char *filename)
 	struct config_setting_t *sk, *conf;
 	char filepath[256];
 	int count=0, index=0;
-	bool duplicate[MAX_SKILL] = {0};
+	bool duplicate[MAX_SKILL_DB] = {0};
 
 	nullpo_retr(false, filename);
 
diff --git a/src/map/skill.h b/src/map/skill.h
index 7544898bb..fa95a1a99 100644
--- a/src/map/skill.h
+++ b/src/map/skill.h
@@ -25,7 +25,7 @@
 #include "map/status.h" // enum sc_type
 #include "common/hercules.h"
 #include "common/db.h"
-#include "common/mmo.h" // MAX_SKILL, struct square
+#include "common/mmo.h" // MAX_SKILL_DB, struct square
 
 /**
  * Declarations
@@ -43,7 +43,6 @@ struct status_change_entry;
 /**
  * Defines
  **/
-#define MAX_SKILL_DB              MAX_SKILL
 #define MAX_SKILL_PRODUCE_DB      270
 #define MAX_PRODUCE_RESOURCE      10
 #define MAX_SKILL_ARROW_DB        140
diff --git a/src/map/status.c b/src/map/status.c
index ccd1e6b11..ddb8c7e6a 100644
--- a/src/map/status.c
+++ b/src/map/status.c
@@ -159,7 +159,7 @@ void initChangeTables(void)
 	for (i = 0; i < SC_MAX; i++)
 		status->dbs->IconChangeTable[i] = SI_BLANK;
 
-	for (i = 0; i < MAX_SKILL; i++)
+	for (i = 0; i < MAX_SKILL_DB; i++)
 		status->dbs->Skill2SCTable[i] = SC_NONE;
 
 	for (i = 0; i < SI_MAX; i++)
@@ -2329,7 +2329,7 @@ int status_calc_pc_(struct map_session_data* sd, enum e_status_calc_opt opt)
 	static int calculating = 0; //Check for recursive call preemption. [Skotlex]
 	struct status_data *bstatus; // pointer to the player's base status
 	const struct status_change *sc;
-	struct s_skill b_skill[MAX_SKILL]; // previous skill tree
+	struct s_skill b_skill[MAX_SKILL_DB]; // previous skill tree
 	int b_weight, b_max_weight, b_cart_weight_max, // previous weight
 		i, k, index, skill_lv,refinedef=0;
 	int64 i64;
diff --git a/src/map/status.h b/src/map/status.h
index a104c66ab..e9c2218e8 100644
--- a/src/map/status.h
+++ b/src/map/status.h
@@ -2257,7 +2257,7 @@ BEGIN_ZEROED_BLOCK; /* Everything within this block will be memset to 0 when sta
 	int HP_table[CLASS_COUNT][MAX_LEVEL + 1];
 	int SP_table[CLASS_COUNT][MAX_LEVEL + 1];
 	int aspd_base[CLASS_COUNT][MAX_SINGLE_WEAPON_TYPE+1]; // +1 for RENEWAL_ASPD
-	sc_type Skill2SCTable[MAX_SKILL];  // skill  -> status
+	sc_type Skill2SCTable[MAX_SKILL_DB];  // skill  -> status
 	int IconChangeTable[SC_MAX];          // status -> "icon" (icon is a bit of a misnomer, since there exist values with no icon associated)
 	unsigned int ChangeFlagTable[SC_MAX]; // status -> flags
 	int SkillChangeTable[SC_MAX];         // status -> skill