Code optimization and bug fixes

- Removed redundant code
- Fixed two small bugs in the distance_client calculation
- Improved the "step action" system added in the last commit
  * Added a "unit_step_timer" that is now used for all step actions; now even skills will always be executed at "full cell" (if you don't lag)
  * The "step action" system will no longer mess with anything and rather just call the normal attack/skill functions that are also called by the client; this removes a lot of unneeded code and also works a lot better since the existing functions have been bug-tested the most; this also fixes a problem with the "auto-follow if you can't reach target" system

src/map/battle.c

@@ -7424,8 +7424,7 @@ bool battle_check_range(struct block_list *src, struct block_list *bl, int range
 
 #ifndef CIRCULAR_AREA
 	if( src->type == BL_PC ) { // Range for players' attacks and skills should always have a circular check. [Angezerus]
-		int dx = src->x - bl->x, dy = src->y - bl->y;
+		if ( !check_distance_client_bl(src, bl, range) )
-		if( !check_distance_client(dx, dy, range) )
 			return false;
 	} else
 #endif

src/map/path.c

@@ -477,6 +477,8 @@ unsigned int distance(int dx, int dy)
  */
 int check_distance_client(int dx, int dy, int distance)
 {
+	if(distance < 0) distance = 0;
+
 	return (distance_client(dx,dy) <= distance);
 }
 
@@ -487,13 +489,15 @@ int check_distance_client(int dx, int dy, int distance)
  * @param dy: Vertical distance
  * @return Circular distance
  */
-unsigned int distance_client(int dx, int dy)
+int distance_client(int dx, int dy)
 {
 	double temp_dist = sqrt((double)(dx*dx + dy*dy));
 
 	//Bonus factor used by client
 	//This affects even horizontal/vertical lines so they are one cell longer than expected
-	temp_dist -= 0.625;
+	temp_dist -= 0.0625;
+
+	if(temp_dist < 0) temp_dist = 0;
 
 	return ((int)temp_dist);
 }

src/map/path.h

@@ -53,6 +53,6 @@ bool path_search_long(struct shootpath_data *spd,int16 m,int16 x0,int16 y0,int16
 int check_distance(int dx, int dy, int distance);
 unsigned int distance(int dx, int dy);
 int check_distance_client(int dx, int dy, int distance);
-unsigned int distance_client(int dx, int dy);
+int distance_client(int dx, int dy);
 
 #endif /* _PATH_H_ */

src/map/unit.c

@@ -240,6 +240,74 @@ int unit_check_start_teleport_timer(struct block_list *sbl)
 	return 0;
 }
 
+/**
+ * Triggered on full step if stepaction is true and executes remembered action.
+ * @param tid: Timer ID
+ * @param tick: Unused
+ * @param id: ID of bl to do the action
+ * @param data: Not used
+ * @return 1: Success 0: Fail (No valid bl)
+ */
+int unit_step_timer(int tid, unsigned int tick, int id, intptr_t data)
+{
+	struct block_list *bl;
+	struct unit_data *ud;
+	int target_id;
+
+	bl = map_id2bl(id);
+
+	if (!bl || bl->prev == NULL)
+		return 0;
+
+	ud = unit_bl2ud(bl);
+
+	if(!ud)
+		return 0;
+
+	if(ud->steptimer != tid) {
+		ShowError("unit_step_timer mismatch %d != %d\n",ud->steptimer,tid);
+		return 0;
+	}
+
+	ud->steptimer = INVALID_TIMER;
+
+	if(!ud->stepaction)
+		return 0;
+
+	//Set to false here because if an error occurs, it should not be executed again
+	ud->stepaction = false;
+
+	if(!ud->target_to)
+		return 0;
+
+	//Flush target_to as it might contain map coordinates which should not be used by other functions
+	target_id = ud->target_to;
+	ud->target_to = 0;
+
+	//If stepaction is set then we remembered a client request that should be executed on the next step
+	//Execute request now if target is in attack range
+	if(ud->stepskill_id && skill_get_inf(ud->stepskill_id) & INF_GROUND_SKILL) {
+		//Execute ground skill
+		struct map_data *md = &map[bl->m];			
+		unit_skilluse_pos(bl, target_id%md->xs, target_id/md->xs, ud->stepskill_id, ud->stepskill_lv);
+	} else {
+		//If a player has target_id set and target is in range, attempt attack
+		struct block_list *tbl = map_id2bl(target_id);
+		if (!tbl || !status_check_visibility(bl, tbl)) {
+			return 0;
+		}
+		if(ud->stepskill_id == 0) {
+			//Execute normal attack
+			unit_attack(bl, tbl->id, ud->state.attack_continue);
+		} else {
+			//Execute non-ground skill
+			unit_skilluse_id(bl, tbl->id, ud->stepskill_id, ud->stepskill_lv);
+		}
+	}
+
+	return 1;
+}
+
 /**
  * Defines when to refresh the walking character to object and restart the timer if applicable
  * Also checks for speed update, target location, and slave teleport timers
@@ -356,58 +424,18 @@ static int unit_walktoxy_timer(int tid, unsigned int tick, int id, intptr_t data
 		return 0;
 
 	//If stepaction is set then we remembered a client request that should be executed on the next step
-	//Execute request now if target is in attack range
 	if (ud->stepaction && ud->target_to) {
+		//Delete old stepaction even if not executed yet, the latest command is what counts
+		if(ud->steptimer != INVALID_TIMER) {
+			delete_timer(ud->steptimer, unit_step_timer);
+			ud->steptimer = INVALID_TIMER;
+		}
 		//Delay stepactions by half a step (so they are executed at full step)
 		if(ud->walkpath.path[ud->walkpath.path_pos]&1)
 			i = status_get_speed(bl)*14/20;
 		else
 			i = status_get_speed(bl)/2;
-		if(ud->stepskill_id && skill_get_inf(ud->stepskill_id) & INF_GROUND_SKILL) {
+		ud->steptimer = add_timer(tick+i, unit_step_timer, bl->id, 0);
-			//Ground skill, create imaginary target
-			struct block_list tbl;
-			struct map_data *md = &map[bl->m];			
-			tbl.type = BL_NUL;
-			tbl.m = bl->m;
-			//Convert target_to back to map coordinates
-			tbl.x = ud->target_to%md->xs;
-			tbl.y = ud->target_to/md->xs;
-			if (battle_check_range(bl, &tbl, ud->chaserange)) {
-				//Execute ground skill
-				ud->stepaction = false;
-				ud->target_to = 0;
-				unit_stop_walking(bl, 1);
-				//TODO: Delay skill use
-				unit_skilluse_pos(bl, tbl.x, tbl.y, ud->stepskill_id, ud->stepskill_lv);
-				return 0;
-			}
-		} else {
-			//If a player has target_to set and target is in range, attempt attack
-			struct block_list *tbl = map_id2bl(ud->target_to);
-			if (!tbl || !status_check_visibility(bl, tbl)) {
-				ud->target_to = 0;
-			}
-			if (battle_check_range(bl, tbl, ud->chaserange)) {
-				// Close enough to attempt an attack
-				if(ud->stepskill_id == 0) {
-					//Execute normal attack
-					ud->stepaction = false;
-					ud->target = ud->target_to;
-					ud->target_to = 0;
-					unit_stop_walking(bl, 1);
-					ud->attacktimer=add_timer(tick+i,unit_attack_timer,bl->id,0);
-					return 0;
-				} else {
-					//Execute non-ground skill
-					ud->stepaction = false;
-					ud->target_to = 0;
-					unit_stop_walking(bl, 1);
-					//TODO: Delay skill use
-					unit_skilluse_id(bl, tbl->id, ud->stepskill_id, ud->stepskill_lv);
-					return 0;
-				}
-			}
-		}
 	}
 
 	if(ud->state.change_walk_target)
@@ -1608,6 +1636,15 @@ int unit_skilluse_id2(struct block_list *src, int target_id, uint16 skill_id, ui
 	else
 		range = skill_get_range2(src, skill_id, skill_lv); // Skill cast distance from database
 
+	// New action request received, delete previous action request if not executed yet
+	if(ud->steptimer != INVALID_TIMER) {
+		delete_timer(ud->steptimer, unit_step_timer);
+		ud->steptimer = INVALID_TIMER;
+	}
+	if(ud->stepaction) {
+		ud->stepaction = false;
+		ud->target_to = 0;
+	}
 	// Remember the skill request from the client while walking to the next cell
 	if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && !battle_check_range(src, target, range-1)) {
 		ud->stepaction = true;
@@ -1616,9 +1653,6 @@ int unit_skilluse_id2(struct block_list *src, int target_id, uint16 skill_id, ui
 		ud->stepskill_id = skill_id;
 		ud->stepskill_lv = skill_lv;
 		return 0; // Attacking will be handled by unit_walktoxy_timer in this case
-	} else {
-		// To make sure a failed stepaction is not remembered any longer
-		ud->stepaction = false;
 	}
 
 	// Check range when not using skill on yourself or is a combo-skill during attack
@@ -1925,6 +1959,15 @@ int unit_skilluse_pos2( struct block_list *src, short skill_x, short skill_y, ui
 	else
 		range = skill_get_range2(src, skill_id, skill_lv); // Skill cast distance from database
 
+	// New action request received, delete previous action request if not executed yet
+	if(ud->steptimer != INVALID_TIMER) {
+		delete_timer(ud->steptimer, unit_step_timer);
+		ud->steptimer = INVALID_TIMER;
+	}
+	if(ud->stepaction) {
+		ud->stepaction = false;
+		ud->target_to = 0;
+	}
 	// Remember the skill request from the client while walking to the next cell
 	if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && !battle_check_range(src, &bl, range-1)) {
 		struct map_data *md = &map[src->m];
@@ -1935,9 +1978,6 @@ int unit_skilluse_pos2( struct block_list *src, short skill_x, short skill_y, ui
 		ud->stepskill_id = skill_id;
 		ud->stepskill_lv = skill_lv;
 		return 0; // Attacking will be handled by unit_walktoxy_timer in this case
-	} else {
-		// To make sure a failed stepaction is not remembered any longer
-		ud->stepaction = false;
 	}
 
 	if( skill_get_state(ud->skill_id) == ST_MOVE_ENABLE ) {
@@ -2141,6 +2181,15 @@ int unit_attack(struct block_list *src,int target_id,int continuous)
 	if(ud->attacktimer != INVALID_TIMER)
 		return 0;
 
+	// New action request received, delete previous action request if not executed yet
+	if(ud->steptimer != INVALID_TIMER) {
+		delete_timer(ud->steptimer, unit_step_timer);
+		ud->steptimer = INVALID_TIMER;
+	}
+	if(ud->stepaction) {
+		ud->stepaction = false;
+		ud->target_to = 0;
+	}
 	// Remember the attack request from the client while walking to the next cell
 	if(src->type == BL_PC && ud->walktimer != INVALID_TIMER && !battle_check_range(src, target, range-1)) {
 		ud->stepaction = true;
@@ -2149,9 +2198,6 @@ int unit_attack(struct block_list *src,int target_id,int continuous)
 		ud->stepskill_id = 0;
 		ud->stepskill_lv = 0;
 		return 0; // Attacking will be handled by unit_walktoxy_timer in this case
-	} else {
-		// To make sure a failed stepaction is not remembered any longer
-		ud->stepaction = false;
 	}
 	
 	if(DIFF_TICK(ud->attackabletime, gettick()) > 0) // Do attack next time it is possible. [Skotlex]
@@ -2613,6 +2659,7 @@ void unit_dataset(struct block_list *bl)
 	ud->walktimer      = INVALID_TIMER;
 	ud->skilltimer     = INVALID_TIMER;
 	ud->attacktimer    = INVALID_TIMER;
+	ud->steptimer      = INVALID_TIMER;
 	ud->attackabletime =
 	ud->canact_tick    =
 	ud->canmove_tick   = gettick();
@@ -3277,6 +3324,7 @@ void do_init_unit(void){
 	add_timer_func_list(unit_delay_walktoxy_timer,"unit_delay_walktoxy_timer");
 	add_timer_func_list(unit_delay_walktobl_timer,"unit_delay_walktobl_timer");
 	add_timer_func_list(unit_teleport_timer,"unit_teleport_timer");
+	add_timer_func_list(unit_step_timer,"unit_step_timer");
 }
 
 /**

src/map/unit.h

@@ -35,6 +35,7 @@ struct unit_data {
 	int   walktimer;
 	int   chaserange;
 	bool  stepaction; //Action should be executed on step [Playtester]
+	int   steptimer; //Timer that triggers the action [Playtester]
 	uint16 stepskill_id,stepskill_lv; //Remembers skill that should be casted on step [Playtester]
 	unsigned int attackabletime;
 	unsigned int canact_tick;
@@ -116,6 +117,9 @@ int unit_skilluse_pos(struct block_list *src, short skill_x, short skill_y, uint
 int unit_skilluse_id2(struct block_list *src, int target_id, uint16 skill_id, uint16 skill_lv, int casttime, int castcancel);
 int unit_skilluse_pos2( struct block_list *src, short skill_x, short skill_y, uint16 skill_id, uint16 skill_lv, int casttime, int castcancel);
 
+// Step timer used for delayed attack and skill use
+int unit_step_timer(int tid, unsigned int tick, int id, intptr_t data);
+
 // Cancel unit cast
 int unit_skillcastcancel(struct block_list *bl, char type);