Codechange: Use vector/unique_ptr to manage network game information. (#13902)

Replaces linked list with manual memory management.

src/network/network.cpp

@@ -650,7 +650,7 @@ public:
 	{
 		Debug(net, 9, "Query::OnFailure(): connection_string={}", this->connection_string);
 
-		NetworkGameList *item = NetworkGameListAddItem(connection_string);
+		NetworkGame *item = NetworkGameListAddItem(connection_string);
 		item->status = NGLS_OFFLINE;
 		item->refreshing = false;
 
@@ -676,7 +676,7 @@ void NetworkQueryServer(const std::string &connection_string)
 	Debug(net, 9, "NetworkQueryServer(): connection_string={}", connection_string);
 
 	/* Mark the entry as refreshing, so the GUI can show the refresh is pending. */
-	NetworkGameList *item = NetworkGameListAddItem(connection_string);
+	NetworkGame *item = NetworkGameListAddItem(connection_string);
 	item->refreshing = true;
 
 	TCPConnecter::Create<TCPQueryConnecter>(connection_string);
@@ -691,12 +691,12 @@ void NetworkQueryServer(const std::string &connection_string)
  * @param never_expire Whether the entry can expire (removed when no longer found in the public listing).
  * @return The entry on the game list.
  */
-NetworkGameList *NetworkAddServer(const std::string &connection_string, bool manually, bool never_expire)
+NetworkGame *NetworkAddServer(const std::string &connection_string, bool manually, bool never_expire)
 {
 	if (connection_string.empty()) return nullptr;
 
 	/* Ensure the item already exists in the list */
-	NetworkGameList *item = NetworkGameListAddItem(connection_string);
+	NetworkGame *item = NetworkGameListAddItem(connection_string);
 	if (item->info.server_name.empty()) {
 		ClearGRFConfigList(item->info.grfconfig);
 		item->info.server_name = connection_string;
@@ -729,14 +729,14 @@ void GetBindAddresses(NetworkAddressList *addresses, uint16_t port)
 	}
 }
 
-/* Generates the list of manually added hosts from NetworkGameList and
+/* Generates the list of manually added hosts from NetworkGame and
  * dumps them into the array _network_host_list. This array is needed
  * by the function that generates the config file. */
 void NetworkRebuildHostList()
 {
 	_network_host_list.clear();
 
-	for (NetworkGameList *item = _network_game_list; item != nullptr; item = item->next) {
+	for (const auto &item : _network_game_list) {
 		if (item->manually) _network_host_list.emplace_back(item->connection_string);
 	}
 }

src/network/network_coordinator.cpp

@@ -151,7 +151,7 @@ bool ClientNetworkCoordinatorSocketHandler::Receive_GC_ERROR(Packet &p)
 			}
 
 			/* Mark the server as offline. */
-			NetworkGameList *item = NetworkGameListAddItem(detail);
+			NetworkGame *item = NetworkGameListAddItem(detail);
 			item->status = NGLS_OFFLINE;
 
 			UpdateNetworkGameWindow();
@@ -247,7 +247,7 @@ bool ClientNetworkCoordinatorSocketHandler::Receive_GC_LISTING(Packet &p)
 		std::string connection_string = p.Recv_string(NETWORK_HOSTNAME_PORT_LENGTH);
 
 		/* Now we know the connection string, we can add it to our list. */
-		NetworkGameList *item = NetworkGameListAddItem(connection_string);
+		NetworkGame *item = NetworkGameListAddItem(connection_string);
 
 		/* Clear any existing GRFConfig chain. */
 		ClearGRFConfigList(item->info.grfconfig);

src/network/network_gamelist.cpp

@@ -20,7 +20,7 @@
 
 #include "../safeguards.h"
 
-NetworkGameList *_network_game_list = nullptr; ///< Game list of this client.
+std::vector<std::unique_ptr<NetworkGame>> _network_game_list; ///< Game list of this client.
 int _network_game_list_version = 0; ///< Current version of all items in the list.
 
 /**
@@ -29,56 +29,36 @@ int _network_game_list_version = 0; ///< Current version of all items in the lis
  * @param connection_string the address of the to-be added item
  * @return a point to the newly added or already existing item
  */
-NetworkGameList *NetworkGameListAddItem(const std::string &connection_string)
+NetworkGame *NetworkGameListAddItem(const std::string &connection_string)
 {
-	NetworkGameList *item, *prev_item;
-
 	/* Parse the connection string to ensure the default port is there. */
 	const std::string resolved_connection_string = ServerAddress::Parse(connection_string, NETWORK_DEFAULT_PORT).connection_string;
 
-	prev_item = nullptr;
+	/* Check if it's already added. */
-	for (item = _network_game_list; item != nullptr; item = item->next) {
+	auto it = std::ranges::find(_network_game_list, resolved_connection_string, &NetworkGame::connection_string);
-		if (item->connection_string == resolved_connection_string) return item;
+	if (it != std::end(_network_game_list)) return it->get();
-		prev_item = item;
-	}
 
-	item = new NetworkGameList(resolved_connection_string);
+	auto &item = _network_game_list.emplace_back(std::make_unique<NetworkGame>(resolved_connection_string));
 	item->info.gamescript_version = -1;
 	item->version = _network_game_list_version;
 
-	if (prev_item == nullptr) {
-		_network_game_list = item;
-	} else {
-		prev_item->next = item;
-	}
-
 	UpdateNetworkGameWindow();
 
-	return item;
+	return item.get();
 }
 
 /**
  * Remove an item from the gamelist linked list
  * @param remove pointer to the item to be removed
  */
-void NetworkGameListRemoveItem(NetworkGameList *remove)
+void NetworkGameListRemoveItem(NetworkGame *remove)
 {
-	NetworkGameList *prev_item = nullptr;
+	auto it = std::ranges::find_if(_network_game_list, [&remove](const auto &item) { return item.get() == remove; });
-	for (NetworkGameList *item = _network_game_list; item != nullptr; item = item->next) {
+	if (it != std::end(_network_game_list)) {
-		if (remove == item) {
+		_network_game_list.erase(it);
-			if (prev_item == nullptr) {
-				_network_game_list = remove->next;
-			} else {
-				prev_item->next = remove->next;
-			}
 
-			delete remove;
+		NetworkRebuildHostList();
-
+		UpdateNetworkGameWindow();
-			NetworkRebuildHostList();
-			UpdateNetworkGameWindow();
-			return;
-		}
-		prev_item = item;
 	}
 }
 
@@ -89,20 +69,8 @@ void NetworkGameListRemoveItem(NetworkGameList *remove)
  */
 void NetworkGameListRemoveExpired()
 {
-	NetworkGameList **prev_item = &_network_game_list;
+	auto it = std::remove_if(std::begin(_network_game_list), std::end(_network_game_list), [](const auto &item) { return !item->manually && item->version < _network_game_list_version; });
-
+	_network_game_list.erase(it, std::end(_network_game_list));
-	for (NetworkGameList *item = _network_game_list; item != nullptr;) {
-		if (!item->manually && item->version < _network_game_list_version) {
-			NetworkGameList *remove = item;
-			item = item->next;
-			*prev_item = item;
-
-			delete remove;
-		} else {
-			prev_item = &item->next;
-			item = item->next;
-		}
-	}
 
 	UpdateNetworkGameWindow();
 }
@@ -113,7 +81,7 @@ void NetworkGameListRemoveExpired()
  */
 void NetworkAfterNewGRFScan()
 {
-	for (NetworkGameList *item = _network_game_list; item != nullptr; item = item->next) {
+	for (const auto &item : _network_game_list) {
 		/* Reset compatibility state */
 		item->info.compatible = item->info.version_compatible;
 

src/network/network_gamelist.h

@@ -15,7 +15,7 @@
 #include "network_type.h"
 
 /** The status a server can be in. */
-enum NetworkGameListStatus : uint8_t {
+enum NetworkGameStatus : uint8_t {
 	NGLS_OFFLINE, ///< Server is offline (or cannot be queried).
 	NGLS_ONLINE,  ///< Server is online.
 	NGLS_FULL,    ///< Server is full and cannot be queried.
@@ -24,23 +24,22 @@ enum NetworkGameListStatus : uint8_t {
 };
 
 /** Structure with information shown in the game list (GUI) */
-struct NetworkGameList {
+struct NetworkGame {
-	NetworkGameList(const std::string &connection_string) : connection_string(connection_string) {}
+	NetworkGame(const std::string &connection_string) : connection_string(connection_string) {}
 
-	NetworkGameInfo info = {};                   ///< The game information of this server.
+	NetworkGameInfo info{};                  ///< The game information of this server.
-	std::string connection_string;               ///< Address of the server.
+	std::string connection_string;           ///< Address of the server.
-	NetworkGameListStatus status = NGLS_OFFLINE; ///< Stats of the server.
+	NetworkGameStatus status = NGLS_OFFLINE; ///< Stats of the server.
-	bool manually = false;                       ///< True if the server was added manually.
+	bool manually = false;                   ///< True if the server was added manually.
-	bool refreshing = false;                     ///< Whether this server is being queried.
+	bool refreshing = false;                 ///< Whether this server is being queried.
-	int version = 0;                             ///< Used to see which servers are no longer available on the Game Coordinator and can be removed.
+	int version = 0;                         ///< Used to see which servers are no longer available on the Game Coordinator and can be removed.
-	NetworkGameList *next = nullptr;             ///< Next pointer to make a linked game list.
 };
 
-extern NetworkGameList *_network_game_list;
+extern std::vector<std::unique_ptr<NetworkGame>> _network_game_list;
 extern int _network_game_list_version;
 
-NetworkGameList *NetworkGameListAddItem(const std::string &connection_string);
+NetworkGame *NetworkGameListAddItem(const std::string &connection_string);
-void NetworkGameListRemoveItem(NetworkGameList *remove);
+void NetworkGameListRemoveItem(NetworkGame *remove);
 void NetworkGameListRemoveExpired();
 
 #endif /* NETWORK_GAMELIST_H */

src/network/network_gui.cpp

@@ -78,7 +78,7 @@ static DropDownList BuildVisibilityDropDownList()
 	return list;
 }
 
-typedef GUIList<NetworkGameList*, std::nullptr_t, StringFilter&> GUIGameServerList;
+typedef GUIList<NetworkGame*, std::nullptr_t, StringFilter&> GUIGameServerList;
 typedef int ServerListPosition;
 static const ServerListPosition SLP_INVALID = -1;
 
@@ -181,8 +181,8 @@ protected:
 	static const std::initializer_list<GUIGameServerList::SortFunction * const> sorter_funcs;
 	static const std::initializer_list<GUIGameServerList::FilterFunction * const> filter_funcs;
 
-	NetworkGameList *server = nullptr; ///< Selected server.
+	NetworkGame *server = nullptr; ///< Selected server.
-	NetworkGameList *last_joined = nullptr; ///< The last joined server.
+	NetworkGame *last_joined = nullptr; ///< The last joined server.
 	GUIGameServerList servers{}; ///< List with game servers.
 	ServerListPosition list_pos = SLP_INVALID; ///< Position of the selected server.
 	Scrollbar *vscroll = nullptr; ///< Vertical scrollbar of the list of servers.
@@ -207,12 +207,12 @@ protected:
 
 		bool found_current_server = false;
 		bool found_last_joined = false;
-		for (NetworkGameList *ngl = _network_game_list; ngl != nullptr; ngl = ngl->next) {
+		for (const auto &ngl : _network_game_list) {
-			this->servers.push_back(ngl);
+			this->servers.push_back(ngl.get());
-			if (ngl == this->server) {
+			if (ngl.get() == this->server) {
 				found_current_server = true;
 			}
-			if (ngl == this->last_joined) {
+			if (ngl.get() == this->last_joined) {
 				found_last_joined = true;
 			}
 		}
@@ -245,7 +245,7 @@ protected:
 	}
 
 	/** Sort servers by name. */
-	static bool NGameNameSorter(NetworkGameList * const &a, NetworkGameList * const &b)
+	static bool NGameNameSorter(NetworkGame * const &a, NetworkGame * const &b)
 	{
 		int r = StrNaturalCompare(a->info.server_name, b->info.server_name, true); // Sort by name (natural sorting).
 		if (r == 0) r = a->connection_string.compare(b->connection_string);
@@ -258,7 +258,7 @@ protected:
 	 * server. If the two servers have the same amount, the one with the
 	 * higher maximum is preferred.
 	 */
-	static bool NGameClientSorter(NetworkGameList * const &a, NetworkGameList * const &b)
+	static bool NGameClientSorter(NetworkGame * const &a, NetworkGame * const &b)
 	{
 		/* Reverse as per default we are interested in most-clients first */
 		int r = a->info.clients_on - b->info.clients_on;
@@ -270,7 +270,7 @@ protected:
 	}
 
 	/** Sort servers by map size */
-	static bool NGameMapSizeSorter(NetworkGameList * const &a, NetworkGameList * const &b)
+	static bool NGameMapSizeSorter(NetworkGame * const &a, NetworkGame * const &b)
 	{
 		/* Sort by the area of the map. */
 		int r = (a->info.map_height) * (a->info.map_width) - (b->info.map_height) * (b->info.map_width);
@@ -280,14 +280,14 @@ protected:
 	}
 
 	/** Sort servers by calendar date. */
-	static bool NGameCalendarDateSorter(NetworkGameList * const &a, NetworkGameList * const &b)
+	static bool NGameCalendarDateSorter(NetworkGame * const &a, NetworkGame * const &b)
 	{
 		auto r = a->info.calendar_date - b->info.calendar_date;
 		return (r != 0) ? r < 0 : NGameClientSorter(a, b);
 	}
 
 	/** Sort servers by the number of ticks the game is running. */
-	static bool NGameTicksPlayingSorter(NetworkGameList * const &a, NetworkGameList * const &b)
+	static bool NGameTicksPlayingSorter(NetworkGame * const &a, NetworkGame * const &b)
 	{
 		if (a->info.ticks_playing == b->info.ticks_playing) {
 			return NGameClientSorter(a, b);
@@ -299,7 +299,7 @@ protected:
 	 * Sort servers by joinability. If both servers are the
 	 * same, prefer the non-passworded server first.
 	 */
-	static bool NGameAllowedSorter(NetworkGameList * const &a, NetworkGameList * const &b)
+	static bool NGameAllowedSorter(NetworkGame * const &a, NetworkGame * const &b)
 	{
 		/* The servers we do not know anything about (the ones that did not reply) should be at the bottom) */
 		int r = a->info.server_revision.empty() - b->info.server_revision.empty();
@@ -332,7 +332,7 @@ protected:
 		}
 	}
 
-	static bool NGameSearchFilter(NetworkGameList * const *item, StringFilter &sf)
+	static bool NGameSearchFilter(NetworkGame * const *item, StringFilter &sf)
 	{
 		assert(item != nullptr);
 		assert((*item) != nullptr);
@@ -348,7 +348,7 @@ protected:
 	 * @param y         from where to draw?
 	 * @param highlight does the line need to be highlighted?
 	 */
-	void DrawServerLine(const NetworkGameList *cur_item, int y, bool highlight) const
+	void DrawServerLine(const NetworkGame *cur_item, int y, bool highlight) const
 	{
 		Rect name = this->GetWidget<NWidgetBase>(WID_NG_NAME)->GetCurrentRect();
 		Rect info = this->GetWidget<NWidgetBase>(WID_NG_INFO)->GetCurrentRect();
@@ -528,7 +528,7 @@ public:
 
 				auto [first, last] = this->vscroll->GetVisibleRangeIterators(this->servers);
 				for (auto it = first; it != last; ++it) {
-					const NetworkGameList *ngl = *it;
+					const NetworkGame *ngl = *it;
 					this->DrawServerLine(ngl, y, ngl == this->server);
 					y += this->resize.step_height;
 				}
@@ -565,7 +565,7 @@ public:
 			this->SortNetworkGameList();
 		}
 
-		NetworkGameList *sel = this->server;
+		NetworkGame *sel = this->server;
 		/* 'Refresh' button invisible if no server selected */
 		this->SetWidgetDisabledState(WID_NG_REFRESH, sel == nullptr);
 		/* 'Join' button disabling conditions */
@@ -610,7 +610,7 @@ public:
 
 	void DrawDetails(const Rect &r) const
 	{
-		NetworkGameList *sel = this->server;
+		NetworkGame *sel = this->server;
 
 		Rect tr = r.Shrink(WidgetDimensions::scaled.frametext);
 		StringID header_msg = this->GetHeaderString();

src/network/network_internal.h

@@ -84,7 +84,7 @@ extern uint8_t _network_reconnect;
 void NetworkQueryServer(const std::string &connection_string);
 
 void GetBindAddresses(NetworkAddressList *addresses, uint16_t port);
-struct NetworkGameList *NetworkAddServer(const std::string &connection_string, bool manually = true, bool never_expire = false);
+struct NetworkGame *NetworkAddServer(const std::string &connection_string, bool manually = true, bool never_expire = false);
 void NetworkRebuildHostList();
 void UpdateNetworkGameWindow();
 

src/network/network_query.cpp

@@ -25,7 +25,7 @@ NetworkRecvStatus QueryNetworkGameSocketHandler::CloseConnection(NetworkRecvStat
 	assert(this->sock != INVALID_SOCKET);
 
 	/* Connection is closed, but we never received a packet. Must be offline. */
-	NetworkGameList *item = NetworkGameListAddItem(this->connection_string);
+	NetworkGame *item = NetworkGameListAddItem(this->connection_string);
 	if (item->refreshing) {
 		item->status = NGLS_OFFLINE;
 		item->refreshing = false;
@@ -91,7 +91,7 @@ NetworkRecvStatus QueryNetworkGameSocketHandler::Receive_SERVER_FULL(Packet &)
 {
 	Debug(net, 9, "Query::Receive_SERVER_FULL()");
 
-	NetworkGameList *item = NetworkGameListAddItem(this->connection_string);
+	NetworkGame *item = NetworkGameListAddItem(this->connection_string);
 	item->status = NGLS_FULL;
 	item->refreshing = false;
 
@@ -104,7 +104,7 @@ NetworkRecvStatus QueryNetworkGameSocketHandler::Receive_SERVER_BANNED(Packet &)
 {
 	Debug(net, 9, "Query::Receive_SERVER_BANNED()");
 
-	NetworkGameList *item = NetworkGameListAddItem(this->connection_string);
+	NetworkGame *item = NetworkGameListAddItem(this->connection_string);
 	item->status = NGLS_BANNED;
 	item->refreshing = false;
 
@@ -117,7 +117,7 @@ NetworkRecvStatus QueryNetworkGameSocketHandler::Receive_SERVER_GAME_INFO(Packet
 {
 	Debug(net, 9, "Query::Receive_SERVER_GAME_INFO()");
 
-	NetworkGameList *item = NetworkGameListAddItem(this->connection_string);
+	NetworkGame *item = NetworkGameListAddItem(this->connection_string);
 
 	/* Clear any existing GRFConfig chain. */
 	ClearGRFConfigList(item->info.grfconfig);
@@ -140,7 +140,7 @@ NetworkRecvStatus QueryNetworkGameSocketHandler::Receive_SERVER_ERROR(Packet &p)
 
 	Debug(net, 9, "Query::Receive_SERVER_ERROR(): error={}", error);
 
-	NetworkGameList *item = NetworkGameListAddItem(this->connection_string);
+	NetworkGame *item = NetworkGameListAddItem(this->connection_string);
 
 	if (error == NETWORK_ERROR_NOT_EXPECTED) {
 		/* If we query a server that is 1.11.1 or older, we get an