Merge pull request #2710 from MishimaHaruna/fix-skill-idx

Fix skill idx

4 files changed, 148 insertions, 97 deletions

diff --git a/src/char/char.c b/src/char/char.c
index aac9ad20c..0406dbecf 100644
--- a/src/char/char.c
+++ b/src/char/char.c
@@ -425,8 +425,6 @@ static struct DBData char_create_charstatus(union DBKey key, va_list args)
 
 static int char_mmo_char_tosql(int char_id, struct mmo_charstatus *p)
 {
-	int i = 0;
-	int count = 0;
 	int diff = 0;
 	char save_status[128]; //For displaying save information. [Skotlex]
 	struct mmo_charstatus *cp;
@@ -591,8 +589,9 @@ static int char_mmo_char_tosql(int char_id, struct mmo_charstatus *p)
 		//insert here.
 		StrBuf->Clear(&buf);
 		StrBuf->Printf(&buf, "INSERT INTO `%s`(`char_id`,`map`,`x`,`y`) VALUES ", memo_db);
-		for( i = 0, count = 0; i < MAX_MEMOPOINTS; ++i )
-		{
+
+		int count = 0;
+		for (int i = 0; i < MAX_MEMOPOINTS; ++i) {
 			if( p->memo_point[i].map )
 			{
 				if( count )
@@ -624,24 +623,29 @@ static 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_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;
-				if( p->skill[i].flag != SKILL_FLAG_PERMANENT && p->skill[i].flag != SKILL_FLAG_PERM_GRANTED && (p->skill[i].flag - SKILL_FLAG_REPLACED_LV_0) == 0 )
-					continue;
-				if( count )
-					StrBuf->AppendStr(&buf, ",");
-				StrBuf->Printf(&buf, "('%d','%d','%d','%d')", char_id, p->skill[i].id,
-								 ( (p->skill[i].flag == SKILL_FLAG_PERMANENT || p->skill[i].flag == SKILL_FLAG_PERM_GRANTED) ? p->skill[i].lv : p->skill[i].flag - SKILL_FLAG_REPLACED_LV_0),
-								 p->skill[i].flag == SKILL_FLAG_PERM_GRANTED ? p->skill[i].flag : 0);/* other flags do not need to be saved */
-				++count;
-			}
+		int count = 0;
+		for (int i = 0; i < MAX_SKILL_DB; ++i) {
+			if (p->skill[i].id == 0)
+				continue;
+			if (p->skill[i].flag == SKILL_FLAG_TEMPORARY)
+				continue;
+			if (p->skill[i].lv == 0 && (p->skill[i].flag == SKILL_FLAG_PERM_GRANTED || p->skill[i].flag == SKILL_FLAG_PERMANENT))
+				continue;
+			if (p->skill[i].flag == SKILL_FLAG_REPLACED_LV_0)
+				continue;
+
+			if (Assert_chk(p->skill[i].flag == SKILL_FLAG_PERMANENT || p->skill[i].flag == SKILL_FLAG_PERM_GRANTED || p->skill[i].flag > SKILL_FLAG_REPLACED_LV_0))
+				continue;
+			if (count != 0)
+				StrBuf->AppendStr(&buf, ",");
+			int saved_lv = (p->skill[i].flag > SKILL_FLAG_REPLACED_LV_0) ? p->skill[i].flag - SKILL_FLAG_REPLACED_LV_0 : p->skill[i].lv;
+			int saved_flag = p->skill[i].flag == SKILL_FLAG_PERM_GRANTED ? p->skill[i].flag : 0; // other flags do not need to be saved
+			StrBuf->Printf(&buf, "('%d','%d','%d','%d')", char_id, p->skill[i].id, saved_lv, saved_flag);
+
+			++count;
 		}
-		if( count )
-		{
-			if( SQL_ERROR == SQL->QueryStr(inter->sql_handle, StrBuf->Value(&buf)) )
-			{
+		if (count != 0) {
+			if (SQL_ERROR == SQL->QueryStr(inter->sql_handle, StrBuf->Value(&buf))) {
 				Sql_ShowDebug(inter->sql_handle);
 				errors++;
 			}
@@ -651,7 +655,7 @@ static int char_mmo_char_tosql(int char_id, struct mmo_charstatus *p)
 	}
 
 	diff = 0;
-	for(i = 0; i < MAX_FRIENDS; i++){
+	for (int i = 0; i < MAX_FRIENDS; i++) {
 		if(p->friends[i].char_id != cp->friends[i].char_id ||
 			p->friends[i].account_id != cp->friends[i].account_id){
 			diff = 1;
@@ -669,8 +673,8 @@ static int char_mmo_char_tosql(int char_id, struct mmo_charstatus *p)
 
 		StrBuf->Clear(&buf);
 		StrBuf->Printf(&buf, "INSERT INTO `%s` (`char_id`, `friend_account`, `friend_id`) VALUES ", friend_db);
-		for( i = 0, count = 0; i < MAX_FRIENDS; ++i )
-		{
+		int count = 0;
+		for (int i = 0; i < MAX_FRIENDS; ++i) {
 			if( p->friends[i].char_id > 0 )
 			{
 				if( count )
@@ -695,7 +699,7 @@ static int char_mmo_char_tosql(int char_id, struct mmo_charstatus *p)
 	StrBuf->Clear(&buf);
 	StrBuf->Printf(&buf, "REPLACE INTO `%s` (`char_id`, `hotkey`, `type`, `itemskill_id`, `skill_lvl`) VALUES ", hotkey_db);
 	diff = 0;
-	for(i = 0; i < ARRAYLENGTH(p->hotkeys); i++){
+	for (int i = 0; i < ARRAYLENGTH(p->hotkeys); i++) {
 		if(memcmp(&p->hotkeys[i], &cp->hotkeys[i], sizeof(struct hotkey)))
 		{
 			if( diff )
@@ -1369,7 +1373,7 @@ static int char_mmo_char_fromsql(int char_id, struct mmo_charstatus *p, bool loa
 		SqlStmt_ShowDebug(stmt);
 	}
 
-	if( tmp_skill.flag != SKILL_FLAG_PERM_GRANTED )
+	if (tmp_skill.flag != SKILL_FLAG_PERM_GRANTED)
 		tmp_skill.flag = SKILL_FLAG_PERMANENT;
 
 	for (i = 0; i < MAX_SKILL_DB && SQL_SUCCESS == SQL->StmtNextRow(stmt); ++i) {
diff --git a/src/map/pc.c b/src/map/pc.c
index 06ab57e0e..5faadf76a 100644
--- a/src/map/pc.c
+++ b/src/map/pc.c
@@ -1605,58 +1605,56 @@ static void pc_calc_skilltree_clear(struct map_session_data *sd)
  *------------------------------------------*/
 static int pc_calc_skilltree(struct map_session_data *sd)
 {
-	int i,id=0,flag;
-	int class = 0, classidx = 0;
-
 	nullpo_ret(sd);
-	i = pc->calc_skilltree_normalize_job(sd);
-	class = pc->mapid2jobid(i, sd->status.sex);
+	uint32 job = pc->calc_skilltree_normalize_job(sd);
+	int class = pc->mapid2jobid(job, sd->status.sex);
 	if (class == -1) {
 		//Unable to normalize job??
-		ShowError("pc_calc_skilltree: Unable to normalize job %d for character %s (%d:%d)\n", i, sd->status.name, sd->status.account_id, sd->status.char_id);
+		ShowError("pc_calc_skilltree: Unable to normalize job %u for character %s (%d:%d)\n", job, sd->status.name, sd->status.account_id, sd->status.char_id);
 		return 1;
 	}
-	classidx = pc->class2idx(class);
+	int classidx = pc->class2idx(class);
 
 	pc->calc_skilltree_clear(sd);
 
-	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.
+	for (int i = 0; i < MAX_SKILL_DB; i++) {
+		if (sd->status.skill[i].flag == SKILL_FLAG_TEMPORARY || sd->status.skill[i].flag >= SKILL_FLAG_REPLACED_LV_0) {
+			// 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;
 			sd->status.skill[i].flag = SKILL_FLAG_PERMANENT;
 		}
 
-		if( sd->sc.count && sd->sc.data[SC_SOULLINK] && sd->sc.data[SC_SOULLINK]->val2 == SL_BARDDANCER && skill->dbs->db[i].nameid >= DC_HUMMING && skill->dbs->db[i].nameid <= DC_SERVICEFORYOU )
-		{ //Enable Bard/Dancer spirit linked skills.
-			if (sd->status.sex) {
-				// Link dancer skills to bard.
-				if (i < 8) {
-					Assert_report(i >= 8);
-					continue;
-				}
-				if (sd->status.skill[i-8].lv < 10)
-					continue;
-				sd->status.skill[i].id = skill->dbs->db[i].nameid;
-				sd->status.skill[i].lv = sd->status.skill[i-8].lv; // Set the level to the same as the linking skill
-				sd->status.skill[i].flag = SKILL_FLAG_TEMPORARY; // Tag it as a non-savable, non-uppable, bonus skill
+		if (sd->sc.count && sd->sc.data[SC_SOULLINK] && sd->sc.data[SC_SOULLINK]->val2 == SL_BARDDANCER
+		 && ((skill->dbs->db[i].nameid >= BA_WHISTLE && skill->dbs->db[i].nameid <= BA_APPLEIDUN)
+		  || (skill->dbs->db[i].nameid >= DC_HUMMING && skill->dbs->db[i].nameid <= DC_SERVICEFORYOU))
+		) {
+			//Enable Bard/Dancer spirit linked skills.
+			int linked_nameid = skill->get_linked_song_dance_id(skill->dbs->db[i].nameid);
+			if (linked_nameid == 0) {
+				Assert_report("Linked bard/dance skill not found");
+				continue;
+			}
+			int copy_from_index;
+			int copy_to_index;
+			if (sd->status.sex == SEX_MALE && skill->dbs->db[i].nameid >= BA_WHISTLE && skill->dbs->db[i].nameid <= BA_APPLEIDUN) {
+				copy_from_index = i;
+				copy_to_index = skill->get_index(linked_nameid);
 			} else {
-				// Link bard skills to dancer.
-				if (i < 8) {
-					Assert_report(i >= 8);
-					continue;
-				}
-				if (sd->status.skill[i].lv < 10)
-					continue;
-				sd->status.skill[i-8].id = skill->dbs->db[i-8].nameid;
-				sd->status.skill[i-8].lv = sd->status.skill[i].lv; // Set the level to the same as the linking skill
-				sd->status.skill[i-8].flag = SKILL_FLAG_TEMPORARY; // Tag it as a non-savable, non-uppable, bonus skill
+				copy_from_index = skill->get_index(linked_nameid);
+				copy_to_index = i;
 			}
+			if (copy_from_index < copy_to_index)
+				continue; // Copy only after the source skill has been filled into the tree
+			if (sd->status.skill[copy_from_index].lv < 10)
+				continue; // Copy only if the linked skill has been mastered
+			sd->status.skill[copy_to_index].id = skill->dbs->db[copy_to_index].nameid;
+			sd->status.skill[copy_to_index].lv = sd->status.skill[copy_from_index].lv; // Set the level to the same as the linking skill
+			sd->status.skill[copy_to_index].flag = SKILL_FLAG_TEMPORARY; // Tag it as a non-savable, non-uppable, bonus skill
 		}
 	}
 
 	if( pc_has_permission(sd, PC_PERM_ALL_SKILL) ) {
-		for (i = 0; i < MAX_SKILL_DB; i++) {
+		for (int 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,
@@ -1688,9 +1686,11 @@ static int pc_calc_skilltree(struct map_session_data *sd)
 		return 0;
 	}
 
+	bool changed = false;
 	do {
-		flag = 0;
-		for (i = 0; i < MAX_SKILL_TREE && (id = pc->skill_tree[classidx][i].id) > 0; i++) {
+		changed = false;
+		int id;
+		for (int i = 0; i < MAX_SKILL_TREE && (id = pc->skill_tree[classidx][i].id) > 0; i++) {
 			int idx = pc->skill_tree[classidx][i].idx;
 			bool satisfied = true;
 			if (sd->status.skill[idx].id > 0)
@@ -1740,10 +1740,10 @@ static int pc_calc_skilltree(struct map_session_data *sd)
 					sd->status.skill[idx].lv = 1; // need to manually specify a skill level
 					sd->status.skill[idx].flag = SKILL_FLAG_TEMPORARY; //So it is not saved, and tagged as a "bonus" skill.
 				}
-				flag = 1; // skill list has changed, perform another pass
+				changed = true; // skill list has changed, perform another pass
 			}
 		}
-	} while(flag);
+	} while (changed);
 
 	pc->calc_skilltree_bonus(sd, classidx);
 
@@ -4206,7 +4206,7 @@ static int pc_skill(struct map_session_data *sd, int id, int level, int flag)
 			if( sd->status.skill[index].id == id ) {
 				if( sd->status.skill[index].lv >= level )
 					return 0;
-				if( sd->status.skill[index].flag == SKILL_FLAG_PERMANENT ) //Non-granted skill, store it's level.
+				if (sd->status.skill[index].flag == SKILL_FLAG_PERMANENT) // Non-granted skill, store its level.
 					sd->status.skill[index].flag = SKILL_FLAG_REPLACED_LV_0 + sd->status.skill[index].lv;
 			} else {
 				sd->status.skill[index].id   = id;
@@ -7568,7 +7568,7 @@ static int pc_allskillup(struct map_session_data *sd)
 	nullpo_ret(sd);
 
 	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) {
+		if (sd->status.skill[i].flag == SKILL_FLAG_TEMPORARY || sd->status.skill[i].flag >= SKILL_FLAG_REPLACED_LV_0) {
 			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;
 			if (sd->status.skill[i].lv == 0)
diff --git a/src/map/skill.c b/src/map/skill.c
index ca90a7672..6f6effddc 100644
--- a/src/map/skill.c
+++ b/src/map/skill.c
@@ -72,6 +72,34 @@ static struct s_skill_dbs skilldbs;
 
 struct skill_interface *skill;
 
+static const struct {
+	int start;
+	int end;
+} skill_idx_ranges[] = {
+	{ NV_BASIC, NPC_LEX_AETERNA },
+	{ KN_CHARGEATK, SA_ELEMENTWIND },
+	{ RK_ENCHANTBLADE, AB_SILENTIUM },
+	{ WL_WHITEIMPRISON, SC_FEINTBOMB },
+	{ LG_CANNONSPEAR, SR_GENTLETOUCH_REVITALIZE },
+	{ WA_SWING_DANCE, WA_MOONLIT_SERENADE },
+	{ MI_RUSH_WINDMILL, MI_HARMONIZE },
+	{ WM_LESSON, WM_UNLIMITED_HUMMING_VOICE },
+	{ SO_FIREWALK, SO_EARTH_INSIGNIA },
+	{ GN_TRAINING_SWORD, GN_SLINGITEM_RANGEMELEEATK },
+	{ AB_SECRAMENT, LG_OVERBRAND_PLUSATK },
+	{ ALL_ODINS_RECALL, ALL_LIGHTGUARD },
+	{ RL_GLITTERING_GREED, RL_GLITTERING_GREED_ATK },
+	{ KO_YAMIKUMO, OB_AKAITSUKI },
+	{ ECL_SNOWFLIP, ALL_THANATOS_RECALL },
+	{ GC_DARKCROW, NC_MAGMA_ERUPTION_DOTDAMAGE },
+	{ SU_BASIC_SKILL, SU_SPIRITOFSEA },
+	{ HLIF_HEAL, MH_VOLCANIC_ASH },
+	{ MS_BASH, MER_INVINCIBLEOFF2 },
+	{ EL_CIRCLE_OF_FIRE, EL_STONE_RAIN },
+	{ GD_APPROVAL, GD_DEVELOPMENT },
+	CUSTOM_SKILL_RANGES
+};
+
 //Since only mob-casted splash skills can hit ice-walls
 static int skill_splash_target(struct block_list *bl)
 {
@@ -96,51 +124,37 @@ static int skill_name2id(const char *name)
 /// Returns the skill's array index, or 0 (Unknown Skill).
 static int skill_get_index(int skill_id)
 {
-	int skillRange[] = { NV_BASIC, NPC_LEX_AETERNA,
-			KN_CHARGEATK, SA_ELEMENTWIND,
-			RK_ENCHANTBLADE, AB_SILENTIUM,
-			WL_WHITEIMPRISON, SC_FEINTBOMB,
-			LG_CANNONSPEAR, SR_GENTLETOUCH_REVITALIZE,
-			WA_SWING_DANCE, WA_MOONLIT_SERENADE,
-			MI_RUSH_WINDMILL, MI_HARMONIZE,
-			WM_LESSON, WM_UNLIMITED_HUMMING_VOICE,
-			SO_FIREWALK, SO_EARTH_INSIGNIA,
-			GN_TRAINING_SWORD, GN_SLINGITEM_RANGEMELEEATK,
-			AB_SECRAMENT, LG_OVERBRAND_PLUSATK,
-			ALL_ODINS_RECALL, ALL_LIGHTGUARD,
-			RL_GLITTERING_GREED, RL_GLITTERING_GREED_ATK,
-			KO_YAMIKUMO, OB_AKAITSUKI,
-			ECL_SNOWFLIP, ALL_THANATOS_RECALL,
-			GC_DARKCROW, NC_MAGMA_ERUPTION_DOTDAMAGE,
-			SU_BASIC_SKILL, SU_SPIRITOFSEA,
-			HLIF_HEAL, MH_VOLCANIC_ASH,
-			MS_BASH, MER_INVINCIBLEOFF2,
-			EL_CIRCLE_OF_FIRE, EL_STONE_RAIN,
-			GD_APPROVAL, GD_DEVELOPMENT
-			CUSTOM_SKILL_RANGES};
-	int length = sizeof(skillRange) / sizeof(int);
-	STATIC_ASSERT(sizeof(skillRange) / sizeof(int) % 2 == 0, "skill_get_index: skillRange should be multiple of 2");
-
-
-	if (skill_id < skillRange[0] || skill_id > skillRange[length - 1]) {
+	int length = ARRAYLENGTH(skill_idx_ranges);
+
+
+	if (skill_id < skill_idx_ranges[0].start || skill_id > skill_idx_ranges[length - 1].end) {
 		ShowWarning("skill_get_index: skill id '%d' is not being handled!\n", skill_id);
+		Assert_report(0);
 		return 0;
 	}
 
 	int skill_idx = 0;
+	bool found = false;
 	// Map Skill ID to Skill Indexes (in reverse order)
-	for (int i = 0; i < length; i += 2) {
+	for (int i = 0; i < length; i++) {
 		// Check if SkillID belongs to this range.
-		if (skill_id <= skillRange[i + 1] && skill_id >= skillRange[i]) {
-			skill_idx += (skillRange[i + 1] - skill_id);
+		if (skill_id <= skill_idx_ranges[i].end && skill_id >= skill_idx_ranges[i].start) {
+			skill_idx += (skill_idx_ranges[i].end - skill_id);
+			found = true;
 			break;
 		}
 		// Add the difference of current range
-		skill_idx += (skillRange[i + 1] - skillRange[i] + 1);
+		skill_idx += (skill_idx_ranges[i].end - skill_idx_ranges[i].start + 1);
 	}
 
+	if (!found) {
+		ShowWarning("skill_get_index: skill id '%d' (idx: %d) is not handled as it lies outside the defined ranges!\n", skill_id, skill_idx);
+		Assert_report(0);
+		return 0;
+	}
 	if (skill_idx >= MAX_SKILL_DB) {
 		ShowWarning("skill_get_index: skill id '%d'(idx: %d) is not being handled as it exceeds MAX_SKILL_DB!\n", skill_id, skill_idx);
+		Assert_report(0);
 		return 0;
 	}
 
@@ -10927,6 +10941,37 @@ static int skill_count_wos(struct block_list *bl, va_list ap)
 	return 0;
 }
 
+/**
+ * Returns the linked song/dance skill ID, if any (for the Bard/Dancer Soul Link).
+ *
+ * @param skill_id The skill ID to look up
+ *
+ * @return The linked song or dance's skill ID if any
+ * @retval 0 if the given skill_id doesn't have a linked skill ID
+ */
+static int skill_get_linked_song_dance_id(int skill_id)
+{
+	switch (skill_id) {
+		case BA_WHISTLE:
+			return DC_HUMMING;
+		case BA_ASSASSINCROSS:
+			return DC_DONTFORGETME;
+		case BA_POEMBRAGI:
+			return DC_FORTUNEKISS;
+		case BA_APPLEIDUN:
+			return DC_SERVICEFORYOU;
+		case DC_HUMMING:
+			return BA_WHISTLE;
+		case DC_DONTFORGETME:
+			return BA_ASSASSINCROSS;
+		case DC_FORTUNEKISS:
+			return BA_POEMBRAGI;
+		case DC_SERVICEFORYOU:
+			return BA_APPLEIDUN;
+	}
+	return 0;
+}
+
 /*==========================================
  *
  *------------------------------------------*/
@@ -21801,4 +21846,5 @@ void skill_defaults(void)
 	skill->splash_target = skill_splash_target;
 	skill->check_npc_chaospanic = skill_check_npc_chaospanic;
 	skill->count_wos = skill_count_wos;
+	skill->get_linked_song_dance_id = skill_get_linked_song_dance_id;
 }
diff --git a/src/map/skill.h b/src/map/skill.h
index 65195dc75..4dbbaf147 100644
--- a/src/map/skill.h
+++ b/src/map/skill.h
@@ -2200,6 +2200,7 @@ struct skill_interface {
 	int (*splash_target) (struct block_list* bl);
 	int (*check_npc_chaospanic) (struct block_list *bl, va_list args);
 	int (*count_wos) (struct block_list *bl, va_list ap);
+	int (*get_linked_song_dance_id) (int skill_id);
 };
 
 #ifdef HERCULES_CORE