Corrected some map property issues (#3435)

* Resolves PvP timers and effects not displaying properly.
* Resolves any GvG effects not displaying properly.
* If PvP or GvG is disabled, stop players from attacking.
* Moved PvP timer and attack functions to appropriate locations.
* Removed redundancy from atcommands.

src/map/atcommand.cpp

@@ -1638,33 +1638,9 @@ ACMD_FUNC(help)
 	return 0;
 }
 
-// helper function, used in foreach calls to stop auto-attack timers
-// parameter: '0' - everyone, 'id' - only those attacking someone with that id
-static int atcommand_stopattack(struct block_list *bl,va_list ap)
-{
-	struct unit_data *ud = unit_bl2ud(bl);
-	int id = va_arg(ap, int);
-	if (ud && ud->attacktimer != INVALID_TIMER && (!id || id == ud->target))
-	{
-		unit_stop_attack(bl);
-		return 1;
-	}
-	return 0;
-}
 /*==========================================
  *
  *------------------------------------------*/
-static int atcommand_pvpoff_sub(struct block_list *bl,va_list ap)
-{
-	TBL_PC* sd = (TBL_PC*)bl;
-	clif_pvpset(sd, 0, 0, 2);
-	if (sd->pvp_timer != INVALID_TIMER) {
-		delete_timer(sd->pvp_timer, pc_calc_pvprank_timer);
-		sd->pvp_timer = INVALID_TIMER;
-	}
-	return 0;
-}
-
 ACMD_FUNC(pvpoff)
 {
 	nullpo_retr(-1, sd);
@@ -1676,11 +1652,6 @@ ACMD_FUNC(pvpoff)
 
 	map_setmapflag(sd->bl.m, MF_PVP, false);
 
-	if (!battle_config.pk_mode){
-		clif_map_property_mapall(sd->bl.m, MAPPROPERTY_NOTHING);
-	}
-	map_foreachinmap(atcommand_pvpoff_sub,sd->bl.m, BL_PC);
-	map_foreachinmap(atcommand_stopattack,sd->bl.m, BL_CHAR, 0);
 	clif_displaymessage(fd, msg_txt(sd,31)); // PvP: Off.
 	return 0;
 }
@@ -1688,20 +1659,6 @@ ACMD_FUNC(pvpoff)
 /*==========================================
  *
  *------------------------------------------*/
-static int atcommand_pvpon_sub(struct block_list *bl,va_list ap)
-{
-	TBL_PC* sd = (TBL_PC*)bl;
-	if (sd->pvp_timer == INVALID_TIMER) {
-		sd->pvp_timer = add_timer(gettick() + 200, pc_calc_pvprank_timer, sd->bl.id, 0);
-		sd->pvp_rank = 0;
-		sd->pvp_lastusers = 0;
-		sd->pvp_point = 5;
-		sd->pvp_won = 0;
-		sd->pvp_lost = 0;
-	}
-	return 0;
-}
-
 ACMD_FUNC(pvpon)
 {
 	nullpo_retr(-1, sd);
@@ -1713,11 +1670,6 @@ ACMD_FUNC(pvpon)
 
 	map_setmapflag(sd->bl.m, MF_PVP, true);
 
-	if (!battle_config.pk_mode) {// display pvp circle and rank
-		clif_map_property_mapall(sd->bl.m, MAPPROPERTY_FREEPVPZONE);
-		map_foreachinmap(atcommand_pvpon_sub,sd->bl.m, BL_PC);
-	}
-
 	clif_displaymessage(fd, msg_txt(sd,32)); // PvP: On.
 
 	return 0;
@@ -1736,8 +1688,6 @@ ACMD_FUNC(gvgoff)
 	}
 
 	map_setmapflag(sd->bl.m, MF_GVG, false);
