Fixed issue with mounts and jobchange, ranger and mechanic mounts weren't being checked and properly removed in pc_setoption

Abstracted running functions into two other methods (unit_run and unit_wugdash)
Added checks in skill_dance_switch so it'd be safe to call it whenever it's needed
Documented unit_run, unit_run_hit, skill_dance_switch and sc_conf_type

6 files changed, 86 insertions, 120 deletions

diff --git a/src/map/pc.c b/src/map/pc.c
index 3afb6c556..47744a839 100644
--- a/src/map/pc.c
+++ b/src/map/pc.c
@@ -7982,22 +7982,21 @@ int pc_setoption(struct map_session_data *sd,int type)
 	else if (!(type&OPTION_FALCON) && p_type&OPTION_FALCON) //Falcon OFF
 		clif->sc_end(&sd->bl,sd->bl.id,AREA,SI_FALCON);
 
-	if( (sd->class_&MAPID_THIRDMASK) == MAPID_RANGER ) {
-		if( type&OPTION_WUGRIDER && !(p_type&OPTION_WUGRIDER) ) { // Mounting
-			clif->sc_load(&sd->bl,sd->bl.id,AREA,SI_WUGRIDER, 0, 0, 0);
-			status_calc_pc(sd,SCO_NONE);
-		} else if( !(type&OPTION_WUGRIDER) && p_type&OPTION_WUGRIDER ) { // Dismount
-			clif->sc_end(&sd->bl,sd->bl.id,AREA,SI_WUGRIDER);
-			status_calc_pc(sd,SCO_NONE);
-		}
+	if( type&OPTION_WUGRIDER && !(p_type&OPTION_WUGRIDER) ) { // Mounting
+		clif->sc_load(&sd->bl,sd->bl.id,AREA,SI_WUGRIDER, 0, 0, 0);
+		status_calc_pc(sd,SCO_NONE);
+	} else if( !(type&OPTION_WUGRIDER) && p_type&OPTION_WUGRIDER ) { // Dismount
+		clif->sc_end(&sd->bl,sd->bl.id,AREA,SI_WUGRIDER);
+		status_calc_pc(sd,SCO_NONE);
 	}
-	if( (sd->class_&MAPID_THIRDMASK) == MAPID_MECHANIC ) {
+
+	if( (type&OPTION_MADOGEAR && !(p_type&OPTION_MADOGEAR))
+	|| (!(type&OPTION_MADOGEAR) && p_type&OPTION_MADOGEAR) ) {
 		int i;
-		if( type&OPTION_MADOGEAR && !(p_type&OPTION_MADOGEAR) )
-			status_calc_pc(sd, SCO_NONE);
-		else if( !(type&OPTION_MADOGEAR) && p_type&OPTION_MADOGEAR )
-			status_calc_pc(sd, SCO_NONE);
-		for( i = 0; i < SC_MAX; i++ ){
+		status_calc_pc(sd, SCO_NONE);
+
+		// End all SCs that can be reset when mado is taken off
+		for( i = 0; i < SC_MAX; i++ ) {
 			if ( !sd->sc.data[i] || !status->get_sc_type(i) )
 				continue;
 			if ( status->get_sc_type(i)&SC_MADO_NO_RESET )
diff --git a/src/map/skill.c b/src/map/skill.c
index 04aee371c..08275ab49 100644
--- a/src/map/skill.c
+++ b/src/map/skill.c
@@ -10630,14 +10630,22 @@ int skill_dance_overlap(struct skill_unit* su, int flag) {
 	return map->foreachincell(skill->dance_overlap_sub, su->bl.m,su->bl.x,su->bl.y,BL_SKILL, su,flag);
 }
 
-/*==========================================
+/**
  * Converts this group information so that it is handled as a Dissonance or Ugly Dance cell.
- * Flag: 0 - Convert, 1 - Revert.
- *------------------------------------------*/
+ * This function is safe to call even when the unit or the group were freed by other function
+ * previously.
+ * @param su Skill unit data (from BA_DISSONANCE or DC_UGLYDANCE)
+ * @param flag 0 Convert
+ * @param flag 1 Revert
+ * @retval true success
+ **/
 bool skill_dance_switch(struct skill_unit* su, int flag) {
 	static int prevflag = 1;  // by default the backup is empty
 	static struct skill_unit_group backup;
-	struct skill_unit_group* group = su->group;
+	struct skill_unit_group* group;
+
+	if( su == NULL || (group = su->group) == NULL )
+		return false;
 
 	// val2&UF_ENSEMBLE is a hack to indicate dissonance
 	if ( !(group->state.song_dance&0x1 && su->val2&UF_ENSEMBLE) )
@@ -16133,9 +16141,6 @@ int skill_unit_move_sub(struct block_list* bl, va_list ap) {
 			}
 		}
 
-		//TODO: Normally, this is dangerous since the unit and group could be freed
-		//inside the onout/onplace functions. Currently it is safe because we know song/dance
-		//cells do not get deleted within them. [Skotlex]
 		if( dissonance ) skill->dance_switch(su, 1);
 
 		if( flag&4 )
diff --git a/src/map/status.c b/src/map/status.c
index 915559260..0787e7cad 100644
--- a/src/map/status.c
+++ b/src/map/status.c
@@ -9502,7 +9502,7 @@ int status_change_start(struct block_list *src, struct block_list *bl, enum sc_t
 			{
 				struct unit_data *ud = unit->bl2ud(bl);
 				if( ud )
-					ud->state.running = unit->run(bl);
+					ud->state.running = unit->run(bl, NULL, SC_RUN);
 			}
 			break;
 		case SC_CASH_BOSS_ALARM:
@@ -9521,7 +9521,7 @@ int status_change_start(struct block_list *src, struct block_list *bl, enum sc_t
 			{
 				struct unit_data *ud = unit->bl2ud(bl);
 				if( ud )
-					ud->state.running = unit->wugdash(bl, sd);
+					ud->state.running = unit->run(bl, sd, SC_WUGDASH);
 			}
 			break;
 		case SC_COMBOATTACK:
diff --git a/src/map/status.h b/src/map/status.h
index be6429eff..13d0a7841 100644
--- a/src/map/status.h
+++ b/src/map/status.h
@@ -52,6 +52,10 @@ enum refine_type {
 	REFINE_TYPE_MAX     = 5
 };
 
+/**
+ * SC configuration type
+ * @see db/sc_config.txt for more information
+ **/
 typedef enum sc_conf_type {
 	SC_NO_REM_DEATH  = 0x01,
 	SC_NO_SAVE       = 0x02,
diff --git a/src/map/unit.c b/src/map/unit.c
index 83a98de8c..875eb30af 100644
--- a/src/map/unit.c
+++ b/src/map/unit.c
@@ -292,7 +292,7 @@ int unit_walktoxy_timer(int tid, int64 tick, int id, intptr_t data) {
 			clif->move(ud);
 	} else if(ud->state.running) {
 		//Keep trying to run.
-		if ( !(unit->run(bl) || unit->wugdash(bl,sd)) )
+		if ( !(unit->run(bl, NULL, SC_RUN) || unit->run(bl, sd, SC_WUGDASH)) )
 			ud->state.running = 0;
 	} else if (ud->target_to) {
 		//Update target trajectory.
@@ -486,137 +486,95 @@ int unit_walktobl(struct block_list *bl, struct block_list *tbl, int range, int
 	return 0;
 }
 
-int unit_run(struct block_list *bl) {
-	struct status_change *sc = status->get_sc(bl);
-	short to_x,to_y,dir_x,dir_y;
-	int lv;
-	int i;
-
-	if (!(sc && sc->data[SC_RUN]))
-		return 0;
-
-	if (!unit->can_move(bl)) {
-		status_change_end(bl, SC_RUN, INVALID_TIMER);
-		return 0;
-	}
-
-	lv = sc->data[SC_RUN]->val1;
-	dir_x = dirx[sc->data[SC_RUN]->val2];
-	dir_y = diry[sc->data[SC_RUN]->val2];
-
-	// determine destination cell
-	to_x = bl->x;
-	to_y = bl->y;
-	for(i=0;i<AREA_SIZE;i++) {
-		if(!map->getcell(bl->m,to_x+dir_x,to_y+dir_y,CELL_CHKPASS))
-			break;
-
-		//if sprinting and there's a PC/Mob/NPC, block the path [Kevin]
-		if(sc->data[SC_RUN] && map->count_oncell(bl->m, to_x+dir_x, to_y+dir_y, BL_PC|BL_MOB|BL_NPC))
-			break;
 
-		to_x += dir_x;
-		to_y += dir_y;
-	}
+/**
+ * Called by unit_run when an object was hit
+ * @param sd Required only when using SC_WUGDASH
+ **/
+void unit_run_hit( struct block_list *bl, struct status_change *sc, struct map_session_data *sd, enum sc_type type ) {
+	int lv = sc->data[type]->val1;
 
-	if( (to_x == bl->x && to_y == bl->y ) || (to_x == (bl->x+1) || to_y == (bl->y+1)) || (to_x == (bl->x-1) || to_y == (bl->y-1))) {
-		//If you can't run forward, you must be next to a wall, so bounce back. [Skotlex]
+	//If you can't run forward, you must be next to a wall, so bounce back. [Skotlex]
+	if( type == SC_RUN )
 		clif->sc_load(bl,bl->id,AREA,SI_TING,0,0,0);
 
-		//Set running to 0 beforehand so status_change_end knows not to enable spurt [Kevin]
-		unit->bl2ud(bl)->state.running = 0;
-		status_change_end(bl, SC_RUN, INVALID_TIMER);
+	//Set running to 0 beforehand so status_change_end knows not to enable spurt [Kevin]
+	unit->bl2ud(bl)->state.running = 0;
+	status_change_end(bl, type, INVALID_TIMER);
 
+	if( type == SC_RUN ) {
 		skill->blown(bl,bl,skill->get_blewcount(TK_RUN,lv),unit->getdir(bl),0);
 		clif->fixpos(bl); //Why is a clif->slide (skill->blown) AND a fixpos needed? Ask Aegis.
 		clif->sc_end(bl,bl->id,AREA,SI_TING);
-		return 0;
-	}
-	if (unit->walktoxy(bl, to_x, to_y, 1))
-		return 1;
-	//There must be an obstacle nearby. Attempt walking one cell at a time.
-	do {
-		to_x -= dir_x;
-		to_y -= dir_y;
-	} while (--i > 0 && !unit->walktoxy(bl, to_x, to_y, 1));
-	if ( i == 0 ) {
-		// copy-paste from above
-		clif->sc_load(bl,bl->id,AREA,SI_TING,0,0,0);
-
-		//Set running to 0 beforehand so status_change_end knows not to enable spurt [Kevin]
-		unit->bl2ud(bl)->state.running = 0;
-		status_change_end(bl, SC_RUN, INVALID_TIMER);
-
-		skill->blown(bl,bl,skill->get_blewcount(TK_RUN,lv),unit->getdir(bl),0);
+	} else if( sd ) {
 		clif->fixpos(bl);
-		clif->sc_end(bl,bl->id,AREA,SI_TING);
-		return 0;
+		skill->castend_damage_id(bl, &sd->bl, RA_WUGDASH, lv, timer->gettick(), SD_LEVEL);
 	}
-	return 1;
+	return;
 }
 
-//Exclusive function to Wug Dash state. [Jobbie/3CeAM]
-int unit_wugdash(struct block_list *bl, struct map_session_data *sd) {
-	struct status_change *sc = status->get_sc(bl);
+/**
+ * Makes character run, used for SC_RUN and SC_WUGDASH
+ * @param sd Required only when using SC_WUGDASH
+ * @retval true Finished running
+ * @retval false Hit an object/Couldn't run
+ **/
+bool unit_run( struct block_list *bl, struct map_session_data *sd, enum sc_type type ) {
+	struct status_change *sc;
 	short to_x,to_y,dir_x,dir_y;
-	int lv;
 	int i;
-	if (!(sc && sc->data[SC_WUGDASH]))
-		return 0;
 
-	nullpo_ret(sd);
-	nullpo_ret(bl);
+	nullpo_retr(false, bl);
+	sc = status->get_sc(bl);
 
-	if (!unit->can_move(bl)) {
-		status_change_end(bl,SC_WUGDASH,INVALID_TIMER);
-		return 0;
+	if( !(sc && sc->data[type]) )
+		return false;
+
+	if( !unit->can_move(bl) ) {
+		status_change_end(bl, type, INVALID_TIMER);
+		return false;
 	}
 
-	lv = sc->data[SC_WUGDASH]->val1;
-	dir_x = dirx[sc->data[SC_WUGDASH]->val2];
-	dir_y = diry[sc->data[SC_WUGDASH]->val2];
+	dir_x = dirx[sc->data[type]->val2];
+	dir_y = diry[sc->data[type]->val2];
 
+	// determine destination cell
 	to_x = bl->x;
 	to_y = bl->y;
-	for(i=0;i<AREA_SIZE;i++) {
+
+	// Search for available path
+	for(i = 0; i < AREA_SIZE; i++) {
 		if(!map->getcell(bl->m,to_x+dir_x,to_y+dir_y,CELL_CHKPASS))
 			break;
 
-		if(sc->data[SC_WUGDASH] && map->count_oncell(bl->m, to_x+dir_x, to_y+dir_y, BL_PC|BL_MOB|BL_NPC))
+		//if sprinting and there's a PC/Mob/NPC, block the path [Kevin]
+		if( map->count_oncell(bl->m, to_x+dir_x, to_y+dir_y, BL_PC|BL_MOB|BL_NPC) )
 			break;
 
 		to_x += dir_x;
 		to_y += dir_y;
 	}
 
-	if(to_x == bl->x && to_y == bl->y) {
+	// Can't run forward
+	if( (to_x == bl->x && to_y == bl->y ) || (to_x == (bl->x+1) || to_y == (bl->y+1)) || (to_x == (bl->x-1) || to_y == (bl->y-1))) {
+		unit->run_hit(bl, sc, sd, type);
+		return false;
+	}
 
-		unit->bl2ud(bl)->state.running = 0;
-		status_change_end(bl,SC_WUGDASH,INVALID_TIMER);
+	if( unit->walktoxy(bl, to_x, to_y, 1) )
+		return true;
 
-		if( sd ){
-			clif->fixpos(bl);
-			skill->castend_damage_id(bl, &sd->bl, RA_WUGDASH, lv, timer->gettick(), SD_LEVEL);
-		}
-		return 0;
-	}
-	if (unit->walktoxy(bl, to_x, to_y, 1))
-		return 1;
+	//There must be an obstacle nearby. Attempt walking one cell at a time.
 	do {
 		to_x -= dir_x;
 		to_y -= dir_y;
 	} while (--i > 0 && !unit->walktoxy(bl, to_x, to_y, 1));
-	if (i==0) {
 
-		unit->bl2ud(bl)->state.running = 0;
-		status_change_end(bl,SC_WUGDASH,INVALID_TIMER);
-
-		if( sd ){
-			clif->fixpos(bl);
-			skill->castend_damage_id(bl, &sd->bl, RA_WUGDASH, lv, timer->gettick(), SD_LEVEL);
-		}
-		return 0;
+	if ( i == 0 ) {
+		unit->run_hit(bl, sc, sd, type);
+		return false;
 	}
+
 	return 1;
 }
 
@@ -2620,7 +2578,7 @@ void unit_defaults(void) {
 	unit->walktobl_sub = unit_walktobl_sub;
 	unit->walktobl = unit_walktobl;
 	unit->run = unit_run;
-	unit->wugdash = unit_wugdash;
+	unit->run_hit = unit_run_hit;
 	unit->escape = unit_escape;
 	unit->movepos = unit_movepos;
 	unit->setdir = unit_setdir;
diff --git a/src/map/unit.h b/src/map/unit.h
index 9e05647b1..f29a6903a 100644
--- a/src/map/unit.h
+++ b/src/map/unit.h
@@ -83,8 +83,8 @@ struct unit_interface {
 	int (*walktoxy) (struct block_list *bl, short x, short y, int flag);
 	int (*walktobl_sub) (int tid, int64 tick, int id, intptr_t data);
 	int (*walktobl) (struct block_list *bl, struct block_list *tbl, int range, int flag);
-	int (*run) (struct block_list *bl);
-	int (*wugdash) (struct block_list *bl, struct map_session_data *sd);
+	bool (*run) (struct block_list *bl, struct map_session_data *sd, enum sc_type type);
+	void (*run_hit) (struct block_list *bl, struct status_change *sc, struct map_session_data *sd, enum sc_type type);
 	int (*escape) (struct block_list *bl, struct block_list *target, short dist);
 	int (*movepos) (struct block_list *bl, short dst_x, short dst_y, int easy, bool checkpath);
 	int (*setdir) (struct block_list *bl, unsigned char dir);