fix: weapons, reverting some changes from XML pr (opentibiabr#784)

Revert (and revise) some changes from: opentibiabr#740

Fixes using weapons event (onUseWeapon)
Fixes monsters spells invert order of target and caster

Converted some raw pointers from "new" to "shared_ptr", thus ensuring that memory is automatically deallocated and no delete is needed, preventing memory leaks.

src/creatures/combat/spells.cpp

@@ -246,20 +246,18 @@ Position Spells::getCasterPosition(Creature* creature, Direction dir)
 	return getNextPosition(dir, creature->getPosition());
 }
 
-CombatSpell::CombatSpell(Combat* newCombat, bool newNeedTarget, bool needDirection) :
+CombatSpell::CombatSpell(Combat* newCombat, bool newNeedTarget, bool newNeedDirection) :
 	Script(&g_spells().getScriptInterface()),
 	combat(newCombat),
-	needDirection(newNeedTarget),
-	needTarget(needDirection)
+	needDirection(newNeedDirection),
+	needTarget(newNeedTarget)
 {
 // Empty
 }
 
-CombatSpell::~CombatSpell() = default;
-
 bool CombatSpell::loadScriptCombat()
 {
-	combat = g_luaEnvironment.getCombatObject(g_luaEnvironment.lastCombatId);
+	combat = g_luaEnvironment.getCombatObject(g_luaEnvironment.lastCombatId).get();
 	return combat != nullptr;
 }
 

src/creatures/combat/spells.h

@@ -95,9 +95,7 @@ class CombatSpell final : public Script, public BaseSpell
 {
 	public:
 		// Constructor
-		CombatSpell(Combat* newCombat, bool newNeedTarget, bool needDirection);
-		// Destructor
-		~CombatSpell() override;
+		CombatSpell(Combat* newCombat, bool newNeedTarget, bool newNeedDirection);
 
 		// The copy constructor and the assignment operator have been deleted to prevent accidental copying.
 		CombatSpell(const CombatSpell&) = delete;

src/items/weapons/weapons.cpp

@@ -183,34 +183,40 @@ bool Weapon::useFist(Player* player, Creature* target)
 
 void Weapon::internalUseWeapon(Player* player, Item* item, Creature* target, int32_t damageModifier) const
 {
-	CombatDamage damage;
-	WeaponType_t weaponType = item->getWeaponType();
-	if (weaponType == WEAPON_AMMO || weaponType == WEAPON_DISTANCE) {
-		damage.origin = ORIGIN_RANGED;
-	} else {
-		damage.origin = ORIGIN_MELEE;
-	}
-
-	damage.primary.type = params.combatType;
-	damage.secondary.type = getElementType();
-
-	if (damage.secondary.type == COMBAT_NONE) {
-		damage.primary.value = (getWeaponDamage(player, target, item) * damageModifier) / 100;
-		damage.secondary.value = 0;
+	if (isLoadedCallback()) {
+		LuaVariant var;
+		var.type = VARIANT_NUMBER;
+		var.number = target->getID();
+		executeUseWeapon(player, var);
 	} else {
+		CombatDamage damage;
+		WeaponType_t weaponType = item->getWeaponType();
+		if (weaponType == WEAPON_AMMO || weaponType == WEAPON_DISTANCE) {
+			damage.origin = ORIGIN_RANGED;
+		} else {
+			damage.origin = ORIGIN_MELEE;
+		}
+		damage.primary.type = params.combatType;
 		damage.primary.value = (getWeaponDamage(player, target, item) * damageModifier) / 100;
-		damage.secondary.value = (getElementDamage(player, target, item) * damageModifier) / 100;
+		damage.secondary.type = getElementType();
+		damage.secondary.value = getElementDamage(player, target, item);
+		Combat::doCombatHealth(player, target, damage, params);
 	}
 
-	Combat::doCombatHealth(player, target, damage, params);
-
 	onUsedWeapon(player, item, target->getTile());
 }
 
 void Weapon::internalUseWeapon(Player* player, Item* item, Tile* tile) const
 {
-	Combat::postCombatEffects(player, tile->getPosition(), params);
-	g_game().addMagicEffect(tile->getPosition(), CONST_ME_POFF);
+	if (isLoadedCallback()) {
+		LuaVariant var;
+		var.type = VARIANT_TARGETPOSITION;
+		var.pos = tile->getPosition();
+		executeUseWeapon(player, var);
+	} else {
+		Combat::postCombatEffects(player, tile->getPosition(), params);
+		g_game().addMagicEffect(tile->getPosition(), CONST_ME_POFF);
+	}
 	onUsedWeapon(player, item, tile);
 }
 
@@ -295,6 +301,29 @@ int32_t Weapon::getHealthCost(const Player* player) const
 	return (player->getMaxHealth() * healthPercent) / 100;
 }
 
+bool Weapon::executeUseWeapon(Player* player, const LuaVariant& var) const
+{
+	//onUseWeapon(player, var)
+	if (!getScriptInterface()->reserveScriptEnv()) {
+		SPDLOG_ERROR("[Weapon::executeUseWeapon - Player {} weaponId {}]"
+                     "Call stack overflow. Too many lua script calls being nested.",
+                     player->getName(), getID());
+		return false;
+	}
+
+	ScriptEnvironment* env = getScriptInterface()->getScriptEnv();
+	env->setScriptId(getScriptId(), getScriptInterface());
+
+	lua_State* L = getScriptInterface()->getLuaState();
+
+	getScriptInterface()->pushFunction(getScriptId());
+	LuaScriptInterface::pushUserdata<Player>(L, player);
+	LuaScriptInterface::setMetatable(L, -1, "Player");
+	getScriptInterface()->pushVariant(L, var);
+
+	return getScriptInterface()->callFunction(2);
+}
+
 void Weapon::decrementItemCount(Item* item)
 {
 	uint16_t count = item->getItemCount();
@@ -305,13 +334,16 @@ void Weapon::decrementItemCount(Item* item)
 	}
 }
 
