Add gameid aliases #14543

nauta-turbidus · 2024-04-14T22:59:27Z

Allows defining gameid aliases, so that a game can be completely renamed.

Fixes #13662

Roadmap: 2.2 (refactoring), sort of.

To do

This PR is Ready for Review

update docs

How to test

Create a world under a game.
Rename the folder of the game.
Define the old folder name as aliases in the game.conf file.
Restart Minetest.
Verify if you can still see the world under the renamed game.
Verify if you can still enter the world.

src/content/subgames.cpp

rubenwardy · 2024-04-15T22:50:45Z

You'll need to add support for this in the ContentDB dialog
- Allow name changing aliases for games here https://github.com/minetest/minetest/blob/master/builtin/mainmenu/content/contentdb.lua#L390-L394
- When updating the package after a game rename, it should rename the folder so that the gameid is correct. See start_install https://github.com/minetest/minetest/blob/master/builtin/mainmenu/content/contentdb.lua#L84
- Any name changing aliases should be written to gameid_alias in game.conf here https://github.com/minetest/minetest/blob/master/builtin/mainmenu/content/contentdb.lua#L131
You should probably consistently ignore the alias if a game with that id already exists. This is done for findSubgames but doesn't appear to be done in Lua
You'll need to normalise aliases to remove the _game. Remember that _game is always ignored in a game id.
- Test what happens when you rename aa_game to bb, or aa to bb_game, or aa to bb with _game in the alias, etc
Should gameid_alias be comma separated? Package aliases are a list in ContentDB

src/content/subgames.cpp

appgurueu

Thanks for the PR. Gave the code a look.

builtin/mainmenu/content/contentdb.lua

builtin/mainmenu/tab_local.lua

src/content/subgames.cpp

src/content/subgames.h

src/script/lua_api/l_mainmenu.cpp

rubenwardy

Partial review

builtin/mainmenu/content/contentdb.lua

doc/lua_api.md

builtin/mainmenu/content/contentdb.lua

builtin/mainmenu/init.lua

builtin/mainmenu/tab_local.lua

src/content/subgames.h

builtin/mainmenu/content/contentdb.lua

sfan5 · 2024-05-12T16:30:03Z

doc/lua_api.md

