* Added a safeguard to skill_get_unit_layout() against incorrectly defined layout ids (will give weird results but won't crash at least)

* Fixed Firewall/Icewall being oriented backwards (no real difference though...)
* Removed some junk Landprotector/Graffiti code; Graffiti doesn't get placed randomly anymore
* Icewall can now be cast on yourself... but for some reason, the cell on yourself immediately expires!
* Merged together functions clif_set0192() and clif_changemapcell()
- also removed its "send to whole map" mode which is just plain wrong (although aegis actually does use it to 'inform' caster about changes)

git-svn-id: https://rathena.svn.sourceforge.net/svnroot/rathena/trunk@11175 54d463be-8e91-2dee-dedb-b68131a5f0ec

6 files changed, 170 insertions, 159 deletions

diff --git a/Changelog-Trunk.txt b/Changelog-Trunk.txt
index 0ade30dee..9b57bba16 100644
--- a/Changelog-Trunk.txt
+++ b/Changelog-Trunk.txt
@@ -4,6 +4,16 @@ AS OF SVN REV. 5091, WE ARE NOW USING TRUNK.  ALL UNTESTED BUGFIXES/FEATURES GO
 IF YOU HAVE A WORKING AND TESTED BUGFIX PUT IT INTO STABLE AS WELL AS TRUNK.
 
 2007/09/10
+	* Added a safeguard to skill_get_unit_layout() against incorrectly
+	  defined layout ids (will give weird results but won't crash at least)
+	* Fixed Firewall/Icewall being oriented backwards (no real difference..)
+	* Removed some junk Landprotector/Graffiti code
+	- Graffiti doesn't get placed randomly anymore
+	* Icewall can now be cast on yourself
+	- TODO: for some reason, the cell on yourself immediately expires...
+	* Merged together functions clif_set0192() and clif_changemapcell()
+	- also removed its "send to whole map" mode which is just plain wrong
+	  (although aegis actually does use it to 'inform' caster about changes)
 	* Added config option to adjust exp gained by 'getexp' [ultramage]
 2007/09/09
 	* Removed bonus bAddEffWhenHitShort as it is unneeded and unused.
diff --git a/src/map/clif.c b/src/map/clif.c
index 16b6f19f0..d95495c1a 100644
--- a/src/map/clif.c
+++ b/src/map/clif.c
@@ -640,7 +640,7 @@ int clif_clearflooritem(struct flooritem_data *fitem, int fd)
 		clif_send(buf, packet_len(0xa1), &fitem->bl, AREA);
 	} else {
 		WFIFOHEAD(fd,packet_len(0xa1));
-		memcpy(WFIFOP(fd,0), buf, 6);
+		memcpy(WFIFOP(fd,0), buf, packet_len(0xa1));
 		WFIFOSET(fd,packet_len(0xa1));
 	}
 
@@ -1184,20 +1184,6 @@ static void clif_spiritball_single(int fd, struct map_session_data *sd)
 	WFIFOSET(fd, packet_len(0x1e1));
 }
 
