Use Town as plain Lua table #4764
Conversation
src/luascript.cpp
Outdated
| @@ -9765,7 +9766,7 @@ int LuaScriptInterface::luaPlayerGetTown(lua_State* L) | |||
| // player:getTown() | |||
| Player* player = tfs::lua::getUserdata<Player>(L, 1); | |||
| if (player) { | |||
| tfs::lua::pushUserdata(L, player->getTown()); | |||
| pushTown(L, *player->getTown()); | |||
| tfs::lua::setMetatable(L, -1, "Town"); | |||
There was a problem hiding this comment.
This is not necessary, since the pushTown method already adds the metatable
src/luascript.cpp
Outdated
| if (town) { | ||
| tfs::lua::pushUserdata(L, town); | ||
| pushTown(L, *town); | ||
| tfs::lua::setMetatable(L, -1, "Town"); |
There was a problem hiding this comment.
Do not set the metatable, since it is already added in the pushTown method
| @@ -0,0 +1,24 @@ | |||
| Town = {} | |||
There was a problem hiding this comment.
This file is never read, it must be added to the core.lua file
There was a problem hiding this comment.
It is also necessary to add the __index metamethod that refers to the same table
Town.__index = Town
src/luascript.cpp
Outdated
| setField(L, "name", town.name); | ||
| tfs::lua::pushPosition(L, town.templePosition); | ||
| lua_setfield(L, -2, "templePosition"); | ||
| tfs::lua::setMetatable(L, -1, "Town"); |
There was a problem hiding this comment.
adding the metatable is impossible, since the table Town does not actually exist in the registry LUA_REGISTRYINDEX this is because the TFS API registers the metatables with the registerClass method in this index
so it will be necessary to search for the table in the global environment and then apply it as a metatable manually
lua_getglobal(L, "Town");
lua_setmetatable(L, -2);
ranisalt
force-pushed
the
town-lua-table
branch
from
4f1b185
to
e41b7b8
Compare
| @@ -1219,7 +1219,7 @@ class Player final : public Creature, public Cylinder | |||
| Party* party = nullptr; | |||
| Player* tradePartner = nullptr; | |||
| SchedulerTask* walkTask = nullptr; | |||
| Town* town = nullptr; | |||
| const Town* town = nullptr; | |||
There was a problem hiding this comment.
could you explain why it became a const?
There was a problem hiding this comment.
This is assigned with the return value of getTown (i.e. player->town = towns->getTown(id)) which became const Town*
| { | ||
| auto it = townMap.find(townId); | ||
| if (it == townMap.end()) { | ||
| return nullptr; | ||
| } | ||
| return it->second; | ||
| return it->second.get(); | ||
| } | ||
|
|
||
| const TownMap& getTowns() const { return townMap; } |
There was a problem hiding this comment.
We could use auto and keep the type inferred directly from the townMap variable ;)
There was a problem hiding this comment.
I'd rather do it separately. Probably would change to decltype(auto) getTowns() const to avoid copying
However, end goal is to not have anything related to towns in the core. The concept of a town only makes sense for the game, not for the engine
Pull Request Prelude
Changes Proposed
The
Townmetatable cannot be used to mutate towns, so it can be pushed as a table for a lighterweight implementation and to cleanup some functions in luascript