Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Memory Leak from Npc events #4483

Merged
merged 5 commits into from
Jun 22, 2023
Merged

Fix Memory Leak from Npc events #4483

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b3c890f
Fix Memory Leak from Npc events
MillhioreBT Jun 8, 2023
08aa41c
unique_ptr and shared_ptr
MillhioreBT Jun 8, 2023
9bd192d
unique_ptr instead shared_ptr
MillhioreBT Jun 9, 2023
795c115
remove friend / public interface
MillhioreBT Jun 22, 2023
0c904bf
correct init order
MillhioreBT Jun 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
48 changes: 20 additions & 28 deletions src/npc.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ extern Game g_game;
extern LuaEnvironment g_luaEnvironment;

uint32_t Npc::npcAutoID = 0x20000000;
NpcScriptInterface* Npc::scriptInterface = nullptr;

void Npcs::reload()
{
Expand All @@ -22,9 +21,6 @@ void Npcs::reload()
it.second->closeAllShopWindows();
}

delete Npc::scriptInterface;
Npc::scriptInterface = nullptr;

for (const auto& it : npcs) {
it.second->reload();
}
Expand All @@ -39,8 +35,7 @@ Npc* Npc::createNpc(const std::string& name)
return npc.release();
}

Npc::Npc(const std::string& name) :
Creature(), filename("data/npc/" + name + ".xml"), npcEventHandler(nullptr), masterRadius(-1), loaded(false)
Npc::Npc(const std::string& name) : Creature(), filename("data/npc/" + name + ".xml"), masterRadius(-1), loaded(false)
{
reset();
}
Expand All @@ -59,11 +54,6 @@ bool Npc::load()

reset();

if (!scriptInterface) {
scriptInterface = new NpcScriptInterface();
scriptInterface->loadNpcLib("data/npc/lib/npc.lua");
}

loaded = loadFromXml();
return loaded;
}
Expand All @@ -80,8 +70,7 @@ void Npc::reset()
focusCreature = 0;
speechBubble = SPEECHBUBBLE_NONE;

delete npcEventHandler;
npcEventHandler = nullptr;
npcEventHandler.reset();

parameters.clear();
shopPlayerSet.clear();
Expand Down Expand Up @@ -208,12 +197,11 @@ bool Npc::loadFromXml()

pugi::xml_attribute scriptFile = npcNode.attribute("script");
if (scriptFile) {
npcEventHandler = new NpcEventsHandler(scriptFile.as_string(), this);
if (!npcEventHandler->isLoaded()) {
delete npcEventHandler;
npcEventHandler = nullptr;
auto handler = std::make_unique<NpcEventsHandler>(scriptFile.as_string(), this);
if (!handler->isLoaded()) {
return false;
}
npcEventHandler = std::move(handler);
}
return true;
}
Expand Down Expand Up @@ -545,8 +533,6 @@ void Npc::closeAllShopWindows()
}
}

NpcScriptInterface* Npc::getScriptInterface() { return scriptInterface; }

NpcScriptInterface::NpcScriptInterface() : LuaScriptInterface("Npc interface")
{
libLoaded = false;
Expand Down Expand Up @@ -1087,8 +1073,14 @@ int NpcScriptInterface::luaNpcCloseShopWindow(lua_State* L)
}

NpcEventsHandler::NpcEventsHandler(const std::string& file, Npc* npc) :
npc(npc), scriptInterface(npc->getScriptInterface())
scriptInterface(std::make_unique<NpcScriptInterface>()), npc(npc)
{
if (!scriptInterface->loadNpcLib("data/npc/lib/npc.lua")) {
std::cout << "[Warning - NpcLib::NpcLib] Can not load lib: " << file << std::endl;
std::cout << scriptInterface->getLastLuaError() << std::endl;
return;
}

loaded = scriptInterface->loadFile("data/npc/scripts/" + file, npc) == 0;
if (!loaded) {
std::cout << "[Warning - NpcScript::NpcScript] Can not load script: " << file << std::endl;
Expand Down Expand Up @@ -1119,7 +1111,7 @@ void NpcEventsHandler::onCreatureAppear(Creature* creature)
}

ScriptEnvironment* env = scriptInterface->getScriptEnv();
env->setScriptId(creatureAppearEvent, scriptInterface);
env->setScriptId(creatureAppearEvent, scriptInterface.get());
env->setNpc(npc);

lua_State* L = scriptInterface->getLuaState();
Expand All @@ -1142,7 +1134,7 @@ void NpcEventsHandler::onCreatureDisappear(Creature* creature)
}