-void WeaponMelee::configureWeapon(const ItemType& it)
+WeaponMelee::WeaponMelee(LuaScriptInterface* interface) : Weapon(interface)
 {
 	// Add combat type and blocked attributes to the weapon
 	params.blockedByArmor = true;
 	params.blockedByShield = true;
 	params.combatType = COMBAT_PHYSICALDAMAGE;
+}
 
+void WeaponMelee::configureWeapon(const ItemType& it)
+{
 	if (it.abilities) {
 		elementType = it.abilities->elementType;
 		elementDamage = it.abilities->elementDamage;
@@ -407,13 +439,17 @@ int32_t WeaponMelee::getWeaponDamage(const Player* player, const Creature*, cons
 	return -normal_random(minValue, (maxValue * static_cast<int32_t>(player->getVocation()->meleeDamageMultiplier)));
 }
 
-void WeaponDistance::configureWeapon(const ItemType& it)
+WeaponDistance::WeaponDistance(LuaScriptInterface* interface) :
+	Weapon(interface)
 {
 	// Add combat type and distance effect to the weapon
 	params.blockedByArmor = true;
 	params.combatType = COMBAT_PHYSICALDAMAGE;
-	params.distanceEffect = it.shootType;
+}
 
+void WeaponDistance::configureWeapon(const ItemType& it)
+{
+	params.distanceEffect = it.shootType;
 	if (it.abilities) {
 		elementType = it.abilities->elementType;
 		elementDamage = it.abilities->elementDamage;

src/items/weapons/weapons.h

@@ -55,11 +55,11 @@ class Weapons final : public Scripts
 
 constexpr auto g_weapons = &Weapons::getInstance;
 
-class Weapon
+class Weapon : public Script
 {
 	public:
-		Weapon() = default;
-		virtual ~Weapon() = default;
+		using Script::Script;
+
 		virtual void configureWeapon(const ItemType& it);
 		virtual bool interruptSwing() const {
 			return false;
@@ -191,6 +191,7 @@ class Weapon
 
 		uint32_t getManaCost(const Player* player) const;
 		int32_t getHealthCost(const Player* player) const;
+		bool executeUseWeapon(Player* player, const LuaVariant& var) const;
 
 		uint16_t id = 0;
 
@@ -227,6 +228,12 @@ class Weapon
 class WeaponMelee final : public Weapon
 {
 	public:
+		explicit WeaponMelee(LuaScriptInterface* interface);
+
+		std::string getScriptTypeName() const override {
+			return "onUseWeapon";
+		}
+
 		void configureWeapon(const ItemType& it) override;
 
 		bool useWeapon(Player* player, Item* item, Creature* target) const override;
@@ -246,6 +253,12 @@ class WeaponMelee final : public Weapon
 class WeaponDistance final : public Weapon
 {
 	public:
+		explicit WeaponDistance(LuaScriptInterface* interface);
+
+		std::string getScriptTypeName() const override {
+			return "onUseWeapon";
+		}
+
 		void configureWeapon(const ItemType& it) override;
 		bool interruptSwing() const override {
 			return true;
@@ -267,6 +280,12 @@ class WeaponDistance final : public Weapon
 class WeaponWand final : public Weapon
 {
 	public:
+		using Weapon::Weapon;
+
+		std::string getScriptTypeName() const override {
+			return "onUseWeapon";
+		}
+
 		void configureWeapon(const ItemType& it) override;
 
 		int32_t getWeaponDamage(const Player* player, const Creature* target, const Item* item, bool maxDamage = false) const override;

src/lua/functions/creatures/combat/combat_functions.cpp

@@ -16,7 +16,7 @@
 
 int CombatFunctions::luaCombatCreate(lua_State* L) {
 	// Combat()
-	pushUserdata<Combat>(L, g_luaEnvironment.createCombatObject(getScriptEnv()->getScriptInterface()));
+	pushUserdata<Combat>(L, g_luaEnvironment.createCombatObject(getScriptEnv()->getScriptInterface()).get());
 	setMetatable(L, -1, "Combat");
 	return 1;
 }

src/lua/functions/items/weapon_functions.cpp

@@ -9,10 +9,11 @@
 
 #include "pch.hpp"
 
+#include "lua/functions/items/weapon_functions.hpp"
+
 #include "game/game.h"
 #include "items/item.h"
-#include "items/weapons/weapons.h"
-#include "lua/functions/items/weapon_functions.hpp"
+#include "lua/scripts/lua_environment.hpp"
 #include "lua/scripts/scripts.h"
 #include "utils/tools.h"
 
@@ -23,34 +24,31 @@ int WeaponFunctions::luaCreateWeapon(lua_State* L) {
 		case WEAPON_SWORD:
 		case WEAPON_AXE:
 		case WEAPON_CLUB: {
-			auto weapon = new WeaponMelee();
-			if (weapon) {
-				pushUserdata<WeaponMelee>(L, weapon);
+			if (auto weaponPtr = g_luaEnvironment.createWeaponObject<WeaponMelee>(getScriptEnv()->getScriptInterface())) {
+				pushUserdata<WeaponMelee>(L, weaponPtr.get());
 				setMetatable(L, -1, "Weapon");
-				weapon->weaponType = type;
+				weaponPtr->weaponType = type;
 			} else {
 				lua_pushnil(L);
 			}
 			break;
 		}
 		case WEAPON_DISTANCE:
 		case WEAPON_AMMO: {
-			auto weapon = new WeaponDistance();
-			if (weapon) {
-				pushUserdata<WeaponDistance>(L, weapon);
+			if (auto weaponPtr = g_luaEnvironment.createWeaponObject<WeaponDistance>(getScriptEnv()->getScriptInterface())) {
+				pushUserdata<WeaponDistance>(L, weaponPtr.get());
 				setMetatable(L, -1, "Weapon");
-				weapon->weaponType = type;
+				weaponPtr->weaponType = type;
 			} else {
 				lua_pushnil(L);
 			}
 			break;
 		}
 		case WEAPON_WAND: {
-			auto weapon = new WeaponWand();
-			if (weapon) {
-				pushUserdata<WeaponWand>(L, weapon);
+			if (auto weaponPtr = g_luaEnvironment.createWeaponObject<WeaponWand>(getScriptEnv()->getScriptInterface())) {
+				pushUserdata<WeaponWand>(L, weaponPtr.get());
 				setMetatable(L, -1, "Weapon");
-				weapon->weaponType = type;
+				weaponPtr->weaponType = type;
 			} else {
 				lua_pushnil(L);
 			}
@@ -123,8 +121,13 @@ int WeaponFunctions::luaWeaponRegister(lua_State* L) {
 
 int WeaponFunctions::luaWeaponOnUseWeapon(lua_State* L) {
 	// weapon:onUseWeapon(callback)
-	const Weapon* weapon = getUserdata<Weapon>(L, 1);
+	Weapon* weapon = getUserdata<Weapon>(L, 1);
 	if (weapon) {
+		if (!weapon->loadCallback()) {
+			pushBoolean(L, false);
+			return 1;
+		}
+
 		pushBoolean(L, true);
 	} else {
 		lua_pushnil(L);

src/lua/scripts/lua_environment.cpp

@@ -76,35 +76,29 @@ LuaScriptInterface * LuaEnvironment::getTestInterface() {
 	return testInterface;
 }
 
-Combat * LuaEnvironment::getCombatObject(uint32_t id) const {
+std::shared_ptr<Combat> LuaEnvironment::getCombatObject(uint32_t id) const {
 	auto it = combatMap.find(id);
 	if (it == combatMap.end()) {
 		return nullptr;
 	}
-	return it -> second;
+	return it->second;
 }
 
-Combat * LuaEnvironment::createCombatObject(LuaScriptInterface * interface) {
-	Combat * combat = new Combat;
+std::shared_ptr<Combat> LuaEnvironment::createCombatObject(LuaScriptInterface *interface) {
+	auto combat = std::make_shared<Combat>();
 	combatMap[++lastCombatId] = combat;
 	combatIdMap[interface].push_back(lastCombatId);
 	return combat;
 }
 
-void LuaEnvironment::clearCombatObjects(LuaScriptInterface * interface) {
+void LuaEnvironment::clearCombatObjects(LuaScriptInterface *interface) {
 	auto it = combatIdMap.find(interface);
 	if (it == combatIdMap.end()) {
 		return;
 	}
 
-	for (uint32_t id: it -> second) {
-		auto itt = combatMap.find(id);
-		if (itt != combatMap.end()) {
-			delete itt -> second;
-			combatMap.erase(itt);
-		}
-	}
-	it -> second.clear();
+	it->second.clear();
+	combatMap.clear();
 }
 
 AreaCombat * LuaEnvironment::getAreaObject(uint32_t id) const {