-	clif_map_property_mapall(sd->bl.m, MAPPROPERTY_NOTHING);
-	map_foreachinmap(atcommand_stopattack,sd->bl.m, BL_CHAR, 0);
 	clif_displaymessage(fd, msg_txt(sd,33)); // GvG: Off.
 
 	return 0;
@@ -1756,7 +1706,6 @@ ACMD_FUNC(gvgon)
 	}
 
 	map_setmapflag(sd->bl.m, MF_GVG, true);
-	clif_map_property_mapall(sd->bl.m, MAPPROPERTY_AGITZONE);
 	clif_displaymessage(fd, msg_txt(sd,34)); // GvG: On.
 
 	return 0;
@@ -5394,7 +5343,7 @@ ACMD_FUNC(killable)
 		clif_displaymessage(fd, msg_txt(sd,242)); // You can now be attacked and killed by players.
 	else {
 		clif_displaymessage(fd, msg_txt(sd,288)); // You are no longer killable.
-		map_foreachinallrange(atcommand_stopattack,&sd->bl, AREA_SIZE, BL_CHAR, sd->bl.id);
+		map_foreachinallrange(unit_stopattack,&sd->bl, AREA_SIZE, BL_CHAR, sd->bl.id);
 	}
 	return 0;
 }

src/map/map.cpp