ScriptEnvironment* env = scriptInterface->getScriptEnv();
env->setScriptId(creatureDisappearEvent, scriptInterface);
env->setScriptId(creatureDisappearEvent, scriptInterface.get());
env->setNpc(npc);

lua_State* L = scriptInterface->getLuaState();
Expand All @@ -1165,7 +1157,7 @@ void NpcEventsHandler::onCreatureMove(Creature* creature, const Position& oldPos
}

ScriptEnvironment* env = scriptInterface->getScriptEnv();
env->setScriptId(creatureMoveEvent, scriptInterface);
env->setScriptId(creatureMoveEvent, scriptInterface.get());
env->setNpc(npc);

lua_State* L = scriptInterface->getLuaState();
Expand All @@ -1190,7 +1182,7 @@ void NpcEventsHandler::onCreatureSay(Creature* creature, SpeakClasses type, cons
}

ScriptEnvironment* env = scriptInterface->getScriptEnv();
env->setScriptId(creatureSayEvent, scriptInterface);
env->setScriptId(creatureSayEvent, scriptInterface.get());
env->setNpc(npc);

lua_State* L = scriptInterface->getLuaState();
Expand All @@ -1216,7 +1208,7 @@ void NpcEventsHandler::onPlayerTrade(Player* player, int32_t callback, uint16_t
}

ScriptEnvironment* env = scriptInterface->getScriptEnv();
env->setScriptId(-1, scriptInterface);
env->setScriptId(-1, scriptInterface.get());
env->setNpc(npc);

lua_State* L = scriptInterface->getLuaState();
Expand Down Expand Up @@ -1244,7 +1236,7 @@ void NpcEventsHandler::onPlayerCloseChannel(Player* player)
}

ScriptEnvironment* env = scriptInterface->getScriptEnv();
env->setScriptId(playerCloseChannelEvent, scriptInterface);
env->setScriptId(playerCloseChannelEvent, scriptInterface.get());
env->setNpc(npc);

lua_State* L = scriptInterface->getLuaState();
Expand All @@ -1267,7 +1259,7 @@ void NpcEventsHandler::onPlayerEndTrade(Player* player)
}

ScriptEnvironment* env = scriptInterface->getScriptEnv();
env->setScriptId(playerEndTradeEvent, scriptInterface);
env->setScriptId(playerEndTradeEvent, scriptInterface.get());
env->setNpc(npc);

lua_State* L = scriptInterface->getLuaState();
Expand All @@ -1290,7 +1282,7 @@ void NpcEventsHandler::onThink()
}

ScriptEnvironment* env = scriptInterface->getScriptEnv();
env->setScriptId(thinkEvent, scriptInterface);
env->setScriptId(thinkEvent, scriptInterface.get());
env->setNpc(npc);

scriptInterface->pushFunction(thinkEvent);
Expand Down
9 changes: 4 additions & 5 deletions src/npc.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,9 +70,10 @@ class NpcEventsHandler

bool isLoaded() const;

std::unique_ptr<NpcScriptInterface> scriptInterface;

private:
Npc* npc;
NpcScriptInterface* scriptInterface;

int32_t creatureAppearEvent = -1;
int32_t creatureDisappearEvent = -1;
Expand Down Expand Up @@ -147,7 +148,7 @@ class Npc final : public Creature
void turnToCreature(Creature* creature);
void setCreatureFocus(Creature* creature);

NpcScriptInterface* getScriptInterface();
auto& getScriptInterface() { return npcEventHandler->scriptInterface; }

static uint32_t npcAutoID;

Expand Down Expand Up @@ -190,7 +191,7 @@ class Npc final : public Creature
std::string name;
std::string filename;

NpcEventsHandler* npcEventHandler;
std::unique_ptr<NpcEventsHandler> npcEventHandler;

Position masterPos;

Expand All @@ -207,8 +208,6 @@ class Npc final : public Creature
bool isIdle;
bool pushable;

static NpcScriptInterface* scriptInterface;

friend class Npcs;
friend class NpcScriptInterface;
};
Expand Down