Implement review comments

- moved some code from header to source - reordered some functions to allow above - made some "variables" const
luanti-org · Oct 7, 2024 · 927f030 · 927f030
1 parent 9b2c04b
commit 927f030
Show file tree

Hide file tree

Showing 2 changed files with 59 additions and 62 deletions.
diff --git a/src/content/subgames.cpp b/src/content/subgames.cpp
@@ -83,6 +83,8 @@ struct GameFindPath
 	GameFindPath() { }
 };
 
+using GamePathMap = std::unordered_map<std::string, GameFindPath>;
+
 
 std::string getSubgamePathEnv()
 {
@@ -154,6 +156,59 @@ static SubgameSpec getSubgameSpec(const std::string &game_id,
 	return spec;
 }
 
+GamePathMap getAvailableGamePaths()
+{
+	GamePathMap gamepaths;
+	std::vector<GameFindPath> game_search_paths{
+		{porting::path_share + DIR_DELIM + "games", false},
+		{porting::path_user + DIR_DELIM + "games", true}
+	};
+
+	Strfnd search_paths(getSubgamePathEnv());
+
+	while (!search_paths.at_end())
+		game_search_paths.emplace_back(search_paths.next(PATH_DELIM), false);
+
+	for (const GameFindPath &search_path : game_search_paths) {
+		std::vector<fs::DirListNode> dirlist = fs::GetDirListing(search_path.path);
+		for (const fs::DirListNode &dln : dirlist) {
+			if (!dln.dir)
+				continue;
+
+			// If configuration file is not found or broken, ignore game
+			Settings conf;
+			std::string conf_path = search_path.path + DIR_DELIM + dln.name +
+						DIR_DELIM + "game.conf";
+			if (!conf.readConfigFile(conf_path.c_str()))
+				continue;
+
+			// Add it to result
+			gamepaths[normalizeGameId(dln.name)]
+				= {search_path.path + DIR_DELIM + dln.name, search_path.user_specific};
+		}
+	}
+	return gamepaths;
+}
+
+std::set<std::string> getAvailableGameIds()
+{
+	GamePathMap gamepaths = getAvailableGamePaths();
+	std::set<std::string> gameids;
+	for (auto &&p : gamepaths)
+		gameids.insert(p.first);
+	return gameids;
+}
+
+std::vector<SubgameSpec> getAvailableGames()
+{
+	std::vector<SubgameSpec> specs;
+	std::set<std::string> gameids = getAvailableGameIds();
+	specs.reserve(gameids.size());
+	for (const auto &gameid : gameids)
+		specs.push_back(findSubgame(gameid));
+	return specs;
+}
+
 SubgameSpec findSubgame(const std::string &id)
 {
 	if (id.empty())
@@ -186,14 +241,14 @@ SubgameSpec findSubgame(const std::string &id)
 		return SubgameSpec();
 
 	// Found the game, proceed
-	GameFindPath &data = found->second;
-	std::string &game_path = data.path;
+	const GameFindPath &data = found->second;
+	const std::string &game_path = data.path;
 	bool user_game = data.user_specific;
 
 
 	// Find mod directories
-	std::string share = porting::path_share;
-	std::string user = porting::path_user;
+	const std::string &share = porting::path_share;
+	const std::string &user = porting::path_user;
 	std::unordered_map<std::string, std::string> mods_paths;
 	mods_paths["mods"] = user + DIR_DELIM + "mods";
 	if (!user_game && user != share)
@@ -222,59 +277,6 @@ SubgameSpec findWorldSubgame(const std::string &world_path)
 	return findSubgame(world_gameid);
 }
 
-GamePathMap getAvailableGamePaths()
-{
-	GamePathMap gamepaths;
-	std::vector<GameFindPath> game_search_paths{
-		{porting::path_share + DIR_DELIM + "games", false},
-		{porting::path_user + DIR_DELIM + "games", true}
-	};
-
-	Strfnd search_paths(getSubgamePathEnv());
-
-	while (!search_paths.at_end())
-		game_search_paths.emplace_back(search_paths.next(PATH_DELIM), false);
-
-	for (const GameFindPath &search_path : game_search_paths) {
-		std::vector<fs::DirListNode> dirlist = fs::GetDirListing(search_path.path);
-		for (const fs::DirListNode &dln : dirlist) {
-			if (!dln.dir)
-				continue;
-
-			// If configuration file is not found or broken, ignore game
-			Settings conf;
-			std::string conf_path = search_path.path + DIR_DELIM + dln.name +
-						DIR_DELIM + "game.conf";
-			if (!conf.readConfigFile(conf_path.c_str()))
-				continue;
-
-			// Add it to result
-			gamepaths[normalizeGameId(dln.name)]
-				= {search_path.path + DIR_DELIM + dln.name, search_path.user_specific};
-		}
-	}
-	return gamepaths;
-}
-
-std::set<std::string> getAvailableGameIds()
-{
-	GamePathMap gamepaths = getAvailableGamePaths();
-	std::set<std::string> gameids;
-	for (auto &&p : gamepaths)
-		gameids.insert(p.first);
-	return gameids;
-}
-
-std::vector<SubgameSpec> getAvailableGames()
-{
-	std::vector<SubgameSpec> specs;
-	std::set<std::string> gameids = getAvailableGameIds();
-	specs.reserve(gameids.size());
-	for (const auto &gameid : gameids)
-		specs.push_back(findSubgame(gameid));
-	return specs;
-}
-
 #define LEGACY_GAMEID "minetest"
 
 bool getWorldExists(const std::string &world_path)

diff --git a/src/content/subgames.h b/src/content/subgames.h
@@ -74,14 +74,9 @@ struct SubgameSpec
 	void checkAndLog() const;
 };
 
-struct GameFindPath;
-
-using GamePathMap = std::unordered_map<std::string, GameFindPath>;
-
 SubgameSpec findSubgame(const std::string &id);
 SubgameSpec findWorldSubgame(const std::string &world_path);
 
-GamePathMap getAvailableGamePaths();
 std::set<std::string> getAvailableGameIds();
 std::vector<SubgameSpec> getAvailableGames();
 // Get the list of paths to mods in the environment variable $MINETEST_MOD_PATH