@@ -4404,12 +4404,12 @@ void map_skill_damage_add(struct map_data *m, uint16 skill_id, int rate[SKILLDMG
 }
 
 /**
- * PvP timer handling
+ * PvP timer handling (starting)
  * @param bl: Player block object
  * @param ap: func* with va_list values
  * @return 0
  */
-static int map_mapflag_pvp_sub(struct block_list *bl, va_list ap)
+static int map_mapflag_pvp_start_sub(struct block_list *bl, va_list ap)
 {
 	struct map_session_data *sd = map_id2sd(bl->id);
 
@@ -4428,6 +4428,26 @@ static int map_mapflag_pvp_sub(struct block_list *bl, va_list ap)
 	return 0;
 }
 
+/**
+ * PvP timer handling (stopping)
+ * @param bl: Player block object
+ * @param ap: func* with va_list values
+ * @return 0
+ */
+static int map_mapflag_pvp_stop_sub(struct block_list *bl, va_list ap)
+{
+	struct map_session_data* sd = map_id2sd(bl->id);
+
+	clif_pvpset(sd, 0, 0, 2);
+
+	if (sd->pvp_timer != INVALID_TIMER) {
+		delete_timer(sd->pvp_timer, pc_calc_pvprank_timer);
+		sd->pvp_timer = INVALID_TIMER;
+	}
+
+	return 0;
+}
+
 /**
  * Return the mapflag enum from the given name.
  * @param name: Mapflag name
@@ -4481,7 +4501,7 @@ bool map_getmapflag_name( enum e_mapflag mapflag, char* output ){
  */
 int map_getmapflag_sub(int16 m, enum e_mapflag mapflag, union u_mapflag_args *args)
 {
-	if (m < 0) {
+	if (m < 0 || m >= MAX_MAP_PER_SERVER) {
 		ShowWarning("map_getmapflag: Invalid map ID %d.\n", m);
 		return -1;
 	}
@@ -4531,7 +4551,7 @@ int map_getmapflag_sub(int16 m, enum e_mapflag mapflag, union u_mapflag_args *ar
  */
 bool map_setmapflag_sub(int16 m, enum e_mapflag mapflag, bool status, union u_mapflag_args *args)
 {
-	if (m < 0) {
+	if (m < 0 || m >= MAX_MAP_PER_SERVER) {
 		ShowWarning("map_setmapflag: Invalid map ID %d.\n", m);
 		return false;
 	}
@@ -4555,11 +4575,16 @@ bool map_setmapflag_sub(int16 m, enum e_mapflag mapflag, bool status, union u_ma
 			mapdata->flag[mapflag] = status;
 			break;
 		case MF_PVP:
-			if (!status)
+			mapdata->flag[mapflag] = status; // Must come first to properly set map property
+			if (!status) {
 				clif_map_property_mapall(m, MAPPROPERTY_NOTHING);
-			else {
-				if (!battle_config.pk_mode)
-					map_foreachinmap(map_mapflag_pvp_sub, m, BL_PC);
+				map_foreachinmap(map_mapflag_pvp_stop_sub, m, BL_PC);
+				map_foreachinmap(unit_stopattack, m, BL_CHAR, 0);
+			} else {
+				if (!battle_config.pk_mode) {
+					clif_map_property_mapall(m, MAPPROPERTY_FREEPVPZONE);
+					map_foreachinmap(map_mapflag_pvp_start_sub, m, BL_PC);
+				}
 				if (mapdata->flag[MF_GVG]) {
 					mapdata->flag[MF_GVG] = false;
 					ShowWarning("map_setmapflag: Unable to set GvG and PvP flags for the same map! Removing GvG flag from %s.\n", mapdata->name);
@@ -4585,13 +4610,14 @@ bool map_setmapflag_sub(int16 m, enum e_mapflag mapflag, bool status, union u_ma
 					ShowWarning("map_setmapflag: Unable to set Battleground and PvP flags for the same map! Removing Battleground flag from %s.\n", mapdata->name);
 				}
 			}
-			mapdata->flag[mapflag] = status;
 			break;
 		case MF_GVG:
 		case MF_GVG_TE:
-			if (!status)
+			mapdata->flag[mapflag] = status; // Must come first to properly set map property
+			if (!status) {
 				clif_map_property_mapall(m, MAPPROPERTY_NOTHING);
-			else {
+				map_foreachinmap(unit_stopattack, m, BL_CHAR, 0);
+			} else {
 				clif_map_property_mapall(m, MAPPROPERTY_AGITZONE);
 				if (mapdata->flag[MF_PVP]) {
 					mapdata->flag[MF_PVP] = false;
@@ -4603,7 +4629,6 @@ bool map_setmapflag_sub(int16 m, enum e_mapflag mapflag, bool status, union u_ma
 					ShowWarning("map_setmapflag: Unable to set Battleground and GvG flags for the same map! Removing Battleground flag from %s.\n", mapdata->name);
 				}
 			}
-			mapdata->flag[mapflag] = status;
 			break;
 		case MF_GVG_CASTLE:
 		case MF_GVG_TE_CASTLE:

src/map/unit.cpp

@@ -2140,6 +2140,26 @@ int unit_set_target(struct unit_data* ud, int target_id)
 	return 0;
 }
 
+/**
+ * Helper function used in foreach calls to stop auto-attack timers
+ * @param bl: Block object
+ * @param ap: func* with va_list values
+ *   Parameter: '0' - everyone, 'id' - only those attacking someone with that id
+ * @return 1 on success or 0 otherwise
+ */
+int unit_stopattack(struct block_list *bl, va_list ap)
+{
+	struct unit_data *ud = unit_bl2ud(bl);
+	int id = va_arg(ap, int);
+
+	if (ud && ud->attacktimer != INVALID_TIMER && (!id || id == ud->target)) {
+		unit_stop_attack(bl);
+		return 1;
+	}
+
+	return 0;
+}
+
 /**
  * Stop a unit's attacks
  * @param bl: Object to stop

src/map/unit.hpp

@@ -134,6 +134,7 @@ bool unit_can_reach_pos(struct block_list *bl,int x,int y,int easy);
 bool unit_can_reach_bl(struct block_list *bl,struct block_list *tbl, int range, int easy, short *x, short *y);
 
 // Unit attack functions
+int unit_stopattack(struct block_list *bl, va_list ap);
 void unit_stop_attack(struct block_list *bl);
 int unit_attack(struct block_list *src,int target_id,int continuous);
 int unit_cancel_combo(struct block_list *bl);