Fix memory leaks on reloads

src/actions.cpp

@@ -25,6 +25,7 @@ void Actions::clearMap(ActionUseMap& map, bool fromLua)
 {
 	for (auto it = map.begin(); it != map.end();) {
 		if (fromLua == it->second.fromLua) {
+			it->second.clearScript();
 			it = map.erase(it);
 		} else {
 			++it;

src/baseevents.cpp

@@ -156,6 +156,15 @@ bool Event::loadCallback()
 	return true;
 }
 
+void Event::clearScript()
+{
+	// We only delete the script if it is lua, this is because the XML interface resets and deletes the reference table
+	// so it is not necessary to delete it manually.
+	if (scriptInterface && fromLua) {
+		scriptInterface->removeEvent(scriptId);
+	}
+}
+
 bool CallBack::loadCallBack(LuaScriptInterface* interface, const std::string& name)
 {
 	if (!interface) {
@@ -175,3 +184,10 @@ bool CallBack::loadCallBack(LuaScriptInterface* interface, const std::string& na
 	loaded = true;
 	return true;
 }
+
+void CallBack::clearScript()
+{
+	if (scriptInterface) {
+		scriptInterface->removeEvent(scriptId);
+	}
+}

src/baseevents.h

@@ -27,7 +27,9 @@ class Event
 	bool scripted = false;
 	bool fromLua = false;
 
-	int32_t getScriptId() { return scriptId; }
+	int32_t getScriptId() const { return scriptId; }
+
+	void clearScript();
 
 protected:
 	virtual std::string_view getScriptEventName() const = 0;
@@ -64,6 +66,8 @@ class CallBack
 
 	bool loadCallBack(LuaScriptInterface* interface, const std::string& name);
 
+	void clearScript();
+
 protected:
 	int32_t scriptId = 0;
 	LuaScriptInterface* scriptInterface = nullptr;

src/chat.cpp

@@ -254,11 +254,40 @@ bool ChatChannel::executeOnSpeakEvent(const Player& player, SpeakClasses& type,
 	return result;
 }
 
+void ChatChannel::clearScripts(LuaScriptInterface& interface) const
+{
+	interface.removeEvent(canJoinEvent);
+	interface.removeEvent(onJoinEvent);
+	interface.removeEvent(onLeaveEvent);
+	interface.removeEvent(onSpeakEvent);
+}
+
 Chat::Chat() : scriptInterface("Chat Interface"), dummyPrivate(CHANNEL_PRIVATE, "Private Chat Channel")
 {
 	scriptInterface.initState();
 }
 
+bool Chat::reload()
+{
+	for (auto&& [_, channel] : normalChannels) {
+		channel.clearScripts(scriptInterface);
+	}
+
+	for (auto&& [_, channel] : privateChannels) {
+		channel.clearScripts(scriptInterface);
+	}
+
+	for (auto&& [_, channel] : partyChannels) {
+		channel.clearScripts(scriptInterface);
+	}
+
+	for (auto&& [_, channel] : guildChannels) {
+		channel.clearScripts(scriptInterface);
+	}
+
+	return load();
+}
+
 bool Chat::load()
 {
 	pugi::xml_document doc;

src/chat.h

@@ -42,6 +42,8 @@ class ChatChannel
 	bool executeOnLeaveEvent(const Player& player);
 	bool executeOnSpeakEvent(const Player& player, SpeakClasses& type, const std::string& message);
 
+	void clearScripts(LuaScriptInterface& interface) const;
+
 protected:
 	UsersMap users;
 
@@ -95,6 +97,7 @@ class Chat
 	Chat(const Chat&) = delete;
 	Chat& operator=(const Chat&) = delete;
 
+	bool reload();
 	bool load();
 
 	ChatChannel* createChannel(const Player& player, uint16_t channelId);

src/combat.cpp

@@ -1429,3 +1429,18 @@ void MagicField::onStepInField(Creature* creature)
 		creature->addCondition(conditionCopy);
 	}
 }
+
+void CombatParams::clearScripts() const
+{
+	if (valueCallback) {
+		valueCallback->clearScript();
+	}
+
+	if (tileCallback) {
+		tileCallback->clearScript();
+	}
+
+	if (targetCallback) {
+		targetCallback->clearScript();
+	}
+}

src/combat.h

@@ -62,6 +62,8 @@ struct CombatParams
 	bool aggressive = true;
 	bool useCharges = false;
 	bool ignoreResistances = false;
+
+	void clearScripts() const;
 };
 
 class AreaCombat
@@ -109,6 +111,7 @@ class Combat
 
 	bool setCallback(CallBackParam_t key);
 	CallBack* getCallback(CallBackParam_t key);
+	void clearScripts() const { params.clearScripts(); }
 
 	bool setParam(CombatParam_t param, uint32_t value);
 	int32_t getParam(CombatParam_t param);

src/creatureevent.cpp

@@ -252,6 +252,7 @@ void CreatureEvent::copyEvent(CreatureEvent* creatureEvent)
 
 void CreatureEvent::clearEvent()
 {
+	clearScript();
 	scriptId = 0;
 	scriptInterface = nullptr;
 	scripted = false;

src/game.cpp

@@ -5802,7 +5802,7 @@ bool Game::reload(ReloadTypes_t reloadType)
 		case RELOAD_TYPE_ACTIONS:
 			return g_actions->reload();
 		case RELOAD_TYPE_CHAT:
-			return g_chat->load();
+			return g_chat->reload();
 		case RELOAD_TYPE_CONFIG:
 			return ConfigManager::load();
 		case RELOAD_TYPE_CREATURESCRIPTS: {
@@ -5866,7 +5866,7 @@ bool Game::reload(ReloadTypes_t reloadType)
 			mounts.reload();
 			ConfigManager::reload();
 			g_events->load();
-			g_chat->load();
+			g_chat->reload();
 			*/
 			return true;
 		}
@@ -5894,7 +5894,7 @@ bool Game::reload(ReloadTypes_t reloadType)
 			mounts.reload();
 			g_globalEvents->reload();
 			g_events->load();
-			g_chat->load();
+			g_chat->reload();
 			g_actions->clear(true);
 			g_creatureEvents->clear(true);
 			g_moveEvents->clear(true);

src/globalevent.cpp

@@ -18,6 +18,7 @@ void GlobalEvents::clearMap(GlobalEventMap& map, bool fromLua)
 {
 	for (auto it = map.begin(); it != map.end();) {
 		if (fromLua == it->second.fromLua) {
+			it->second.clearScript();
 			it = map.erase(it);
 		} else {
 			++it;

src/movement.cpp

@@ -23,6 +23,7 @@ void MoveEvents::clearMap(MoveListMap& map, bool fromLua)
 			auto& moveEvents = it->second.moveEvent[eventType];
 			for (auto find = moveEvents.begin(); find != moveEvents.end();) {
 				if (fromLua == find->fromLua) {
+					find->clearScript();
 					find = moveEvents.erase(find);
 				} else {
 					++find;
@@ -39,6 +40,7 @@ void MoveEvents::clearPosMap(MovePosListMap& map, bool fromLua)
 			auto& moveEvents = it->second.moveEvent[eventType];
 			for (auto find = moveEvents.begin(); find != moveEvents.end();) {
 				if (fromLua == find->fromLua) {
+					find->clearScript();
 					find = moveEvents.erase(find);
 				} else {
 					++find;

src/talkaction.cpp

@@ -15,6 +15,7 @@ void TalkActions::clear(bool fromLua)
 {
 	for (auto it = talkActions.begin(); it != talkActions.end();) {
 		if (fromLua == it->second.fromLua) {
+			it->second.clearScript();
 			it = talkActions.erase(it);
 		} else {
 			++it;

src/weapons.cpp

@@ -29,13 +29,14 @@ const Weapon* Weapons::getWeapon(const Item* item) const
 	if (it == weapons.end()) {
 		return nullptr;
 	}
-	return it->second;
+	return it->second.get();
 }
 
 void Weapons::clear(bool fromLua)
 {
 	for (auto it = weapons.begin(); it != weapons.end();) {
 		if (fromLua == it->second->fromLua) {
+			it->second->clearScript();
 			it = weapons.erase(it);
 		} else {
 			++it;
@@ -59,9 +60,9 @@ void Weapons::loadDefaults()
 			case WEAPON_AXE:
 			case WEAPON_SWORD:
 			case WEAPON_CLUB: {
-				WeaponMelee* weapon = new WeaponMelee(&scriptInterface);
+				auto weapon = std::make_unique<WeaponMelee>(&scriptInterface);
 				weapon->configureWeapon(it);
-				weapons[i] = weapon;
+				weapons.emplace(i, std::move(weapon));
 				break;
 			}
 
@@ -71,9 +72,9 @@ void Weapons::loadDefaults()
 					continue;
 				}
 
-				WeaponDistance* weapon = new WeaponDistance(&scriptInterface);
+				auto weapon = std::make_unique<WeaponDistance>(&scriptInterface);
 				weapon->configureWeapon(it);
-				weapons[i] = weapon;
+				weapons.emplace(i, std::move(weapon));
 				break;
 			}
 
@@ -97,9 +98,9 @@ Event_ptr Weapons::getEvent(const std::string& nodeName)
 
 bool Weapons::registerEvent(Event_ptr event, const pugi::xml_node&)
 {
-	Weapon* weapon = static_cast<Weapon*>(event.release()); // event is guaranteed to be a Weapon
+	Weapon_ptr weapon(static_cast<Weapon*>(event.release())); // event is guaranteed to be a Weapon
 
-	auto result = weapons.emplace(weapon->getID(), weapon);
+	auto result = weapons.emplace(weapon->getID(), std::move(weapon));
 	if (!result.second) {
 		std::cout << "[Warning - Weapons::registerEvent] Duplicate registered item with id: " << weapon->getID()
 		          << std::endl;
@@ -109,7 +110,8 @@ bool Weapons::registerEvent(Event_ptr event, const pugi::xml_node&)
 
 bool Weapons::registerLuaEvent(Weapon* weapon)
 {
-	weapons[weapon->getID()] = weapon;
+	Weapon_ptr weaponPtr{weapon};
+	weapons.emplace(weapon->getID(), std::move(weaponPtr));
 	return true;
 }
 

src/weapons.h

@@ -41,7 +41,7 @@ class Weapons final : public BaseEvents
 	Event_ptr getEvent(const std::string& nodeName) override;
 	bool registerEvent(Event_ptr event, const pugi::xml_node& node) override;
 
-	std::map<uint32_t, Weapon*> weapons;
+	std::map<uint32_t, Weapon_ptr> weapons;
 
 	LuaScriptInterface scriptInterface{"Weapon Interface"};
 };