Rewrite pkgmgr and contentdb in C++ #14763
Conversation
swagtoy
changed the title
|
Note that per our coding style, functions that are not methods should use snake_case naming. |
|
@JosiahWI The usage of snake case in the lua bound methods seems consistent, so there's nothing wrong there. |
src/content/content.h
Outdated
| @@ -50,6 +55,10 @@ struct ContentSpec | |||
| std::string textdomain; | |||
| }; | |||
|
|
|||
| std::string contentTypeToString(ContentType &t); | |||
There was a problem hiding this comment.
This should be content_type_to_string.
There was a problem hiding this comment.
Look 2 lines below that function. Those were in camelCase as well and I didn't write those. What do you want me to do there?
There was a problem hiding this comment.
I've rewritten that function's name but left the other 2 alone. If you want me to change those, do tell.
There was a problem hiding this comment.
Those are methods as opposed to non-method functions and therefore are named in the correct format. I don't know why we name methods and non-methods differently - that's just the way it is in our style guide. 😄
There was a problem hiding this comment.
They aren't methods, they are just wrapped in a namespace. They are regular functions. They really should be camelCase
| static bool isValidModname(const std::string& str); | ||
| static std::string getContentdbId(const ContentSpec& content); |
There was a problem hiding this comment.
Yes, I see now, because these are static they should be snake_case.
| static void Initialize(lua_State *L); | ||
| static void InitializeAsync(lua_State *L, int top); |
Rewrites pkgmgr and contentdb functions in C++, as well as tests. Will close #12295.
The intention of this PR is the start of adding a CLI and TUI interface for mod management, updating, configuration, which I plan to do later in a separate PR. Of course, that will close in the future #7358 (along with an additional ncurses TUI) and some other features.
It was discussed in #minetest-dev if it was worth just loading a Lua runtime on the CLI. However, since tests weren't written yet, this rewrite shouldn't be hard, and an issue was already requesting it, I figured I would tackle it. It will also add a convenient C++ API for these functions without a Lua dependency.
To do
ATM I got the base done here and one simple function ported to wrap my head around the minetest-lua codebase. This is a draft, please do not merge.
How to test
Coming soon in theaters June 2024