-/*==========================================
- *
- *------------------------------------------*/
-static void clif_set0192(int fd, int m, int x, int y, int type)
-{
-	WFIFOHEAD(fd,packet_len(0x192));
-	WFIFOW(fd,0) = 0x192;
-	WFIFOW(fd,2) = x;
-	WFIFOW(fd,4) = y;
-	WFIFOW(fd,6) = type;
-	mapindex_getmapname_ext(map[m].name, (char*)WFIFOP(fd,8));
-	WFIFOSET(fd,packet_len(0x192));
-}
-
 // new and improved weather display [Valaris]
 static void clif_weather_sub(int fd, int id, int type)
 {
@@ -3908,6 +3894,33 @@ void clif_standing(struct block_list* bl)
 /*==========================================
  *
  *------------------------------------------*/
+void clif_changemapcell(int fd, short m, short x, short y, int type)
+{
+	unsigned char buf[32];
+
+	WBUFW(buf,0) = 0x192;
+	WBUFW(buf,2) = x;
+	WBUFW(buf,4) = y;
+	WBUFW(buf,6) = type;
+	mapindex_getmapname_ext(map[m].name,(char*)WBUFP(buf,8));
+
+	if (fd == 0) {
+		struct block_list bl;
+		bl.type = BL_NUL;
+		bl.m = m;
+		bl.x = x;
+		bl.y = y;
+		clif_send(buf,packet_len(0x192),&bl,AREA);
+	} else {
+		WFIFOHEAD(fd,packet_len(0x192));
+		memcpy(WFIFOP(fd,0), buf, packet_len(0x192));
+		WFIFOSET(fd,packet_len(0x192));
+	}
+}
+
+/*==========================================
+ *
+ *------------------------------------------*/
 void clif_getareachar_item(struct map_session_data* sd,struct flooritem_data* fitem)
 {
 	int view,fd;
@@ -3948,7 +3961,7 @@ int clif_getareachar_skillunit(struct map_session_data *sd,struct skill_unit *un
 		WFIFOL(fd, 2)=unit->bl.id;
 		WFIFOL(fd, 6)=unit->group->src_id;
 		WFIFOW(fd,10)=unit->bl.x;
-		WFIFOW(fd,12)=unit->bl.y; // might be typo? [Lance]
+		WFIFOW(fd,12)=unit->bl.y;
 		WFIFOB(fd,14)=unit->group->unit_id;
 		WFIFOB(fd,15)=1;
 		WFIFOB(fd,16)=1;
@@ -3973,7 +3986,7 @@ int clif_getareachar_skillunit(struct map_session_data *sd,struct skill_unit *un
 	WFIFOSET(fd,packet_len(0x11f));
 
 	if(unit->group->skill_id == WZ_ICEWALL)
-		clif_set0192(fd,unit->bl.m,unit->bl.x,unit->bl.y,5);
+		clif_changemapcell(fd,unit->bl.m,unit->bl.x,unit->bl.y,5);
 	return 0;
 /* Previous implementation guess of packet 0x1c9, who can understand what all those fields are for? [Skotlex]
 	WFIFOHEAD(fd,packet_len(0x1c9));
@@ -4034,7 +4047,7 @@ int clif_clearchar_skillunit(struct skill_unit *unit,int fd)
 	WFIFOL(fd, 2)=unit->bl.id;
 	WFIFOSET(fd,packet_len(0x120));
 	if(unit->group && unit->group->skill_id == WZ_ICEWALL)
-		clif_set0192(fd,unit->bl.m,unit->bl.x,unit->bl.y,unit->val2);
+		clif_changemapcell(fd,unit->bl.m,unit->bl.x,unit->bl.y,unit->val2);
 
 	return 0;
 }
@@ -6355,26 +6368,6 @@ int clif_bladestop(struct block_list *src,struct block_list *dst,
 }
 
 /*==========================================
- *
- *------------------------------------------*/
-void clif_changemapcell(short m, short x, short y, int cell_type, int type)
-{
-	struct block_list bl;
-	unsigned char buf[32];
-
-	bl.type = BL_NUL;
-	bl.m = m;
-	bl.x = x;
-	bl.y = y;
-	WBUFW(buf,0) = 0x192;
-	WBUFW(buf,2) = x;
-	WBUFW(buf,4) = y;
-	WBUFW(buf,6) = cell_type;
-	mapindex_getmapname_ext(map[m].name,(char*)WBUFP(buf,8));
-	clif_send(buf,packet_len(0x192),&bl,(!type)?AREA:ALL_SAMEMAP);
-}
-
-/*==========================================
  * MVPエフェクト
  *------------------------------------------*/
 int clif_mvp_effect(struct map_session_data *sd)
diff --git a/src/map/clif.h b/src/map/clif.h
index 54a643b85..db8dd92dd 100644
--- a/src/map/clif.h
+++ b/src/map/clif.h
@@ -229,7 +229,7 @@ int clif_marionette(struct block_list *src, struct block_list *target);
 int clif_spiritball(struct map_session_data *sd);
 int clif_combo_delay(struct block_list *src,int wait);
 int clif_bladestop(struct block_list *src,struct block_list *dst,int bool_);
-void clif_changemapcell(short m, short x, short y, int cell_type, int type);
+void clif_changemapcell(int fd, short m, short x, short y, int type);
 
 int clif_status_load(struct block_list *bl,int type, int flag);
 int clif_status_change(struct block_list *bl,int type,int flag);
diff --git a/src/map/map.c b/src/map/map.c
index 42a22d120..fcb1a58ac 100644
--- a/src/map/map.c
+++ b/src/map/map.c
@@ -2150,8 +2150,8 @@ int map_calc_dir(struct block_list* src, int x, int y)
 	dy = y-src->y;
 	if( dx == 0 && dy == 0 )
 	{	// both are standing on the same spot
-		//dir = 6; // aegis-style, causes knockback to the left
-		dir = unit_getdir(src); // athena-style, causes knockback opposite to src's current direction
+		//dir = 6; // aegis-style, makes knockback default to the left
+		dir = unit_getdir(src); // athena-style, makes knockback default to behind 'src'
 	}
 	else if( dx >= 0 && dy >=0 )
 	{	// upper-right
diff --git a/src/map/skill.c b/src/map/skill.c
index 27701cf40..f27d29298 100644
--- a/src/map/skill.c
+++ b/src/map/skill.c
@@ -6,6 +6,7 @@
 #include "../common/nullpo.h"
 #include "../common/malloc.h"
 #include "../common/showmsg.h"
+#include "../common/strlib.h"
 #include "../common/ers.h"
 
 #include "skill.h"
@@ -14,7 +15,7 @@
 #include "pc.h"
 #include "status.h"
 #include "pet.h"
-#include "mercenary.h"	//[orn]
+#include "mercenary.h"
 #include "mob.h"
 #include "npc.h"
 #include "battle.h"
@@ -666,6 +667,10 @@ struct skill_produce_db skill_produce_db[MAX_SKILL_PRODUCE_DB];
 struct skill_arrow_db skill_arrow_db[MAX_SKILL_ARROW_DB];
 struct skill_abra_db skill_abra_db[MAX_SKILL_ABRA_DB];
 
+struct skill_unit_layout skill_unit_layout[MAX_SKILL_UNIT_LAYOUT];
+int firewall_unit_pos;
+int icewall_unit_pos;
+
 // macros to check for out of bounds errors [celest]
 // i: Skill ID, l: Skill Level, var: Value to return after checking
 // for values that don't require level just put a one (putting 0 will trigger return 0; instead
@@ -986,30 +991,28 @@ int skillnotok_hom (int skillid, struct homun_data *hd)
 	return skillnotok(skillid, hd->master);
 }
 
-struct skill_unit_layout skill_unit_layout[MAX_SKILL_UNIT_LAYOUT];
-int firewall_unit_pos;
-int icewall_unit_pos;
-
-struct skill_unit_layout *skill_get_unit_layout (int skillid, int skilllv, struct block_list *src, int x, int y)
+struct skill_unit_layout* skill_get_unit_layout (int skillid, int skilllv, struct block_list* src, int x, int y)
 {	
 	int pos = skill_get_unit_layout_type(skillid,skilllv);
 	int dir;
 
-	if (pos != -1)
+	if (pos < -1 || pos >= MAX_SKILL_UNIT_LAYOUT) {
+		ShowError("skill_get_unit_layout: unsupported layout type %d for skill %d (level %d)\n", pos, skillid, skilllv);
+		pos = cap_value(pos, 0, MAX_SQUARE_LAYOUT); // cap to nearest square layout
+	}
+
+	if (pos != -1) // simple single-definition layout
 		return &skill_unit_layout[pos];
 
-	if (src->x == x && src->y == y)
-		dir = 2;
-	else
-		dir = map_calc_dir(src,x,y);
+	dir = (src->x == x && src->y == y) ? 6 : map_calc_dir(src,x,y); // 6 - default aegis direction
 
 	if (skillid == MG_FIREWALL)
 		return &skill_unit_layout [firewall_unit_pos + dir];
 	else if (skillid == WZ_ICEWALL)
 		return &skill_unit_layout [icewall_unit_pos + dir];
 
-	ShowError("Unknown unit layout for skill %d, %d\n",skillid,skilllv);
-	return &skill_unit_layout[0];
+	ShowError("skill_get_unit_layout: unknown unit layout for skill %d (level %d)\n", skillid, skilllv);
+	return &skill_unit_layout[0]; // default 1x1 layout
 }
 
 /*==========================================
@@ -2341,8 +2344,7 @@ static int skill_check_unit_range (struct block_list *bl, int x, int y, int skil
 	}
 
 	range += layout_type;
-	return map_foreachinarea(skill_check_unit_range_sub,bl->m,
-			x-range,y-range,x+range,y+range,BL_SKILL,skillid);
+	return map_foreachinarea(skill_check_unit_range_sub,bl->m,x-range,y-range,x+range,y+range,BL_SKILL,skillid);
 }
 
 static int skill_check_unit_range2_sub (struct block_list *bl, va_list ap)
@@ -5959,21 +5961,22 @@ int skill_castend_pos (int tid, unsigned int tick, int id, int data)
 
 	ud->skilltimer=-1;
 	do {
-		if(status_isdead(src)) break;
+		if(status_isdead(src))
+			break;
 
-		if (!(battle_config.skill_reiteration && src->type&battle_config.skill_reiteration) &&
+		if( !(src->type&battle_config.skill_reiteration) &&
 			skill_get_unit_flag(ud->skillid)&UF_NOREITERATION &&
 			skill_check_unit_range(src,ud->skillx,ud->skilly,ud->skillid,ud->skilllv)
-		)
+		  ) 
 			break;
 
-		if (battle_config.skill_nofootset && src->type&battle_config.skill_nofootset &&
+		if( src->type&battle_config.skill_nofootset &&
 			skill_get_unit_flag(ud->skillid)&UF_NOFOOTSET &&
 			skill_check_unit_range2(src,ud->skillx,ud->skilly,ud->skillid,ud->skilllv)
-		)
+		  )
 			break;
 		
-		if(battle_config.land_skill_limit && src->type&battle_config.land_skill_limit &&
+		if( src->type&battle_config.land_skill_limit &&
 			(maxcount = skill_get_maxcount(ud->skillid, ud->skilllv)) > 0
 		  ) {
 			int i;
@@ -5981,7 +5984,7 @@ int skill_castend_pos (int tid, unsigned int tick, int id, int data)
 				if(ud->skillunit[i]->skill_id == ud->skillid)
 					maxcount--;
 			}
-			if(!maxcount)
+			if( maxcount == 0 )
 				break;
 		}
 
@@ -6647,7 +6650,6 @@ struct skill_unit_group *skill_unitsetting (struct block_list *src, int skillid,
 {
 	struct skill_unit_group *group;
 	int i,limit,val1=0,val2=0,val3=0;
-	int count=0;
 	int target,interval,range,unit_flag;
 	struct skill_unit_layout *layout;
 	struct map_session_data *sd;
@@ -6737,13 +6739,6 @@ struct skill_unit_group *skill_unitsetting (struct block_list *src, int skillid,
 		break;
 
 	case SA_LANDPROTECTOR:
-		{
-			int aoe_diameter;	// -- aoe_diameter (moonsoul) added for sage Area Of Effect skills
-			val1=skilllv*15+10;
-			aoe_diameter=skilllv+skilllv%2+5;
-			count=aoe_diameter*aoe_diameter;	// -- this will not function if changed to ^2 (moonsoul)
-		}
-	//No break because we also have to check if we use gemstones. [Skotlex]
 	case SA_VOLCANO:
 	case SA_DELUGE:
 	case SA_VIOLENTGALE:
@@ -6845,9 +6840,6 @@ struct skill_unit_group *skill_unitsetting (struct block_list *src, int skillid,
 		val1 = 55 + skilllv*5;	//Elemental Resistance
 		val2 = skilllv*10;	//Status ailment resistance
 		break;
-	case RG_GRAFFITI:			/* Graffiti */
-		count=1;	// Leave this at 1 [Valaris]
-		break;
 	case WE_CALLPARTNER:
 		if (sd) val1 = sd->status.partner_id;
 		break;
@@ -6894,8 +6886,7 @@ struct skill_unit_group *skill_unitsetting (struct block_list *src, int skillid,
 		}
 	}
 
-	nullpo_retr(NULL, group=skill_initunitgroup(src,(count > 0 ? count : layout->count),
-		skillid,skilllv,skill_get_unit_id(skillid,flag&1)+subunt, limit, interval));
+	nullpo_retr(NULL, group=skill_initunitgroup(src,layout->count,skillid,skilllv,skill_get_unit_id(skillid,flag&1)+subunt, limit, interval));
 	group->val1=val1;
 	group->val2=val2;
 	group->val3=val3;
@@ -6903,47 +6894,39 @@ struct skill_unit_group *skill_unitsetting (struct block_list *src, int skillid,
 	group->bl_flag= skill_get_unit_bl_target(skillid);
 	group->state.into_abyss = (sc && sc->data[SC_INTOABYSS].timer != -1); //Store into abyss state, to know it shouldn't give traps back. [Skotlex]
 	group->state.magic_power = (flag&2 || (sc && sc->data[SC_MAGICPOWER].timer != -1)); //Store the magic power flag. [Skotlex]
-	//Store if this skill needs to consume ammo.
-	group->state.ammo_consume = (sd && sd->state.arrow_atk && skillid != GS_GROUNDDRIFT);
+	group->state.ammo_consume = (sd && sd->state.arrow_atk && skillid != GS_GROUNDDRIFT); //Store if this skill needs to consume ammo.
 	group->state.song_dance = (unit_flag&(UF_DANCE|UF_SONG)?1:0)|(unit_flag&UF_ENSEMBLE?2:0); //Signals if this is a song/dance/duet
 
   	//if tick is greater than current, do not invoke onplace function just yet. [Skotlex]
 	if (DIFF_TICK(group->tick, gettick()) > 100)
 		active_flag = 0;
 
-	if(skillid==HT_TALKIEBOX ||
-	   skillid==RG_GRAFFITI){
+	if(skillid==HT_TALKIEBOX || skillid==RG_GRAFFITI){
 		group->valstr=(char *) aMalloc(MESSAGE_SIZE*sizeof(char));
 		if (sd)
-			memcpy(group->valstr,sd->message,MESSAGE_SIZE);
+			safestrncpy(group->valstr, sd->message, MESSAGE_SIZE);
 		else //Eh... we have to write something here... even though mobs shouldn't use this. [Skotlex]
-			strcpy(group->valstr, "Boo!");
+			safestrncpy(group->valstr, "Boo!", MESSAGE_SIZE);
 	}
 
 	//Why redefine local variables when the ones of the function can be reused? [Skotlex]
 	val1=skilllv;
 	val2=0;
 	limit=group->limit;
-	count=group->unit_count;
-	for(i=0;i<count;i++){
+	for(i=0;i<layout->count;i++)
+	{
 		struct skill_unit *unit;
 		int ux,uy,alive=1;
 		ux = x + layout->dx[i];
 		uy = y + layout->dy[i];
+
 		switch (skillid) {
 		case MG_FIREWALL:
 		case NJ_KAENSIN:
 			val2=group->val2;
 			break;
 		case WZ_ICEWALL:
-			if(skilllv <= 1)
-				val1 = 500;
-			else
-				val1 = 200 + 200*skilllv;
-			break;
-		case RG_GRAFFITI:	/* Graffiti [Valaris] */
-			ux+=(i%5-2);
-			uy+=(i/5-2);
+			val1 = (skilllv <= 1) ? 500 : 200 + 200*skilllv;
 			break;
 		case GS_DESPERADO:
 			val1 = abs(layout->dx[i]);
@@ -6968,25 +6951,19 @@ struct skill_unit_group *skill_unitsetting (struct block_list *src, int skillid,
 		if(alive && map_getcell(src->m,ux,uy,CELL_CHKWALL))
 			alive = 0;
 		
-		if (alive && battle_config.skill_wall_check) {
-			//Check if there's a path between cell and center of casting.
-			if (!path_search_long(NULL,src->m,ux,uy,x,y))
-				alive = 0;
-		}
+		if(alive && battle_config.skill_wall_check && !path_search_long(NULL,src->m,ux,uy,x,y))
+			alive = 0; //no path between cell and center of casting.
 					
 		if(alive && skillid == WZ_ICEWALL) {
-			if(src->x == x && src->y==y) // Ice Wall not allowed on self [DracoRPG]
-				alive=0;
-			else {
-				val2=map_getcell(src->m,ux,uy,CELL_GETTYPE);
-				if(val2==5 || val2==1)
+				int celltype = map_getcell(src->m,ux,uy,CELL_GETTYPE);
+				if(celltype==5 || celltype==1)
 					alive=0;
 				else
-					clif_changemapcell(src->m,ux,uy,5,0);
-			}
+					clif_changemapcell(0,src->m,ux,uy,5);
 		}
 
 		if(alive){
+			//FIXME: why not calculate val1/val2 in here? [ultramage]
 			nullpo_retr(NULL, unit=skill_initunit(group,i,ux,uy,val1,val2));
 			unit->limit=limit;
 			unit->range=range;
@@ -7681,8 +7658,7 @@ static int skill_unit_onleft (int skill_id, struct block_list *bl, unsigned int
 		case BD_ROKISWEIL:
 		case BD_INTOABYSS:
 		case BD_SIEGFRIED:
-			if(sc && sc->data[SC_DANCING].timer != -1 &&
-				(sc->data[SC_DANCING].val1&0xFFFF) == skill_id)
+			if(sc && sc->data[SC_DANCING].timer != -1 && (sc->data[SC_DANCING].val1&0xFFFF) == skill_id)
 			{	//Check if you just stepped out of your ensemble skill to cancel dancing. [Skotlex]
 				//We don't check for SC_LONGING because someone could always have knocked you back and out of the song/dance.
 				//FIXME: This code is not perfect, it doesn't checks for the real ensemble's owner,
@@ -7735,12 +7711,12 @@ static int skill_unit_onleft (int skill_id, struct block_list *bl, unsigned int
 				}
 			}
 			break;
-	case UNT_GOSPEL:
-		if (sc && sc->data[type].timer != -1 && sc->data[type].val4 == BCT_ALL) //End item-no-use Gospel Effect. [Skotlex]
-			status_change_end(bl, type, -1);
-		break;
-
+		case UNT_GOSPEL:
+			if (sc && sc->data[type].timer != -1 && sc->data[type].val4 == BCT_ALL) //End item-no-use Gospel Effect. [Skotlex]
+				status_change_end(bl, type, -1);
+			break;
 	}
+
 	return skill_id;
 }
 
@@ -7811,7 +7787,7 @@ int skill_unit_onlimit (struct skill_unit *src, unsigned int tick, int flag)
 		break;
 
 	case UNT_ICEWALL:
-		clif_changemapcell(src->bl.m,src->bl.x,src->bl.y,src->val2,1);
+		clif_changemapcell(0,src->bl.m,src->bl.x,src->bl.y,src->val2);
 		break;
 	case UNT_CALLFAMILY:
 		if (!flag)
@@ -9904,11 +9880,13 @@ int skill_delunit (struct skill_unit *unit, int flag)
 		return 0;
 	nullpo_retr(0, group=unit->group);
 
+	// invoke onlimit event
 	skill_unit_onlimit( unit,gettick(), flag);
 
 	if (group->state.song_dance&0x1) //Restore dissonance effect.
 		skill_dance_overlap(unit, 0);
 
+	// invoke onout event
 	if (!unit->range) {
 		map_foreachincell(skill_unit_effect,unit->bl.m,
 			unit->bl.x,unit->bl.y,group->bl_flag,&unit->bl,gettick(),4);
@@ -9946,10 +9924,11 @@ int skill_delunit (struct skill_unit *unit, int flag)
  *
  *------------------------------------------*/
 static int skill_unit_group_newid = MAX_SKILL_DB;
+
 struct skill_unit_group *skill_initunitgroup (struct block_list *src, int count, int skillid, int skilllv, int unit_id, int limit, int interval)
 {
-	struct unit_data *ud = unit_bl2ud(src);
-	struct skill_unit_group *group=NULL;
+	struct unit_data* ud = unit_bl2ud( src );
+	struct skill_unit_group* group;
 	int i;
 
 	if(skilllv <= 0) return 0;
@@ -9957,9 +9936,11 @@ struct skill_unit_group *skill_initunitgroup (struct block_list *src, int count,
 	nullpo_retr(NULL, src);
 	nullpo_retr(NULL, ud);
 	
-	for(i=0;i<MAX_SKILLUNITGROUP && ud->skillunit[i]; i++);
-	
-	if(i == MAX_SKILLUNITGROUP) {
+	// find a free spot to store the new unit group
+	ARR_FIND( 0, MAX_SKILLUNITGROUP, i, ud->skillunit[i] == NULL );
+	if(i == MAX_SKILLUNITGROUP)
+	{
+		// array is full, make room by discarding oldest group
 		int j=0;
 		unsigned maxdiff=0,x,tick=gettick();
 		for(i=0;i<MAX_SKILLUNITGROUP && ud->skillunit[i];i++)
@@ -9971,30 +9952,35 @@ struct skill_unit_group *skill_initunitgroup (struct block_list *src, int count,
 		//Since elements must have shifted, we use the last slot.
 		i = MAX_SKILLUNITGROUP-1;
 	}
-	if (!ud->skillunit[i])
-		ud->skillunit[i] = ers_alloc(skill_unit_ers, struct skill_unit_group);
-	group=ud->skillunit[i];
 
-	group->src_id=src->id;
-	group->party_id=status_get_party_id(src);
-	group->guild_id=status_get_guild_id(src);
-	group->group_id=skill_unit_group_newid++;
+	group             = ers_alloc(skill_unit_ers, struct skill_unit_group);
+	group->src_id     = src->id;
+	group->party_id   = status_get_party_id(src);
+	group->guild_id   = status_get_guild_id(src);
+	group->group_id   = skill_unit_group_newid++;
+	group->unit       = (struct skill_unit *)aCalloc(count,sizeof(struct skill_unit));
+	group->unit_count = count;
+	group->alive_count = 0;
+	group->val1       = 0;
+	group->val2       = 0;
+	group->val3       = 0;
+	group->skill_id   = skillid;
+	group->skill_lv   = skilllv;
+	group->unit_id    = unit_id;
+	group->map        = src->m;
+	group->limit      = limit;
+	group->interval   = interval;
+	group->tick       = gettick();
+	group->valstr     = NULL;
+
+	ud->skillunit[i] = group;
+
 	if(skill_unit_group_newid<=0)
 		skill_unit_group_newid = MAX_SKILL_DB;
-	group->unit=(struct skill_unit *)aCalloc(count,sizeof(struct skill_unit));
-	group->unit_count=count;
-	group->alive_count=0;
-	group->val1=group->val2=group->val3=0;
-	group->skill_id=skillid;
-	group->skill_lv=skilllv;
-	group->unit_id=unit_id;
-	group->map=src->m;
-	group->limit=limit;
-	group->interval=interval;
-	group->tick=gettick();
+
 	if (skillid == PR_SANCTUARY) //Sanctuary starts healing +1500ms after casted. [Skotlex]
 		group->tick += 1500;
-	group->valstr=NULL;
+
 	return group;
 }
 
@@ -10060,16 +10046,18 @@ int skill_delunitgroup (struct block_list *src, struct skill_unit_group *group,
 	group->group_id=0;
 	group->unit_count=0;
 
-	//Locate and clear this unit from the array.
-	for (i=0; i<MAX_SKILLUNITGROUP && ud->skillunit[i]!=group; i++);
-	for (j=i; j<MAX_SKILLUNITGROUP && ud->skillunit[j]; j++);
-	j--;
-	if (i<MAX_SKILLUNITGROUP) {
+	// locate this group, swap with the last entry and delete it
+	ARR_FIND( 0, MAX_SKILLUNITGROUP, i, ud->skillunit[i] == group );
+	ARR_FIND( i, MAX_SKILLUNITGROUP, j, ud->skillunit[j] == NULL ); j--;
+	if( i < MAX_SKILLUNITGROUP )
+	{
 		ud->skillunit[i] = ud->skillunit[j];
 		ud->skillunit[j] = NULL;
 		ers_free(skill_unit_ers, group);
-	} else
+	}
+	else
 		ShowError("skill_delunitgroup: Group not found! (src_id: %d skill_id: %d)\n", group->src_id, group->skill_id);
+
 	return 1;
 }
 
@@ -10392,13 +10380,12 @@ int skill_unit_move (struct block_list *bl, unsigned int tick, int flag)
 		skill_unit_index=0;
 	}
 		
-	map_foreachincell(skill_unit_move_sub,
-			bl->m,bl->x,bl->y,BL_SKILL,bl,tick,flag);
+	map_foreachincell(skill_unit_move_sub,bl->m,bl->x,bl->y,BL_SKILL,bl,tick,flag);
 
 	if (flag&2 && flag&1)
 	{ //Onplace, check any skill units you have left.
 		int i;
-		for (i=0; i < (sizeof(skill_unit_temp)/sizeof(int)) && skill_unit_temp[i]; i++)
+		for (i=0; i < ARRAYLENGTH(skill_unit_temp) && skill_unit_temp[i]; i++)
 			skill_unit_onleft(skill_unit_temp[i], bl, tick);
 	}
 
@@ -11077,6 +11064,8 @@ void skill_init_unit_layout (void)
 	int i,j,size,pos = 0;
 
 	memset(skill_unit_layout,0,sizeof(skill_unit_layout));
+
+	// standard square layouts go first
 	for (i=0; i<=MAX_SQUARE_LAYOUT; i++) {
 		size = i*2+1;
 		skill_unit_layout[i].count = size*size;
@@ -11085,6 +11074,8 @@ void skill_init_unit_layout (void)
 			skill_unit_layout[i].dy[j] = (j/size-i);
 		}
 	}
+
+	// afterwards add special ones
 	pos = i;
 	for (i=0;i<MAX_SKILL_DB;i++) {
 		if (!skill_db[i].unit_id[0] || skill_db[i].unit_layout_type[0] != -1)
@@ -11092,6 +11083,7 @@ void skill_init_unit_layout (void)
 		switch (i) {
 			case MG_FIREWALL:
 			case WZ_ICEWALL:
+				// these will be handled later
 				break;
 			case PR_SANCTUARY:
 			case NPC_EVILLAND:
@@ -11234,6 +11226,8 @@ void skill_init_unit_layout (void)
 			skill_db[i].unit_layout_type[j] = pos;
 		pos++;
 	}
+
+	// firewall and icewall have 8 layouts (direction-dependent)
 	firewall_unit_pos = pos;
 	for (i=0;i<8;i++) {
 		if (i&1) {
diff --git a/src/map/skill.h b/src/map/skill.h
index 5195c728d..3f3420628 100644
--- a/src/map/skill.h
+++ b/src/map/skill.h
@@ -98,17 +98,17 @@ struct skill_unit_layout {
 };
 
 enum {
-	UF_DEFNOTENEMY		= 0x0001,	// defnotenemy 設定でBCT_NOENEMYに切り替え
-	UF_NOREITERATION	= 0x0002,	// 重複置き禁止 
-	UF_NOFOOTSET		= 0x0004,	// 足元置き禁止
-	UF_NOOVERLAP		= 0x0008,	// ユニット効果が重複しない
-	UF_NOPC		= 0x0010,	//May not target players
-	UF_NOMOB		= 0x0020,	//May not target mobs
-	UF_SKILL		= 0x0080,	//May target skills
-	UF_DANCE			= 0x0100,	//Dance
-	UF_ENSEMBLE			= 0x0200,	//Duet
-	UF_SONG		= 0x0400,	//Song
-	UF_DUALMODE	= 0x0800,	//Spells should trigger both ontimer and onplace/onout/onleft effects.
+	UF_DEFNOTENEMY   = 0x0001,	// defnotenemy 設定でBCT_NOENEMYに切り替え
+	UF_NOREITERATION = 0x0002,	// 重複置き禁止 
+	UF_NOFOOTSET     = 0x0004,	// 足元置き禁止
+	UF_NOOVERLAP     = 0x0008,	// ユニット効果が重複しない
+	UF_NOPC          = 0x0010,	//May not target players
+	UF_NOMOB         = 0x0020,	//May not target mobs
+	UF_SKILL         = 0x0080,	//May target skills
+	UF_DANCE         = 0x0100,	//Dance
+	UF_ENSEMBLE      = 0x0200,	//Duet
+	UF_SONG          = 0x0400,	//Song
+	UF_DUALMODE      = 0x0800,	//Spells should trigger both ontimer and onplace/onout/onleft effects.
 };
 
 // アイテム作成デ?タベ?ス
@@ -274,8 +274,20 @@ int skill_attack( int attack_type, struct block_list* src, struct block_list *ds
 void skill_reload(void);
 
 enum {
-	ST_NONE,ST_HIDING,ST_CLOAKING,ST_HIDDEN,ST_RIDING,ST_FALCON,ST_CART,ST_SHIELD,ST_SIGHT,ST_EXPLOSIONSPIRITS,ST_CARTBOOST,
-	ST_RECOV_WEIGHT_RATE,ST_MOVE_ENABLE,ST_WATER,
+	ST_NONE,
+	ST_HIDING,
+	ST_CLOAKING,
+	ST_HIDDEN,
+	ST_RIDING,
+	ST_FALCON,
+	ST_CART,
+	ST_SHIELD,
+	ST_SIGHT,
+	ST_EXPLOSIONSPIRITS,
+	ST_CARTBOOST,
+	ST_RECOV_WEIGHT_RATE,
+	ST_MOVE_ENABLE,
+	ST_WATER,
 };
 
 enum _skill {
@@ -933,7 +945,9 @@ enum {
 	UNT_ATTACK_SKILLS, //These show no effect on the client, therefore can be used for attack skills.
 	UNT_FIREPILLAR_WAITING,
 	UNT_FIREPILLAR_ACTIVE,
-	//0x89, 0x8a, 0x8b
+	//0x89
+	//0x8a
+	//0x8b
 	UNT_USED_TRAPS = 0x8c,
 	UNT_ICEWALL,
 	UNT_QUAGMIRE,