@@ -93,6 +93,10 @@ The game directory can contain the following files:
                 an internal ID used to track versions.
    * `textdomain`: Textdomain used to translate description. Defaults to game id.
      See [Translating content meta](#translating-content-meta).
+	* `aliases = <comma-separated aliases>`
+	  e.g. `aliases = aa,bb` (game has been named "aa" and then "bb" before)


this makes it sound like the order is relevant. is it?

It should also clarify that it's the technical game name, gameid. For example:

* `aliases = <comma-separated gameid aliases>` e.g. `aliases = foo, bar` (where "foo" and "bar" are the legacy names)

appgurueu

(Also tested with void game, works for worlds.)

appgurueu · 2024-05-12T20:32:45Z

src/content/subgames.cpp

@@ -101,6 +101,15 @@ SubgameSpec findSubgame(const std::string &id)
 	std::string share = porting::path_share;
 	std::string user = porting::path_user;

+	auto normalize_game_id = [&](std::string_view name){


Just make this lambda return a std::string so you don't need to cast or construct for every usage, and don't need to worry about lifetimes either. Game names are short and few, this is absolutely irrelevant in terms of performance.

appgurueu · 2024-05-12T20:34:08Z

src/content/subgames.cpp

-		return SubgameSpec();
+	// Failed to find the game, try to find aliased game
+	if (game_path.empty()) {
+		std::vector<GameFindPath> gamespaths;


Style nitpick: You could use a list-initialization rather than emplacing the two paths.

appgurueu · 2024-05-12T20:42:36Z

src/script/lua_api/l_mainmenu.cpp

@@ -332,6 +332,18 @@ int ModApiMainMenu::l_get_games(lua_State *L)
 		lua_pushstring(L,  game.menuicon_path.c_str());
 		lua_settable(L,    top_lvl2);

+		lua_pushstring(L,  "aliases");


Formatting nitpick: Odd argument list spacing. Surrounding code does this in an attempt to align the calls. I'd say just don't.

appgurueu · 2024-05-12T20:43:48Z

src/content/subgames.cpp

@@ -170,14 +224,22 @@ SubgameSpec findSubgame(const std::string &id)
 	if (conf.exists("release"))
 		game_release = conf.getS32("release");

+	std::unordered_set<std::string> aliases;
+	if (conf.exists("aliases"))
+	{


fmt nitpick: if (...) {

appgurueu · 2024-05-12T22:28:44Z

src/content/subgames.cpp

+					std::string conf_path = path + DIR_DELIM + dln.name +
+								DIR_DELIM + "game.conf";
+					if (!conf.readConfigFile(conf_path.c_str()))
+						continue;


The code up to this point - the code for finding games with conf files - should be mostly identical to the code in getAvailableGameIds. Why not extract it into a common function? (Very roughly, I'm imagining something like findGames(std::function<void(Settings)> cb) or std::vector<Settings> findConfFiles or similar)

(Going through all other games for every game to determine aliases also doesn't feel quite right to me, though it's most probably fine in practice.)

wsor4035 added Feature ✨ PRs that add or enhance a feature @ Content / PkgMgr labels Apr 14, 2024

nauta-turbidus marked this pull request as ready for review April 14, 2024 23:42

JosiahWI reviewed Apr 14, 2024

View reviewed changes

src/content/subgames.cpp Outdated Show resolved Hide resolved

sfan5 added the WIP The PR is still being worked on by its author and not ready yet. label Apr 15, 2024

rubenwardy self-assigned this Apr 15, 2024

rubenwardy added the Supported by core dev Not on the roadmap, yet some core dev decided to take care of this PR label Apr 15, 2024

rubenwardy marked this pull request as draft April 15, 2024 23:11

rubenwardy added Action / change needed Code still needs changes (PR) / more information requested (Issues) and removed Action / change needed Code still needs changes (PR) / more information requested (Issues) labels Apr 17, 2024

JosiahWI reviewed Apr 17, 2024

View reviewed changes

src/content/subgames.cpp Outdated Show resolved Hide resolved

sfan5 reviewed Apr 17, 2024

View reviewed changes

src/content/subgames.cpp Outdated Show resolved Hide resolved

nauta-turbidus force-pushed the game_rebranding branch from 7dc075d to 3e76d71 Compare April 17, 2024 17:52

nauta-turbidus marked this pull request as ready for review April 17, 2024 19:08

rubenwardy changed the title ~~WIP: Add gameid aliases~~ Add gameid aliases Apr 17, 2024

rubenwardy removed the WIP The PR is still being worked on by its author and not ready yet. label Apr 17, 2024

rubenwardy self-requested a review April 17, 2024 19:29

appgurueu reviewed Apr 20, 2024

View reviewed changes

rubenwardy reviewed Apr 20, 2024

View reviewed changes

nauta-turbidus added 10 commits April 21, 2024 21:06

Add gameid aliases

b237a53

Worlds are displayed according to alias

2660d92

Alias lookup in another place

eb7778f

Exposed gameid_alias directly to Lua

3310aeb

Lua consistently checks for nonalias game first

a0a25c6

Ignore _game suffix when resolving aliases

44d730e

Allow any CDB package aliases

631d48d

Update comments and documentation

be8f4de

Support comma-separated aliases in C++

5cfb356

Trim the alias and refactor with lambda

fb5b7e0

nauta-turbidus added 2 commits April 21, 2024 21:06

Refactor to utilize std::stringview::substr()

c7bfcd4

Update now works with package rename

627db36

nauta-turbidus force-pushed the game_rebranding branch from d96d51c to 627db36 Compare April 21, 2024 19:10

nauta-turbidus added 2 commits April 21, 2024 23:38

Refactor Lua code handling aliases

6d59c9d

Refactor cpp code and update docs, adjust lua

625601b

nauta-turbidus requested review from rubenwardy and appgurueu April 21, 2024 22:39

sfan5 reviewed May 12, 2024

View reviewed changes

appgurueu reviewed May 12, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add gameid aliases #14543

Add gameid aliases #14543

nauta-turbidus commented Apr 14, 2024 •

edited by appgurueu

rubenwardy commented Apr 15, 2024 •

edited

appgurueu left a comment

rubenwardy left a comment

sfan5 May 12, 2024

SmallJoker May 12, 2024

appgurueu left a comment

appgurueu May 12, 2024

appgurueu May 12, 2024

appgurueu May 12, 2024

appgurueu May 12, 2024

appgurueu May 12, 2024

Add gameid aliases #14543

Are you sure you want to change the base?

Add gameid aliases #14543

Conversation

nauta-turbidus commented Apr 14, 2024 • edited by appgurueu

To do

How to test

rubenwardy commented Apr 15, 2024 • edited

appgurueu left a comment

Choose a reason for hiding this comment

rubenwardy left a comment

Choose a reason for hiding this comment

sfan5 May 12, 2024

Choose a reason for hiding this comment

SmallJoker May 12, 2024

Choose a reason for hiding this comment

appgurueu left a comment

Choose a reason for hiding this comment

appgurueu May 12, 2024

Choose a reason for hiding this comment

appgurueu May 12, 2024

Choose a reason for hiding this comment

appgurueu May 12, 2024

Choose a reason for hiding this comment

appgurueu May 12, 2024

Choose a reason for hiding this comment

appgurueu May 12, 2024

Choose a reason for hiding this comment

nauta-turbidus commented Apr 14, 2024 •

edited by appgurueu

rubenwardy commented Apr 15, 2024 •

edited