Corrects skill_damage_db parsing (#4667)

* Fixes #4574.
* Removes the usage of atoi as any failure returns 0.
* Adds checks for cases of invalid data.
* General cleanups and fixes.
* Fixes the import database documentation.
Thanks to @Skormie!

db/import-tmpl/skill_damage_db.txt

@@ -4,12 +4,12 @@
 // SkillName,Caster,Map,Damage against Players{,Damage against Mobs{,Damage against Bosses{,Damage against Other}}}
 //
 // Caster: The groups for which the adjustment takes effect. (bitmask)
-//		 1 = Player
+//		BL_PC = Player
-//		 2 = Monster
+//		BL_MOB = Monster
-//		 4 = Pet
+//		BL_PET = Pet
-//		 8 = Homunculus
+//		BL_HOM = Homunculus
-//		16 = Mercenary
+//		BL_MER = Mercenary
-//		32 = Elemental
+//		BL_ELEM = Elemental
 //
 // Map:
 // 		1    - Normal (the maps that aren't classified as these maps below)
@@ -31,7 +31,7 @@
 //  	Negative values decrease damage and positive values increase it (0 = no change).
 //
 // Examples:
-// MC_MAMMONITE,1,1,50 // In normal maps, players deal +50% damage to other players with Mammonite.
+// MC_MAMMONITE,BL_PC,1,50 // In normal maps, players deal +50% damage to other players with Mammonite.
-// MO_EXTREMITYFIST,1,6,-50 // In PVP and GVG, players deal -50% (half) damage to other players with Asura Strike.
+// MO_EXTREMITYFIST,BL_PC,6,-50 // In PVP and GVG, players deal -50% (half) damage to other players with Asura Strike.
-// AB_ADORAMUS,1,6,50,0,10,15 // In PVP and GVG, players deal +50% damage to other players, +0% to mobs, +10% to bosses, and +15% to other with Adoramus.
+// AB_ADORAMUS,BL_PC,6,50,0,10,15 // In PVP and GVG, players deal +50% damage to other players, +0% to mobs, +10% to bosses, and +15% to other with Adoramus.
 

src/map/battle.cpp

@@ -2200,27 +2200,25 @@ static enum e_skill_damage_type battle_skill_damage_type( struct block_list* bl
  * @return Skill damage rate
  */
 static int battle_skill_damage_skill(struct block_list *src, struct block_list *target, uint16 skill_id) {
-	int16 m = src->m;
 	std::shared_ptr<s_skill_db> skill = skill_db.find(skill_id);
-	struct s_skill_damage *damage = NULL;
 
 	if (!skill || !skill->damage.map)
 		return 0;
 
-	damage = &skill->damage;
+	s_skill_damage *damage = &skill->damage;
 
 	//check the adjustment works for specified type
 	if (!(damage->caster&src->type))
 		return 0;
 
-	struct map_data *mapdata = map_getmapdata(m);
+	map_data *mapdata = map_getmapdata(src->m);
 
 	if ((damage->map&1 && (!mapdata->flag[MF_PVP] && !mapdata_flag_gvg2(mapdata) && !mapdata->flag[MF_BATTLEGROUND] && !mapdata->flag[MF_SKILL_DAMAGE] && !mapdata->flag[MF_RESTRICTED])) ||
 		(damage->map&2 && mapdata->flag[MF_PVP]) ||
 		(damage->map&4 && mapdata_flag_gvg2(mapdata)) ||
 		(damage->map&8 && mapdata->flag[MF_BATTLEGROUND]) ||
 		(damage->map&16 && mapdata->flag[MF_SKILL_DAMAGE]) ||
-		(damage->map&(mapdata->zone) && mapdata->flag[MF_RESTRICTED]))
+		(damage->map&mapdata->zone && mapdata->flag[MF_RESTRICTED]))
 	{
 		return damage->rate[battle_skill_damage_type(target)];
 	}
@@ -2236,12 +2234,13 @@ static int battle_skill_damage_skill(struct block_list *src, struct block_list *
  * @return Skill damage rate
  */
 static int battle_skill_damage_map(struct block_list *src, struct block_list *target, uint16 skill_id) {
-	int rate = 0;
+	map_data *mapdata = map_getmapdata(src->m);
-	struct map_data *mapdata = map_getmapdata(src->m);
 
 	if (!mapdata || !mapdata->flag[MF_SKILL_DAMAGE])
 		return 0;
 
+	int rate = 0;
+
 	// Damage rate for all skills at this map
 	if (mapdata->damage_adjust.caster&src->type)
 		rate = mapdata->damage_adjust.rate[battle_skill_damage_type(target)];

src/map/map.cpp

@@ -4426,25 +4426,19 @@ int cleanup_sub(struct block_list *bl, va_list ap)
  * Add new skill damage adjustment entry for a map
  * @param m: Map data
  * @param skill_id: Skill ID
- * @param pc: Rate to PC
+ * @param args: Mapflag arguments
- * @param mobs: Rate to Monster
- * @param boss: Rate to Boss-monster
- * @param other: Rate to Other target
- * @param caster: Caster type
  */
-void map_skill_damage_add(struct map_data *m, uint16 skill_id, int rate[SKILLDMG_MAX], uint16 caster) {
+void map_skill_damage_add(struct map_data *m, uint16 skill_id, union u_mapflag_args *args) {
-	struct s_skill_damage entry = {};
+	nullpo_retv(m);
+	nullpo_retv(args);
 
-	for (int i = 0; i < SKILLDMG_MAX; i++)
+	if (m->skill_damage.find(skill_id) != m->skill_damage.end()) { // Entry exists
-		entry.rate[i] = rate[i];
+		args->skill_damage.caster |= m->skill_damage[skill_id].caster;
-	entry.caster = caster;
+		m->skill_damage[skill_id] = args->skill_damage;
-
-	if (m->skill_damage.find(skill_id) != m->skill_damage.end()) {
-		m->skill_damage[skill_id] = entry;
 		return;
 	}
 
-	m->skill_damage.insert({ skill_id, entry });
+	m->skill_damage.insert({ skill_id, args->skill_damage }); // Add new entry
 }
 
 /**
@@ -4454,6 +4448,8 @@ void map_skill_damage_add(struct map_data *m, uint16 skill_id, int rate[SKILLDMG
  * @param per: Skill duration adjustment value in percent
  */
 void map_skill_duration_add(struct map_data *mapd, uint16 skill_id, uint16 per) {
+	nullpo_retv(mapd);
+
 	if (mapd->skill_duration.find(skill_id) != mapd->skill_duration.end()) // Entry exists
 		mapd->skill_duration[skill_id] += per;
 	else // Update previous entry
@@ -4824,13 +4820,13 @@ bool map_setmapflag_sub(int16 m, enum e_mapflag mapflag, bool status, union u_ma
 				nullpo_retr(false, args);
 
 				if (!args->flag_val) { // Signifies if it's a single skill or global damage adjustment
-					if (!args->skill_damage.caster) {
+					if (args->skill_damage.caster == 0) {
 						ShowError("map_setmapflag: Skill damage adjustment without casting type for map %s.\n", mapdata->name);
 						return false;
 					}
 
-					mapdata->damage_adjust.caster = args->skill_damage.caster;
+					mapdata->damage_adjust.caster |= args->skill_damage.caster;
-					for (int i = 0; i < SKILLDMG_MAX; i++)
+					for (int i = SKILLDMG_PC; i < SKILLDMG_MAX; i++)
 						mapdata->damage_adjust.rate[i] = cap_value(args->skill_damage.rate[i], -100, 100000);
 				}
 			}

src/map/map.hpp

@@ -1123,7 +1123,7 @@ void map_removemobs(int16 m); // [Wizputer]
 void map_addmap2db(struct map_data *m);
 void map_removemapdb(struct map_data *m);
 
-void map_skill_damage_add(struct map_data *m, uint16 skill_id, int rate[SKILLDMG_MAX], uint16 caster);
+void map_skill_damage_add(struct map_data *m, uint16 skill_id, union u_mapflag_args *args);
 void map_skill_duration_add(struct map_data *mapd, uint16 skill_id, uint16 per);
 
 enum e_mapflag map_getmapflag_by_name(char* name);

src/map/npc.cpp

@@ -4209,23 +4209,23 @@ static const char* npc_parse_mapflag(char* w1, char* w2, char* w3, char* w4, con
 						args.skill_damage.caster = atoi(caster_constant);
 					else {
 						int64 val_tmp;
-						int val;
 
 						if (!script_get_constant(caster_constant, &val_tmp)) {
 							ShowError( "npc_parse_mapflag: Unknown constant '%s'. Skipping (file '%s', line '%d').\n", caster_constant, filepath, strline(buffer, start - buffer) );
 							break;
 						}
 
-						val = static_cast<int>(val_tmp);
+						args.skill_damage.caster = static_cast<uint16>(val_tmp);
-						args.skill_damage.caster = val;
 					}
 					
-					if (!args.skill_damage.caster)
+					if (args.skill_damage.caster == 0)
 						args.skill_damage.caster = BL_ALL;
 
-					for (int i = 0; i < SKILLDMG_MAX; i++)
+					for (int i = SKILLDMG_PC; i < SKILLDMG_MAX; i++)
 						args.skill_damage.rate[i] = cap_value(args.skill_damage.rate[i], -100, 100000);
 
+					trim(skill_name);
+
 					if (strcmp(skill_name, "all") == 0) // Adjust damage for all skills
 						map_setmapflag_sub(m, MF_SKILL_DAMAGE, true, &args);
 					else if (skill_name2id(skill_name) <= 0)
@@ -4233,7 +4233,7 @@ static const char* npc_parse_mapflag(char* w1, char* w2, char* w3, char* w4, con
 					else { // Adjusted damage for specified skill
 						args.flag_val = 1;
 						map_setmapflag_sub(m, MF_SKILL_DAMAGE, true, &args);
-						map_skill_damage_add(map_getmapdata(m), skill_name2id(skill_name), args.skill_damage.rate, args.skill_damage.caster);
+						map_skill_damage_add(map_getmapdata(m), skill_name2id(skill_name), &args);
 					}
 				}
 			}

src/map/skill.cpp

@@ -22360,13 +22360,21 @@ static bool skill_parse_row_changematerialdb(char* split[], int columns, int cur
 static bool skill_parse_row_skilldamage(char* split[], int columns, int current)
 {
 	int64 caster_tmp;
-	uint16 id = 0;
+	uint16 id;
-	int caster;
+	int caster, value;
+	char *result;
 
 	trim(split[0]);
-	if (ISDIGIT(split[0][0]))
+	if (ISDIGIT(split[0][0])) {
-		id = atoi(split[0]);
+		value = strtol(split[0], &result, 10);
-	else
+
+		if (*result) {
+			ShowError("skill_parse_row_skilldamage: Invalid skill %s given for skill %d, skipping.\n", result, id);
+			return false;
+		}
+
+		id = value;
+	} else
 		id = skill_name2id(split[0]);
 
 	std::shared_ptr<s_skill_db> skill = skill_db.find(id);
@@ -22376,20 +22384,41 @@ static bool skill_parse_row_skilldamage(char* split[], int columns, int current)
 
 	skill->damage = {};
 	trim(split[1]);
-	if (ISDIGIT(split[1][0]))
+	if (ISDIGIT(split[1][0])) {
-		caster = atoi(split[1]);
+		value = strtol(split[1], &result, 10);
-	else { // Try to parse caster as constant
+
-		if (!script_get_constant(split[1], &caster_tmp)) {
+		if (*result) {
-			ShowError("skill_parse_row_skilldamage: Invalid caster constant given for skill %d. Skipping.", id);
+			ShowError("skill_parse_row_skilldamage: Invalid caster %s given for skill %d, skipping.\n", result, id);
 			return false;
 		}
-		caster = static_cast<int>(caster_tmp);
+
+		caster = value;
+	} else { // Try to parse caster as constant
+		if (!script_get_constant(split[1], &caster_tmp)) {
+			ShowError("skill_parse_row_skilldamage: Invalid caster constant given for skill %d, skipping.\n", id);
+			return false;
+		}
+		caster = static_cast<uint16>(caster_tmp);
 	}
 	skill->damage.caster |= caster;
-	skill->damage.map |= atoi(split[2]);
 
-	for(int offset = 3, i = 0; i < SKILLDMG_MAX && offset < columns; i++, offset++ ){
+	value = strtol(split[2], &result, 10);
-		skill->damage.rate[i] = cap_value(atoi(split[offset]), -100, INT_MAX);
+
+	if (*result) {
+		ShowError("skill_parse_row_skilldamage: Invalid map %s given for skill %d, skipping.\n", result, id);
+		return false;
+	}
+
+	skill->damage.map |= value;
+
+	for(int offset = 3, i = SKILLDMG_PC; i < SKILLDMG_MAX && offset < columns; i++, offset++ ){
+		value = strtol(split[offset], &result, 10);
+
+		if (*result) {
+			ShowError("skill_parse_row_skilldamage: Invalid damage %s given for skill %d, defaulting to 0.\n", result, id);
+			value = 0;
+		}
+		skill->damage.rate[i] = cap_value(value, -100, 100000);
 	}
 
 	return true;