It kinda works!

src/map/CMakeLists.txt

@@ -43,7 +43,8 @@ set(MAP_SOURCES
 	"vending.cpp"
 
 	"skills/skill.cpp"
-	"skills/swordsman/bash.cpp"
+	"skills/skillrepository.cpp"
+	"skills/swordsman.cpp"
 )
 
 set(MAP_HEADERS

src/map/skill.cpp

@@ -46,6 +46,8 @@
 #include "status.hpp"
 #include "unit.hpp"
 
+#include "skills/skillrepository.hpp"
+
 using namespace rathena;
 
 #define SKILLUNITTIMER_INTERVAL	100
@@ -5135,9 +5137,18 @@ int skill_castend_damage_id (struct block_list* src, struct block_list *bl, uint
 
 	map_freeblock_lock();
 
+
+	try {
+		const Skill& skill = skillRepository.getSkill(static_cast<e_skill>(skill_id));
+		int ret = skill.castendDamage(src, bl, skill_lv, tick, flag);
+		if (ret != 0) {
+			map_freeblock_unlock();
+			return ret;
+		}
+	} catch (SkillRepository::SkillNotFoundException e) {
+		// fallback on switch
 		switch(skill_id) {
 		case MER_CRASH:
-	case SM_BASH:
 		case MS_BASH:
 		case MC_MAMMONITE:
 		case TF_DOUBLE:
@@ -7142,6 +7153,7 @@ int skill_castend_damage_id (struct block_list* src, struct block_list *bl, uint
 			map_freeblock_unlock();
 			return 1;
 		}
+	}
 
 	if( sc && sc->getSCE(SC_CURSEDCIRCLE_ATKER) ) //Should only remove after the skill has been casted.
 		status_change_end(src,SC_CURSEDCIRCLE_ATKER);
@@ -7365,6 +7377,12 @@ int skill_castend_nodamage_id (struct block_list *src, struct block_list *bl, ui
 		return 1; //Skills that cause an status should be blocked if the target element blocks its element.
 
 	map_freeblock_lock();
+	
+	try {
+		const Skill& skill = skillRepository.getSkill(static_cast<e_skill>(skill_id));
+		skill.castendNoDamage(src, bl, skill_lv, tick, flag);
+	} catch (SkillRepository::SkillNotFoundException e) {
+		// fallback on switch
 		switch(skill_id)
 		{
 		case HLIF_HEAL:	//[orn]
@@ -8328,7 +8346,6 @@ int skill_castend_nodamage_id (struct block_list *src, struct block_list *bl, ui
 			}
 			break;
 
-	case SM_PROVOKE:
 		case SM_SELFPROVOKE:
 		case MER_PROVOKE:
 			if( status_has_mode(tstatus,MD_STATUSIMMUNE) || battle_check_undead(tstatus->race,tstatus->def_ele) ) {
@@ -12994,6 +13011,8 @@ int skill_castend_nodamage_id (struct block_list *src, struct block_list *bl, ui
 			return 1;
 			}
 		}
+	}
+
 
 	if (skill_id != SR_CURSEDCIRCLE && skill_id != NPC_SR_CURSEDCIRCLE) {
 		if (sc && sc->getSCE(SC_CURSEDCIRCLE_ATKER)) // Should only remove after the skill had been casted.
@@ -25344,6 +25363,8 @@ void do_init_skill(void)
 {
 	skill_readdb();
 
+	init_skill_repository();
+
 	skillunit_db = idb_alloc(DB_OPT_BASE);
 	skillusave_db = idb_alloc(DB_OPT_RELEASE_DATA);
 	bowling_db = idb_alloc(DB_OPT_BASE);

src/map/skills/main.cpp

@@ -1,41 +0,0 @@
-
-#include "skill.hpp"
-#include "skills.hpp"
-// #include "swordsman/bash.hpp"
-// #include "swordsman/provoke.hpp"
-
-#include "skilllist.hpp"
-
-
-
-int main() {
-
-	constexpr int skill_id = SM_BASH;
-
-	const auto &sk = skill_db.at(static_cast<e_skill>(skill_id));
-
-	const SkillImpl sk2 = Bash{};
-
-	std::visit([](auto &skill) {
-		skill.getSkillID();
-
-		skill.castend_damage_id(); // error
-		// skill.hpp:19:26: error: ‘const class Provoke’ has no member named ‘castendDamageId’; did you mean ‘castendNoDamageId’?
-		//    19 |   return as_underlying().castendDamageId();
-		//       |          ~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~
-		//       |          castendNoDamageId
-
-	}, sk2);
-
-
-	
-	for (auto &it : skill_db) {
-		std::visit([](auto &skill) {
-			skill.getSkillID();
-
-			// skill.castend_damage_id();
-		}, it.second);
-	}
-
-	return 0;
-}

src/map/skills/skill.cpp

@@ -0,0 +1,53 @@
+// Copyright (c) rAthena Dev Teams - Licensed under GNU GPL
+// For more information, see LICENCE in the main folder
+#include <common/showmsg.hpp>
+
+#include "skill.hpp"
+#include "map/battle.hpp"
+#include "map/skill.hpp"
+#include "map/clif.hpp"
+#include "map/map.hpp"
+#include "map/status.hpp"
+
+uint16_t Skill::getSkillId() const {
+    return skill_id_;
+}
+
+int Skill::castendDamageImpl(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const {
+    throw SkillNotImplementedException(skill_id_);
+}
+
+int Skill::castendNoDamageImpl(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const {
+    throw SkillNotImplementedException(skill_id_);
+}
+
+int Skill::castendPositionImpl() const {
+    throw SkillNotImplementedException(skill_id_);
+}
+
+int Skill::castendDamage(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const {
+    try {
+        return castendDamageImpl(src, target, skill_lv, tick, flag);
+    } catch (SkillNotImplementedException e) {
+        ShowWarning("castendDamage: %s\n", e.what());
+        clif_skill_damage(src, target, tick, status_get_amotion(src), status_get_status_data(target)->dmotion, 0, abs(skill_get_num(skill_id_, skill_lv)), skill_id_, skill_lv, skill_get_hit(skill_id_));
+        return 1;
+    }
+}
+
+int Skill::castendNoDamage(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const {
+    try {
+        return castendNoDamageImpl(src, target, skill_lv, tick, flag);
+    } catch (SkillNotImplementedException e) {
+        ShowWarning("castendNoDamage: %s\n", e.what());
+        clif_skill_damage(src, target, tick, status_get_amotion(src), status_get_status_data(target)->dmotion, 0, abs(skill_get_num(skill_id_, skill_lv)), skill_id_, skill_lv, skill_get_hit(skill_id_));
+        return 1;
+    }
+}
+
+int WeaponSkill::castendDamageImpl(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const {
+    skill_attack(BF_WEAPON, src, src, target, skill_id_, skill_lv, tick, flag);
+    return 0;
+};
+
+

src/map/skills/skill.hpp

@@ -7,30 +7,37 @@
 #include <array>
 #include <string>
 
-#include "skills.hpp"
-
-constexpr int MAX_SKILL_LEVEL = 13;
+#include "map/skill.hpp"
 
+class SkillNotImplementedException : public std::logic_error {
+public:
+	explicit SkillNotImplementedException(const std::string &what_arg) : std::logic_error(what_arg) {};
+	explicit SkillNotImplementedException(uint16_t skill_id) : std::logic_error("Skill " + std::to_string(skill_id) + " not implemented") {};
+};
 
 class Skill {
 public:
-	virtual int castendDamageId() const;
-	virtual int castendNodamageId() const;
-	virtual int castendPos2() const;
+	uint16_t getSkillId() const;
 
-	uint16_t getSkillID() const {
-		return nameid;
-	}
+	virtual ~Skill() = default;
+
+	explicit Skill(e_skill skillid) : skill_id_(static_cast<uint16_t>(skillid)) {};
+
+	int castendDamage(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const;
+	int castendNoDamage(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const;
 
 protected:
-	explicit Skill(e_skill skillid) : nameid(static_cast<uint16_t>(skillid)) {};
-private:
-
-	uint16_t nameid;
-	std::string name;
-	std::string desc;
-	std::array<int32_t, MAX_SKILL_LEVEL> range;
+	virtual int castendDamageImpl(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const;
+	virtual int castendNoDamageImpl(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const;
+	virtual int castendPositionImpl() const;
+	uint16_t skill_id_;
 };
 
+class WeaponSkill : public Skill {
+public:
+	explicit WeaponSkill(e_skill skill_id) : Skill(skill_id) {};
+protected:
+    virtual int castendDamageImpl(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const override;
+};
 
 #endif // MAP_SKILL_HPP

src/map/skills/skilllist.hpp

@@ -1,15 +0,0 @@
-#include <variant>
-
-#include "skill.hpp"
-#include "skills.hpp"
-
-#include "swordsman/bash.hpp"
-#include "swordsman/provoke.hpp"
-
-
-using SkillImpl = std::variant<Bash, Provoke>;
-
-std::unordered_map<e_skill, SkillImpl> skill_db = {
-	{ e_skill::SM_BASH, Bash{} },
-	{ e_skill::SM_PROVOKE, Provoke{} }
-};

src/map/skills/skillrepository.cpp

@@ -0,0 +1,25 @@
+// Copyright (c) rAthena Dev Teams - Licensed under GNU GPL
+// For more information, see LICENCE in the main folder
+
+#include "skillrepository.hpp"
+#include "swordsman.hpp"
+
+#include <memory>
+
+SkillRepository skillRepository;
+
+const Skill& SkillRepository::getSkill(e_skill skill_id) {
+    auto skill = skill_db_.find(skill_id);
+    if (skill == skill_db_.end()) {
+        throw SkillNotFoundException{};
+    }
+    return *skill->second;
+}
+
+void SkillRepository::addSkill(e_skill skill_id, std::unique_ptr<Skill> skill) {
+    skill_db_.emplace(skill_id, std::move(skill));
+}
+
+void init_skill_repository() {
+    init_swordsman_skills(skillRepository);
+}

src/map/skills/skillrepository.hpp

@@ -0,0 +1,25 @@
+// Copyright (c) rAthena Dev Teams - Licensed under GNU GPL
+// For more information, see LICENCE in the main folder
+
+#pragma once
+
+#include <memory>
+
+#include "skill.hpp"
+#include "map/skill.hpp"
+
+class SkillRepository {
+public:
+	class SkillNotFoundException : public std::exception {};
+
+	const Skill& getSkill(e_skill skill_id);
+
+	void addSkill(e_skill skill_id, std::unique_ptr<Skill> skill);
+
+private:
+	std::unordered_map<e_skill, std::unique_ptr<Skill>> skill_db_;
+};
+
+extern SkillRepository skillRepository;
+
+void init_skill_repository();

src/map/skills/swordsman.cpp

@@ -0,0 +1,56 @@
+
+#include "swordsman.hpp"
+
+#include "skillrepository.hpp"
+
+
+#include "map/battle.hpp"
+#include "map/clif.hpp"
+#include "map/mob.hpp"
+#include "map/skill.hpp"
+#include "map/status.hpp"
+#include "map/unit.hpp"
+
+
+Provoke::Provoke() : Skill(e_skill::SM_PROVOKE) {};
+
+int Provoke::castendNoDamageImpl(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const {
+    status_data *target_status = status_get_status_data(target);
+    if (status_has_mode(target_status, MD_STATUSIMMUNE) || battle_check_undead(target_status->race, target_status->def_ele)) {
+        return 1;
+    }
+
+    map_session_data *sd = BL_CAST(BL_PC, src);
+    mob_data *target_md = BL_CAST(BL_MOB, target);
+
+    sc_type type = skill_get_sc(skill_id_);
+    // official chance is 70% + 3% per skill level + srcBaseLevel% - targetBaseLevel%
+    int chance = 70 + 3 * skill_lv + status_get_lv(src) - status_get_lv(target);
+    int i = sc_start(src, target, type, skill_id_ == SM_SELFPROVOKE ? 100 : chance, skill_lv, skill_get_time(skill_id_, skill_lv));
+    if (!i) {
+        if (sd) {
+            clif_skill_fail(*sd, skill_id_);
+        }
+        return 0;
+    }
+    clif_skill_nodamage(src, target, skill_id_ == SM_SELFPROVOKE ? SM_PROVOKE : skill_id_, skill_lv, i);
+    unit_skillcastcancel(target, 2);
+    
+    if (target_md) {
+        target_md->state.provoke_flag = src->id;
+        mob_target(target_md, src, skill_get_range2(src, skill_id_, skill_lv, true));
+    }
+
+    // Provoke can cause Coma even though it's a nodamage skill
+    if (sd && battle_check_coma(*sd, *target, BF_MISC)) {
+        status_change_start(src, target, SC_COMA, 10000, skill_lv, 0, src->id, 0, 0, SCSTART_NONE);
+    }
+    return 0;
+}
+
+
+void init_swordsman_skills(SkillRepository& repo) {
+    repo.addSkill(e_skill::SM_BASH, std::make_unique<WeaponSkill>(e_skill::SM_BASH));
+    repo.addSkill(e_skill::SM_PROVOKE, std::make_unique<Provoke>());
+}
+

src/map/skills/swordsman.hpp

@@ -0,0 +1,12 @@
+#pragma once
+
+#include "skillrepository.hpp"
+
+class Provoke : public Skill {
+public:
+    Provoke();
+
+    virtual int castendNoDamageImpl(block_list *src, block_list *target, uint16 skill_lv, t_tick tick, int flag) const override;
+};
+
+void init_swordsman_skills(SkillRepository& repo);

src/map/skills/swordsman/bash.hpp

@@ -1,18 +0,0 @@
-#ifndef SKILLS_SWORDSMAN_BASH_HPP
-#define SKILLS_SWORDSMAN_BASH_HPP
-
-#include "../skill.hpp"
-#include "../skills.hpp"
-
-
-class Bash : public Skill {
-public:
-	int castendDamageId() const {
-		skill_attack(BF_WEAPON,src,src,bl,skill_id,skill_lv,tick,flag);
-	};
-
-	Bash() : Skill(e_skill::SM_BASH) {};
-
-};
-
-#endif // SKILLS_SWORDSMAN_BASH_HPP

src/map/skills/swordsman/provoke.hpp

@@ -1,18 +0,0 @@
-#ifndef SKILLS_SWORDSMAN_PROVOKE_HPP
-#define SKILLS_SWORDSMAN_PROVOKE_HPP
-
-#include "../skill.hpp"
-
-
-class Provoke : public Skill {
-public:
-	int castendNoDamageId() const {
-		return 0;
-	};
-
-	Provoke() : Skill(e_skill::SM_PROVOKE) {};
-
-private:
-};
-
-#endif // SKILLS_SWORDSMAN_PROVOKE_HPP