From 4b2ef32b86d0b1aac5510b86de0639deaeaf655a Mon Sep 17 00:00:00 2001 From: uramer Date: Sun, 29 Jan 2023 17:06:09 +0100 Subject: [PATCH] Move implementation of UI Content to Lua (!2661 for 0.48) --- apps/openmw/mwlua/uibindings.cpp | 71 +-------- .../openmw_test_suite/lua/test_ui_content.cpp | 103 +++++++++---- components/CMakeLists.txt | 1 + components/lua_ui/content.cpp | 107 ++----------- components/lua_ui/content.hpp | 109 +++++++++++--- components/lua_ui/content.lua | 140 ++++++++++++++++++ components/lua_ui/element.cpp | 4 +- files/lua_api/openmw/ui.lua | 6 + 8 files changed, 326 insertions(+), 215 deletions(-) create mode 100644 components/lua_ui/content.lua diff --git a/apps/openmw/mwlua/uibindings.cpp b/apps/openmw/mwlua/uibindings.cpp index eec6509034..37351549ef 100644 --- a/apps/openmw/mwlua/uibindings.cpp +++ b/apps/openmw/mwlua/uibindings.cpp @@ -89,69 +89,6 @@ namespace MWLua sol::table initUserInterfacePackage(const Context& context) { - auto uiContent = context.mLua->sol().new_usertype("UiContent"); - uiContent[sol::meta_function::length] = [](const LuaUi::Content& content) - { - return content.size(); - }; - uiContent[sol::meta_function::index] = sol::overload( - [](const LuaUi::Content& content, size_t index) - { - return content.at(fromLuaIndex(index)); - }, - [](const LuaUi::Content& content, std::string_view name) - { - return content.at(name); - }); - uiContent[sol::meta_function::new_index] = sol::overload( - [](LuaUi::Content& content, size_t index, const sol::table& table) - { - content.assign(fromLuaIndex(index), table); - }, - [](LuaUi::Content& content, size_t index, sol::nil_t nil) - { - content.remove(fromLuaIndex(index)); - }, - [](LuaUi::Content& content, std::string_view name, const sol::table& table) - { - content.assign(name, table); - }, - [](LuaUi::Content& content, std::string_view name, sol::nil_t nil) - { - content.remove(name); - }); - uiContent["insert"] = [](LuaUi::Content& content, size_t index, const sol::table& table) - { - content.insert(fromLuaIndex(index), table); - }; - uiContent["add"] = [](LuaUi::Content& content, const sol::table& table) - { - content.insert(content.size(), table); - }; - uiContent["indexOf"] = [](LuaUi::Content& content, const sol::table& table) -> sol::optional - { - size_t index = content.indexOf(table); - if (index < content.size()) - return toLuaIndex(index); - else - return sol::nullopt; - }; - { - auto pairs = [](LuaUi::Content& content) - { - auto next = [](LuaUi::Content& content, size_t i) -> sol::optional> - { - if (i < content.size()) - return std::make_tuple(i + 1, content.at(i)); - else - return sol::nullopt; - }; - return std::make_tuple(next, content, 0); - }; - uiContent[sol::meta_function::ipairs] = pairs; - uiContent[sol::meta_function::pairs] = pairs; - } - auto element = context.mLua->sol().new_usertype("Element"); element["layout"] = sol::property( [](LuaUi::Element& element) @@ -210,12 +147,8 @@ namespace MWLua luaManager->addAction([wm, obj=obj.as()]{ wm->setConsoleSelectedObject(obj.ptr()); }); } }; - api["content"] = [](const sol::table& table) - { - return LuaUi::Content(table); - }; - api["create"] = [context](const sol::table& layout) - { + api["content"] = LuaUi::loadContentConstructor(context.mLua); + api["create"] = [context](const sol::table& layout) { auto element = LuaUi::Element::make(layout); context.mLuaManager->addAction(std::make_unique(UiAction::CREATE, element, context.mLua)); return element; diff --git a/apps/openmw_test_suite/lua/test_ui_content.cpp b/apps/openmw_test_suite/lua/test_ui_content.cpp index f478c618dc..bf33bbd697 100644 --- a/apps/openmw_test_suite/lua/test_ui_content.cpp +++ b/apps/openmw_test_suite/lua/test_ui_content.cpp @@ -1,62 +1,105 @@ #include #include +#include #include namespace { using namespace testing; - sol::state state; - - sol::table makeTable() + struct LuaUiContentTest : Test { - return sol::table(state, sol::create); + LuaUtil::LuaState mLuaState{ nullptr, nullptr }; + sol::protected_function mNew; + LuaUiContentTest() + { + mLuaState.addInternalLibSearchPath("resources/lua_libs"); + mNew = LuaUi::loadContentConstructor(&mLuaState); + } + + LuaUi::ContentView makeContent(sol::table source) + { + auto result = mNew.call(source); + if (result.get_type() != sol::type::table) + throw std::logic_error("Expected table"); + return LuaUi::ContentView(result.get()); + } + + sol::table makeTable() { return sol::table(mLuaState.sol(), sol::create); } + + sol::table makeTable(std::string name) + { + auto result = makeTable(); + result["name"] = name; + return result; + } + }; + + TEST_F(LuaUiContentTest, ProtectedMetatable) + { + mLuaState.sol()["makeContent"] = mNew; + mLuaState.sol()["M"] = makeContent(makeTable()).getMetatable(); + std::string testScript = R"( + assert(not pcall(function() setmetatable(makeContent{}, {}) end), 'Metatable is not protected') + assert(getmetatable(makeContent{}) ~= M, 'Metatable is not protected') + )"; + EXPECT_NO_THROW(mLuaState.sol().safe_script(testScript)); } - sol::table makeTable(std::string name) - { - auto result = makeTable(); - result["name"] = name; - return result; - } - - TEST(LuaUiContentTest, Create) + TEST_F(LuaUiContentTest, Create) { auto table = makeTable(); table.add(makeTable()); table.add(makeTable()); table.add(makeTable()); - LuaUi::Content content(table); + LuaUi::ContentView content = makeContent(table); EXPECT_EQ(content.size(), 3); } - TEST(LuaUiContentTest, CreateWithHole) + TEST_F(LuaUiContentTest, Insert) { auto table = makeTable(); table.add(makeTable()); table.add(makeTable()); - table[4] = makeTable(); - EXPECT_ANY_THROW(LuaUi::Content content(table)); + table.add(makeTable()); + LuaUi::ContentView content = makeContent(table); + content.insert(2, makeTable("inserted")); + EXPECT_EQ(content.size(), 4); + auto inserted = content.at("inserted"); + auto index = content.indexOf(inserted); + EXPECT_TRUE(index.has_value()); + EXPECT_EQ(index.value(), 2); } - TEST(LuaUiContentTest, WrongType) + TEST_F(LuaUiContentTest, MakeHole) + { + auto table = makeTable(); + table.add(makeTable()); + table.add(makeTable()); + LuaUi::ContentView content = makeContent(table); + sol::table t = makeTable(); + EXPECT_ANY_THROW(content.assign(3, t)); + } + + TEST_F(LuaUiContentTest, WrongType) { auto table = makeTable(); table.add(makeTable()); table.add("a"); table.add(makeTable()); - EXPECT_ANY_THROW(LuaUi::Content content(table)); + EXPECT_ANY_THROW(makeContent(table)); } - TEST(LuaUiContentTest, NameAccess) + TEST_F(LuaUiContentTest, NameAccess) { auto table = makeTable(); table.add(makeTable()); table.add(makeTable("a")); - LuaUi::Content content(table); + LuaUi::ContentView content = makeContent(table); EXPECT_NO_THROW(content.at("a")); content.remove("a"); + EXPECT_EQ(content.size(), 1); content.assign(content.size(), makeTable("b")); content.assign("b", makeTable()); EXPECT_ANY_THROW(content.at("b")); @@ -67,31 +110,35 @@ namespace EXPECT_ANY_THROW(content.at("c")); } - TEST(LuaUiContentTest, IndexOf) + TEST_F(LuaUiContentTest, IndexOf) { auto table = makeTable(); table.add(makeTable()); table.add(makeTable()); table.add(makeTable()); - LuaUi::Content content(table); + LuaUi::ContentView content = makeContent(table); auto child = makeTable(); content.assign(2, child); - EXPECT_EQ(content.indexOf(child), 2); - EXPECT_EQ(content.indexOf(makeTable()), content.size()); + EXPECT_EQ(content.indexOf(child).value(), 2); + EXPECT_TRUE(!content.indexOf(makeTable()).has_value()); } - TEST(LuaUiContentTest, BoundsChecks) + TEST_F(LuaUiContentTest, BoundsChecks) { auto table = makeTable(); - LuaUi::Content content(table); + LuaUi::ContentView content = makeContent(table); EXPECT_ANY_THROW(content.at(0)); + EXPECT_EQ(content.size(), 0); content.assign(content.size(), makeTable()); + EXPECT_EQ(content.size(), 1); content.assign(content.size(), makeTable()); + EXPECT_EQ(content.size(), 2); content.assign(content.size(), makeTable()); + EXPECT_EQ(content.size(), 3); EXPECT_ANY_THROW(content.at(3)); EXPECT_ANY_THROW(content.remove(3)); - EXPECT_NO_THROW(content.remove(1)); - EXPECT_NO_THROW(content.at(1)); + content.remove(2); EXPECT_EQ(content.size(), 2); + EXPECT_ANY_THROW(content.at(2)); } } diff --git a/components/CMakeLists.txt b/components/CMakeLists.txt index 29940cef6f..977e972b2b 100644 --- a/components/CMakeLists.txt +++ b/components/CMakeLists.txt @@ -271,6 +271,7 @@ add_component_dir (lua_ui properties widget element util layers content alignment resources adapter text textedit window image container flex ) +copy_resource_file("lua_ui/content.lua" "${OPENMW_RESOURCES_ROOT}" "resources/lua_libs/content.lua") if(WIN32) diff --git a/components/lua_ui/content.cpp b/components/lua_ui/content.cpp index e7cf474bc9..2e1d4ca0c4 100644 --- a/components/lua_ui/content.cpp +++ b/components/lua_ui/content.cpp @@ -2,104 +2,21 @@ namespace LuaUi { - Content::Content(const sol::table& table) + sol::protected_function loadContentConstructor(LuaUtil::LuaState* state) { - size_t size = table.size(); - for (size_t index = 0; index < size; ++index) - { - sol::object value = table.get(index + 1); - if (value.is()) - assign(index, value.as()); - else - throw std::logic_error("UI Content children must all be tables."); - } + sol::function loader = state->loadInternalLib("content"); + sol::set_environment(state->newInternalLibEnvironment(), loader); + sol::table metatable = loader().get(); + if (metatable["new"].get_type() != sol::type::function) + throw std::logic_error("Expected function"); + return metatable["new"].get(); } - void Content::assign(size_t index, const sol::table& table) + bool isValidContent(const sol::object& object) { - if (mOrdered.size() < index) - throw std::logic_error("Can't have gaps in UI Content."); - if (index == mOrdered.size()) - mOrdered.push_back(table); - else - { - sol::optional oldName = mOrdered[index]["name"]; - if (oldName.has_value()) - mNamed.erase(oldName.value()); - mOrdered[index] = table; - } - sol::optional name = table["name"]; - if (name.has_value()) - mNamed[name.value()] = index; - } - - void Content::assign(std::string_view name, const sol::table& table) - { - auto it = mNamed.find(name); - if (it != mNamed.end()) - assign(it->second, table); - else - throw std::logic_error(std::string("Can't find a UI Content child with name ") += name); - } - - void Content::insert(size_t index, const sol::table& table) - { - if (mOrdered.size() < index) - throw std::logic_error("Can't have gaps in UI Content."); - mOrdered.insert(mOrdered.begin() + index, table); - for (size_t i = index; i < mOrdered.size(); ++i) - { - sol::optional name = mOrdered[i]["name"]; - if (name.has_value()) - mNamed[name.value()] = index; - } - } - - sol::table Content::at(size_t index) const - { - if (index > size()) - throw std::logic_error("Invalid UI Content index."); - return mOrdered.at(index); - } - - sol::table Content::at(std::string_view name) const - { - auto it = mNamed.find(name); - if (it == mNamed.end()) - throw std::logic_error("Invalid UI Content name."); - return mOrdered.at(it->second); - } - - size_t Content::remove(size_t index) - { - sol::table table = at(index); - sol::optional name = table["name"]; - if (name.has_value()) - { - auto it = mNamed.find(name.value()); - if (it != mNamed.end()) - mNamed.erase(it); - } - mOrdered.erase(mOrdered.begin() + index); - return index; - } - - size_t Content::remove(std::string_view name) - { - auto it = mNamed.find(name); - if (it == mNamed.end()) - throw std::logic_error("Invalid UI Content name."); - size_t index = it->second; - remove(index); - return index; - } - - size_t Content::indexOf(const sol::table& table) - { - auto it = std::find(mOrdered.begin(), mOrdered.end(), table); - if (it == mOrdered.end()) - return size(); - else - return it - mOrdered.begin(); + if (object.get_type() != sol::type::table) + return false; + sol::table table = object; + return table.traverse_get>(sol::metatable_key, "__Content").value_or(false); } } diff --git a/components/lua_ui/content.hpp b/components/lua_ui/content.hpp index e970744b3d..2caa1ff8dc 100644 --- a/components/lua_ui/content.hpp +++ b/components/lua_ui/content.hpp @@ -6,36 +6,105 @@ #include +#include + namespace LuaUi { - class Content + sol::protected_function loadContentConstructor(LuaUtil::LuaState* state); + + bool isValidContent(const sol::object& object); + + class ContentView { - public: - using iterator = std::vector::iterator; + public: + // accepts only Lua tables returned by ui.content + explicit ContentView(sol::table table) + : mTable(std::move(table)) + { + if (!isValidContent(mTable)) + throw std::domain_error("Expected a Content table"); + } - Content() {} + size_t size() const { return mTable.size(); } - // expects a Lua array - a table with keys from 1 to n without any nil values in between - // any other keys are ignored - explicit Content(const sol::table&); + void assign(std::string_view name, const sol::table& table) + { + if (indexOf(name).has_value()) + mTable[name] = table; + else + throw std::domain_error("Invalid Content key"); + } + void assign(size_t index, const sol::table& table) + { + if (index <= size()) + mTable[toLua(index)] = table; + else + throw std::range_error("Invalid Content index"); + } + void insert(size_t index, const sol::table& table) { callMethod("insert", toLua(index), table); } - size_t size() const { return mOrdered.size(); } + sol::table at(size_t index) const + { + if (index < size()) + return mTable.get(toLua(index)); + else + throw std::range_error("Invalid Content index"); + } + sol::table at(std::string_view name) const + { + if (indexOf(name).has_value()) + return mTable.get(name); + else + throw std::range_error("Invalid Content key"); + } + void remove(size_t index) + { + if (index < size()) + // for some reason mTable[key] = value doesn't call __newindex + getMetatable()[sol::meta_function::new_index].get()( + mTable, toLua(index), sol::nil); + else + throw std::range_error("Invalid Content index"); + } + void remove(std::string_view name) + { + auto index = indexOf(name); + if (index.has_value()) + remove(index.value()); + else + throw std::domain_error("Invalid Content key"); + } + std::optional indexOf(std::string_view name) const + { + sol::object result = callMethod("indexOf", name); + if (result.is()) + return fromLua(result.as()); + else + return std::nullopt; + } + std::optional indexOf(const sol::table& table) const + { + sol::object result = callMethod("indexOf", table); + if (result.is()) + return fromLua(result.as()); + else + return std::nullopt; + } - void assign(std::string_view name, const sol::table& table); - void assign(size_t index, const sol::table& table); - void insert(size_t index, const sol::table& table); + sol::table getMetatable() const { return mTable[sol::metatable_key].get(); } - sol::table at(size_t index) const; - sol::table at(std::string_view name) const; - size_t remove(size_t index); - size_t remove(std::string_view name); - size_t indexOf(const sol::table& table); + private: + sol::table mTable; - private: - std::map> mNamed; - std::vector mOrdered; + template + sol::object callMethod(std::string_view name, Arg&&... arg) const + { + return mTable.get(name)(mTable, arg...); + } + + static inline size_t toLua(size_t index) { return index + 1; } + static inline size_t fromLua(size_t index) { return index - 1; } }; - } #endif // COMPONENTS_LUAUI_CONTENT diff --git a/components/lua_ui/content.lua b/components/lua_ui/content.lua new file mode 100644 index 0000000000..cd63efc7ca --- /dev/null +++ b/components/lua_ui/content.lua @@ -0,0 +1,140 @@ +local M = {} +M.__Content = true +M.new = function(source) + local result = {} + result.__nameIndex = {} + for i, v in ipairs(source) do + if type(v) ~= 'table' then + error('Content can only contain tables') + end + result[i] = v + if type(v.name) == 'string' then + result.__nameIndex[v.name] = i + end + end + return setmetatable(result, M) +end +local function validateIndex(self, index) + if type(index) ~= 'number' then + error('Unexpected Content key: ' .. tostring(index)) + end + if index < 1 or (#self + 1) < index then + error('Invalid Content index: ' .. tostring(index)) + end +end + +local function getIndexFromKey(self, key) + local index = key + if type(key) == 'string' then + index = self.__nameIndex[key] + if not index then + error('Unexpected content key:' .. key) + end + end + validateIndex(self, index) + return index +end + +local methods = { + insert = function(self, index, value) + validateIndex(self, index) + if type(value) ~= 'table' then + error('Content can only contain tables') + end + for i = #self, index, -1 do + rawset(self, i + 1, rawget(self, i)) + local name = rawget(self, i + 1) + if name then + self.__nameIndex[name] = i + 1 + end + end + rawset(self, index, value) + if value.name then + self.__nameIndex[value.name] = index + end + end, + indexOf = function(self, value) + if type(value) == 'string' then + return self.__nameIndex[value] + elseif type(value) == 'table' then + for i = 1, #self do + if rawget(self, i) == value then + return i + end + end + end + return nil + end, + add = function(self, value) + self:insert(#self + 1, value) + return #self + end, +} +M.__index = function(self, key) + if methods[key] then return methods[key] end + local index = getIndexFromKey(self, key) + return rawget(self, index) +end +local function nameAt(self, index) + local v = rawget(self, index) + return v and type(v.name) == 'string' and v.name +end + +local function remove(self, index) + local oldName = nameAt(self, index) + if oldName then + self.__nameIndex[oldName] = nil + end + if index > #self then + error('Invalid Content index:' .. tostring(index)) + end + for i = index, #self - 1 do + local v = rawget(self, i + 1) + rawset(self, i, v) + if type(v.name) == 'string' then + self.__nameIndex[v.name] = i + end + end + rawset(self, #self, nil) +end + +local function assign(self, index, value) + local oldName = nameAt(self, index) + if oldName then + self.__nameIndex[oldName] = nil + end + rawset(self, index, value) + if value.name then + self.__nameIndex[value.name] = index + end +end + +M.__newindex = function(self, key, value) + local index = getIndexFromKey(self, key) + if value == nil then + remove(self, index) + elseif type(value) == 'table' then + assign(self, index, value) + else + error('Content can only contain tables') + end +end +M.__tostring = function(self) + return ('UiContent{%d layouts}'):format(#self) +end +local function next(self, index) + local v = rawget(self, index) + if v then + return index + 1, v + else + return nil, nil + end +end + +M.__pairs = function(self) + return next, self, 1 +end +M.__ipairs = M.__pairs +M.__metatable = {} + +return M diff --git a/components/lua_ui/element.cpp b/components/lua_ui/element.cpp index bd9f84fec2..df400109d3 100644 --- a/components/lua_ui/element.cpp +++ b/components/lua_ui/element.cpp @@ -62,9 +62,7 @@ namespace LuaUi destroyWidget(w); return result; } - if (!contentObj.is()) - throw std::logic_error("Layout content field must be a openmw.ui.content"); - Content content = contentObj.as(); + ContentView content(contentObj.as()); result.resize(content.size()); size_t minSize = std::min(children.size(), content.size()); for (size_t i = 0; i < minSize; i++) diff --git a/files/lua_api/openmw/ui.lua b/files/lua_api/openmw/ui.lua index 164d8768a9..1638aa4e71 100644 --- a/files/lua_api/openmw/ui.lua +++ b/files/lua_api/openmw/ui.lua @@ -172,6 +172,12 @@ -- for i = 1, #content do -- print('widget',content[i].name,'at',i) -- end +-- @usage +-- -- Note: layout names can collide with method names. Because of that you can't use a layout name such as "insert": +-- local content = ui.content { +-- { name = 'insert '} +-- } +-- content.insert.content = ui.content {} -- fails here, content.insert is a function! --- -- Puts the layout at given index by shifting all the elements after it