Added a check for MAX_SKILL being to small (#6504)

Fixes #6494

Thanks to @voyfmyuh and @CairoLee

Co-authored-by: Aleos <aleos89@users.noreply.github.com>

src/common/mmo.hpp

@@ -83,7 +83,7 @@ typedef uint32 t_itemid;
 #define MAX_BANK_ZENY SINT32_MAX ///Max zeny in Bank
 #define MAX_FAME 1000000000 ///Max fame points
 #define MAX_CART 100 ///Maximum item in cart
-#define MAX_SKILL 1450 ///Maximum skill can be hold by Player, Homunculus, & Mercenary (skill list) AND skill_db limit
+#define MAX_SKILL 1454 ///Maximum skill can be hold by Player, Homunculus, & Mercenary (skill list) AND skill_db limit
 #define DEFAULT_WALK_SPEED 150 ///Default walk speed
 #define MIN_WALK_SPEED 20 ///Min walk speed
 #define MAX_WALK_SPEED 1000 ///Max walk speed

src/map/achievement.cpp

@@ -348,11 +348,11 @@ uint64 AchievementDatabase::parseBodyNode(const YAML::Node &node){
 }
 
 void AchievementDatabase::loadingFinished(){
-	for (const auto &achit : achievement_db) {
+	for (const auto &achit : *this) {
 		const std::shared_ptr<s_achievement_db> ach = achit.second;
 
 		for (auto dep = ach->dependent_ids.begin(); dep != ach->dependent_ids.end(); dep++) {
-			if (!achievement_db.exists(*dep)) {
+			if (!this->exists(*dep)) {
 				ShowWarning("achievement_read_db: An invalid Dependent ID %d was given for Achievement %d. Removing from list.\n", *dep, ach->achievement_id);
 				dep = ach->dependent_ids.erase(dep);
 

src/map/pc.cpp

@@ -6596,7 +6596,7 @@ uint8 pc_checkskill(struct map_session_data *sd, uint16 skill_id)
 #ifdef RENEWAL
 	if ((idx = skill_get_index(skill_id)) == 0) {
 #else
-	if( ( idx = skill_get_index_( skill_id, skill_id >= RK_ENCHANTBLADE, __FUNCTION__, __FILE__, __LINE__ ) ) == 0 ){
+	if( ( idx = skill_db.get_index( skill_id, skill_id >= RK_ENCHANTBLADE, __FUNCTION__, __FILE__, __LINE__ ) ) == 0 ){
 		if( skill_id >= RK_ENCHANTBLADE ){
 			// Silently fail for now -> future update planned
 			return 0;

src/map/skill.cpp

@@ -51,24 +51,6 @@ using namespace rathena;
 #define SKILLUNITTIMER_INTERVAL	100
 #define TIMERSKILL_INTERVAL	150
 
-// ranges reserved for mapping skill ids to skilldb offsets
-#define HM_SKILLRANGEMIN 700
-#define HM_SKILLRANGEMAX HM_SKILLRANGEMIN + MAX_HOMUNSKILL
-#define MC_SKILLRANGEMIN HM_SKILLRANGEMAX + 1
-#define MC_SKILLRANGEMAX MC_SKILLRANGEMIN + MAX_MERCSKILL
-#define EL_SKILLRANGEMIN MC_SKILLRANGEMAX + 1
-#define EL_SKILLRANGEMAX EL_SKILLRANGEMIN + MAX_ELEMENTALSKILL
-#define ABR_SKILLRANGEMIN EL_SKILLRANGEMAX + 1
-#define ABR_SKILLRANGEMAX ABR_SKILLRANGEMIN + MAX_ABRSKILL
-#define GD_SKILLRANGEMIN ABR_SKILLRANGEMAX + 1
-#define GD_SKILLRANGEMAX GD_SKILLRANGEMIN + MAX_GUILDSKILL
-#if GD_SKILLRANGEMAX > 999
-	#error GD_SKILLRANGEMAX is greater than 999
-#endif
-
-static uint16 skilldb_id2idx[(UINT16_MAX + 1)];	/// Skill ID to Index lookup: skill_index = skill_get_index(skill_id) - [FWI] 20160423 the whole index thing should be removed.
-static uint16 skill_num = 1;			 		/// Skill count, also as last index
-
 static struct eri *skill_timer_ers = NULL; //For handling skill_timerskills [Skotlex]
 static DBMap* bowling_db = NULL; // int mob_id -> struct mob_data*
 
@@ -128,10 +110,6 @@ static inline int splash_target(struct block_list* bl) {
 	return ( bl->type == BL_MOB ) ? BL_SKILL|BL_CHAR : BL_CHAR;
 }
 
-uint16 SKILL_MAX_DB(void) {
-	return skill_num;
-}
-
 /**
  * Get skill id from name
  * @param name
@@ -149,20 +127,6 @@ uint16 skill_name2id(const char* name) {
 	return 0;
 }
 
-/**
- * Get skill index from skill_db array. The index is also being used for skill lookup in mmo_charstatus::skill[]
- * @param skill_id
- * @param silent If Skill is undefined, show error message!
- * @return Skill Index or 0 if not found/unset
- **/
-uint16 skill_get_index_(uint16 skill_id, bool silent, const char *func, const char *file, int line) {
-	uint16 idx = skilldb_id2idx[skill_id];
-
-	if (!idx && skill_id != 0 && !silent)
-		ShowError("Skill '%d' is undefined! %s:%d::%s\n", skill_id, file, line, func);
-	return idx;
-}
-
 /**
  * Get Skill name
  * @param skill_id
@@ -19229,7 +19193,7 @@ int skill_autospell(struct map_session_data *sd, uint16 skill_id)
 {
 	nullpo_ret(sd);
 
-	if (skill_id == 0 || skill_get_index_(skill_id, true, __FUNCTION__, __FILE__, __LINE__) == 0 || SKILL_CHK_GUILD(skill_id))
+	if (skill_id == 0 || skill_db.get_index(skill_id, true, __FUNCTION__, __FILE__, __LINE__) == 0 || SKILL_CHK_GUILD(skill_id))
 		return 0;
 
 	uint16 lv = pc_checkskill(sd, skill_id), skill_lv = sd->menuskill_val;
@@ -24261,8 +24225,8 @@ uint64 SkillDatabase::parseBodyNode(const YAML::Node &node) {
 
 	if (!exists) {
 		this->put(skill_id, skill);
-		skilldb_id2idx[skill_id] = skill_num;
+		this->skilldb_id2idx[skill_id] = this->skill_num;
-		skill_num++;
+		this->skill_num++;
 	}
 
 	return 1;
@@ -24270,8 +24234,30 @@ uint64 SkillDatabase::parseBodyNode(const YAML::Node &node) {
 
 void SkillDatabase::clear() {
 	TypesafeCachedYamlDatabase::clear();
-	memset(skilldb_id2idx, 0, sizeof(skilldb_id2idx));
+	memset( this->skilldb_id2idx, 0, sizeof( this->skilldb_id2idx ) );
-	skill_num = 1;
+	this->skill_num = 1;
+}
+
+void SkillDatabase::loadingFinished(){
+	if( this->skill_num > MAX_SKILL ){
+		ShowError( "There are more skills defined in the skill database (%d) than the MAX_SKILL (%d) define. Please increase it and recompile.\n", this->skill_num, MAX_SKILL );
+	}
+}
+
+/**
+ * Get skill index from skill_db array. The index is also being used for skill lookup in mmo_charstatus::skill[]
+ * @param skill_id
+ * @param silent If Skill is undefined, show error message!
+ * @return Skill Index or 0 if not found/unset
+ **/
+uint16 SkillDatabase::get_index( uint16 skill_id, bool silent, const char *func, const char *file, int line ){
+	uint16 idx = this->skilldb_id2idx[skill_id];
+
+	if( idx == 0 && skill_id != 0 && !silent ){
+		ShowError( "Skill '%d' is undefined! %s:%d::%s\n", skill_id, file, line, func );
+	}
+
+	return idx;
 }
 
 SkillDatabase skill_db;

src/map/skill.hpp

@@ -308,15 +308,25 @@ struct s_skill_db {
 };
 
 class SkillDatabase : public TypesafeCachedYamlDatabase <uint16, s_skill_db> {
+private:
+	/// Skill ID to Index lookup: skill_index = skill_get_index(skill_id) - [FWI] 20160423 the whole index thing should be removed.
+	uint16 skilldb_id2idx[(UINT16_MAX + 1)];
+	/// Skill count, also as last index
+	uint16 skill_num;
+
 public:
 	SkillDatabase() : TypesafeCachedYamlDatabase("SKILL_DB", 3, 1) {
-
+		this->clear();
 	}
 
 	const std::string getDefaultLocation();
 	template<typename T, size_t S> bool parseNode(std::string nodeName, std::string subNodeName, YAML::Node node, T (&arr)[S]);
 	uint64 parseBodyNode(const YAML::Node &node);
 	void clear();
+	void loadingFinished();
+
+	// Additional
+	uint16 get_index( uint16 skill_id, bool silent, const char* func, const char* file, int line );
 };
 
 extern SkillDatabase skill_db;
@@ -500,8 +510,7 @@ const char*	skill_get_desc( uint16 skill_id ); 	// [Skotlex]
 int skill_tree_get_max( uint16 skill_id, int b_class );	// Celest
 
 // Accessor to the skills database
-uint16 skill_get_index_(uint16 skill_id, bool silent, const char *func, const char *file, int line);
+#define skill_get_index(skill_id) skill_db.get_index((skill_id), false, __FUNCTION__, __FILE__, __LINE__) /// Get skill index from skill_id (common usage on source)
-#define skill_get_index(skill_id)  skill_get_index_((skill_id), false, __FUNCTION__, __FILE__, __LINE__) /// Get skill index from skill_id (common usage on source)
 int skill_get_type( uint16 skill_id );
 e_damage_type skill_get_hit( uint16 skill_id );
 int skill_get_inf( uint16 skill_id );
@@ -554,8 +563,6 @@ unsigned short skill_dummy2skill_id(unsigned short skill_id);
 
 uint16 skill_name2id(const char* name);
 
-uint16 SKILL_MAX_DB(void);
-
 int skill_isammotype(struct map_session_data *sd, unsigned short skill_id);
 TIMER_FUNC(skill_castend_id);
 TIMER_FUNC(skill_castend_pos);

src/tool/yaml.hpp

@@ -628,6 +628,9 @@ void SkillDatabase::clear() {
 	TypesafeCachedYamlDatabase::clear();
 }
 
+void SkillDatabase::loadingFinished(){
+}
+
 SkillDatabase skill_db;
 
 const std::string MobDatabase::getDefaultLocation(){