Merge lp:~widelands-dev/widelands/nethost-split into lp:widelands

[Notabilis]

Extracting an interface class out of the NetHost/NetClient.

Extracting an interface class out of the NetHost/NetClient.

Useless with only this branch but the network relay branch will make use of it.

[GunChleoc (gunchleoc)]

Cod LGTM, not tested.

review: Approve

[GunChleoc (gunchleoc)]

Tested, good to go :)

@bunnybot merge

[Notabilis (notabilis27)]

Thanks!

[GunChleoc (gunchleoc)]

@bunnybot merge

I hope to have time next week to look at your bigger changes.

[kaputtnik (franku)]

Is bunnybot ill? Another try:

@bunnybot merge

[bunnybot (widelandsofficial)]

Continuous integration builds have changed state:

Travis build 2721. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/295046033.
Appveyor build 2533. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_nethost_split-2533.

=== modified file 'src/network/CMakeLists.txt'
--- src/network/CMakeLists.txt	2017-06-26 11:35:50 +0000
+++ src/network/CMakeLists.txt	2017-10-25 10:15:57 +0000
@@ -10,8 +10,10 @@
     gameclient.h
     gamehost.cc
     gamehost.h
+    netclient_interface.h
     netclient.cc
     netclient.h
+    nethost_interface.h
     nethost.cc
     nethost.h
     network.cc

=== modified file 'src/network/gamehost.cc'
--- src/network/gamehost.cc	2017-09-20 21:27:25 +0000
+++ src/network/gamehost.cc	2017-10-25 10:15:57 +0000
@@ -50,6 +50,7 @@
 #include "map_io/widelands_map_loader.h"
 #include "network/constants.h"
 #include "network/internet_gaming.h"
+#include "network/nethost.h"
 #include "network/network_gaming_messages.h"
 #include "network/network_lan_promotion.h"
 #include "network/network_player_settings_backend.h"
@@ -387,7 +388,7 @@
 };
 
 struct Client {
-	NetHost::ConnectionId sock_id;
+	NetHostInterface::ConnectionId sock_id;
 	uint8_t playernum;
 	int16_t usernum;
 	std::string build_id;
@@ -411,7 +412,7 @@
 	NetworkPlayerSettingsBackend npsb;
 
 	LanGamePromoter* promoter;
-	std::unique_ptr<NetHost> net;
+	std::unique_ptr<NetHostInterface> net;
 
 	/// List of connected clients. Note that clients are not in the same
 	/// order as players. In fact, a client must not be assigned to a player.
@@ -1420,12 +1421,14 @@
 
 // Send the packet to all properly connected clients
 void GameHost::broadcast(SendPacket& packet) {
+	std::vector<NetHostInterface::ConnectionId> receivers;
 	for (const Client& client : d->clients) {
 		if (client.playernum != UserSettings::not_connected()) {
 			assert(client.sock_id > 0);
-			d->net->send(client.sock_id, packet);
+			receivers.push_back(client.sock_id);
 		}
 	}
+	d->net->send(receivers, packet);
 }
 
 void GameHost::write_setting_map(SendPacket& packet) {
@@ -2195,7 +2198,7 @@
 	}
 }
 
-void GameHost::send_file_part(NetHost::ConnectionId csock_id, uint32_t part) {
+void GameHost::send_file_part(NetHostInterface::ConnectionId csock_id, uint32_t part) {
 	assert(part < file_->parts.size());
 
 	uint32_t left = file_->bytes - NETFILEPARTSIZE * part;

=== modified file 'src/network/gamehost.h'
--- src/network/gamehost.h	2017-08-11 15:30:42 +0000
+++ src/network/gamehost.h	2017-10-25 10:15:57 +0000
@@ -24,7 +24,7 @@
 #include "logic/game_settings.h"
 #include "logic/player_end_result.h"
 #include "logic/widelands.h"
-#include "network/nethost.h"
+#include "network/nethost_interface.h"
 #include "network/network.h"
 
 struct ChatMessage;
@@ -127,7 +127,7 @@
 
 	void handle_packet(uint32_t i, RecvPacket&);
 	void handle_network();
-	void send_file_part(NetHost::ConnectionId client_sock_id, uint32_t part);
+	void send_file_part(NetHostInterface::ConnectionId client_sock_id, uint32_t part);
 
 	void check_hung_clients();
 	void broadcast_real_speed(uint32_t speed);

=== modified file 'src/network/netclient.h'
--- src/network/netclient.h	2017-07-01 08:22:54 +0000
+++ src/network/netclient.h	2017-10-25 10:15:57 +0000
@@ -22,6 +22,7 @@
 
 #include <memory>
 
+#include "network/netclient_interface.h"
 #include "network/network.h"
 
 /**
@@ -30,7 +31,7 @@
  * This class only tries to create a single socket, either for IPv4 and IPv6.
  * Which is used depends on what kind of address is given on call to connect().
  */
-class NetClient {
+class NetClient : public NetClientInterface {
 public:
 	/**
 	 * Tries to establish a connection to the given host.
@@ -39,10 +40,6 @@
 	 */
 	static std::unique_ptr<NetClient> connect(const NetAddress& host);
 
-	/**
-	 * Closes the connection.
-	 * If you want to send a goodbye-message to the host, do so before freeing the object.
-	 */
 	~NetClient();
 
 	/**
@@ -53,33 +50,11 @@
 	 */
 	bool get_remote_address(NetAddress* addr) const;
 
-	/**
-	 * Returns whether the client is connected.
-	 * \return \c true if the connection is open, \c false otherwise.
-	 */
-	bool is_connected() const;
-
-	/**
-	 * Closes the connection.
-	 * If you want to send a goodbye-message to the host, do so before calling this.
-	 */
-	void close();
-
-	/**
-	 * Tries to receive a packet.
-	 * \param packet A packet that should be overwritten with the received data.
-	 * \return \c true if a packet is available, \c false otherwise.
-	 *   The given packet is only modified when \c true is returned.
-	 *   Calling this on a closed connection will return false.
-	 */
-	bool try_receive(RecvPacket* packet);
-
-	/**
-	 * Sends a packet.
-	 * Calling this on a closed connection will silently fail.
-	 * \param packet The packet to send.
-	 */
-	void send(const SendPacket& packet);
+	// Inherited from NetClientInterface
+	bool is_connected() const override;
+	void close() override;
+	bool try_receive(RecvPacket* packet) override;
+	void send(const SendPacket& packet) override;
 
 private:
 	/**

=== added file 'src/network/netclient_interface.h'
--- src/network/netclient_interface.h	1970-01-01 00:00:00 +0000
+++ src/network/netclient_interface.h	2017-10-25 10:15:57 +0000
@@ -0,0 +1,73 @@
+/*
+ * Copyright (C) 2008-2017 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ */
+
+#ifndef WL_NETWORK_NETCLIENT_INTERFACE_H
+#define WL_NETWORK_NETCLIENT_INTERFACE_H
+
+#include <memory>
+
+#include "network/network.h"
+
+/**
+ * NetClient manages the network connection for a network game in which this computer
+ * participates as a client.
+ * This class provides the interface all NetClient implementation have to follow.
+ * Currently two implementations exists: A "real" NetClient for local games and a
+ * NetClientProxy which relays commands over a relay server.
+ */
+class NetClientInterface {
+public:
+
+	/**
+	 * Closes the connection.
+	 * If you want to send a goodbye-message to the host, do so before freeing the object.
+	 */
+	virtual ~NetClientInterface() {
+	}
+
+	/**
+	 * Returns whether the client is connected.
+	 * \return \c true if the connection is open, \c false otherwise.
+	 */
+	virtual bool is_connected() const = 0;
+
+	/**
+	 * Closes the connection.
+	 * If you want to send a goodbye-message to the host, do so before calling this.
+	 */
+	virtual void close() = 0;
+
+	/**
+	 * Tries to receive a packet.
+	 * \param packet A packet that should be overwritten with the received data.
+	 * \return \c true if a packet is available, \c false otherwise.
+	 *   The given packet is only modified when \c true is returned.
+	 *   Calling this on a closed connection will return false.
+	 */
+	virtual bool try_receive(RecvPacket* packet) = 0;
+
+	/**
+	 * Sends a packet.
+	 * Calling this on a closed connection will silently fail.
+	 * \param packet The packet to send.
+	 */
+	virtual void send(const SendPacket& packet) = 0;
+};
+
+#endif  // end of include guard: WL_NETWORK_NETCLIENT_INTERFACE_H

=== modified file 'src/network/nethost.cc'
--- src/network/nethost.cc	2017-06-19 18:32:06 +0000
+++ src/network/nethost.cc	2017-10-25 10:15:57 +0000
@@ -191,6 +191,12 @@
 	}
 }
 
+void NetHost::send(const std::vector<ConnectionId>& ids, const SendPacket& packet) {
+	for (ConnectionId id : ids) {
+		send(id, packet);
+	}
+}
+
 NetHost::NetHost(const uint16_t port)
    : clients_(), next_id_(1), io_service_(), acceptor_v4_(io_service_), acceptor_v6_(io_service_) {
 

=== modified file 'src/network/nethost.h'
--- src/network/nethost.h	2017-06-24 13:44:38 +0000
+++ src/network/nethost.h	2017-10-25 10:15:57 +0000
@@ -23,17 +23,15 @@
 #include <map>
 #include <memory>
 
-#include "network/network.h"
+#include "network/nethost_interface.h"
 
 /**
  * NetHost manages the client connections of a network game in which this computer
  * participates as a server.
- * This class tries to create sockets for IPv4 and IPv6.
+ * This class tries to create sockets for IPv4 and IPv6 for gaming in the local network.
  */
-class NetHost {
+class NetHost : public NetHostInterface {
 public:
-	/// IDs used to enumerate the clients.
-	using ConnectionId = uint32_t;
 
 	/**
 	 * Tries to listen on the given port.
@@ -47,60 +45,30 @@
 	 */
 	~NetHost();
 
+	// Inherited from NetHostInterface
+	bool is_connected(ConnectionId id) const override;
+	void close(ConnectionId id) override;
+	bool try_accept(ConnectionId* new_id) override;
+	bool try_receive(ConnectionId id, RecvPacket* packet) override;
+	void send(ConnectionId id, const SendPacket& packet) override;
+	void send(const std::vector<ConnectionId>& ids, const SendPacket& packet) override;
+
+private:
+
 	/**
 	 * Returns whether the server is started and is listening.
 	 * \return \c true if the server is listening, \c false otherwise.
 	 */
+	// Feel free to make this method public if you need it
 	bool is_listening() const;
 
 	/**
-	 * Returns whether the given client is connected.
-	 * \param The id of the client to check.
-	 * \return \c true if the connection is open, \c false otherwise.
-	 */
-	bool is_connected(ConnectionId id) const;
-
-	/**
 	 * Stops listening for connections.
 	 */
+	// Feel free to make this method public if you need it
 	void stop_listening();
 
 	/**
-	 * Closes the connection to the given client.
-	 * \param id The id of the client to close the connection to.
-	 */
-	void close(ConnectionId id);
-
-	/**
-	 * Tries to accept a new client.
-	 * \param new_id The connection id of the new client will be stored here.
-	 * \return \c true if a client has connected, \c false otherwise.
-	 *   The given id is only modified when \c true is returned.
-	 *   Calling this on a closed server will return false.
-	 *   The returned id is always greater than 0.
-	 */
-	bool try_accept(ConnectionId* new_id);
-
-	/**
-	 * Tries to receive a packet.
-	 * \param id The connection id of the client that should be received.
-	 * \param packet A packet that should be overwritten with the received data.
-	 * \return \c true if a packet is available, \c false otherwise.
-	 *   The given packet is only modified when \c true is returned.
-	 *   Calling this on a closed connection will return false.
-	 */
-	bool try_receive(ConnectionId id, RecvPacket* packet);
-
-	/**
-	 * Sends a packet.
-	 * Calling this on a closed connection will silently fail.
-	 * \param id The connection id of the client that should be sent to.
-	 * \param packet The packet to send.
-	 */
-	void send(ConnectionId id, const SendPacket& packet);
-
-private:
-	/**
 	 * Tries to listen on the given port.
 	 * If it fails, is_listening() will return \c false.
 	 * \param port The port to listen on.
@@ -129,9 +97,9 @@
 
 	/// A map linking client ids to the respective data about the clients.
 	/// Client ids not in this map should be considered invalid.
-	std::map<NetHost::ConnectionId, Client> clients_;
+	std::map<NetHostInterface::ConnectionId, Client> clients_;
 	/// The next client id that will be used
-	NetHost::ConnectionId next_id_;
+	NetHostInterface::ConnectionId next_id_;
 	/// An io_service needed by boost.asio. Primary needed for async operations.
 	boost::asio::io_service io_service_;
 	/// The acceptor we get IPv4 connection requests to.

=== added file 'src/network/nethost_interface.h'
--- src/network/nethost_interface.h	1970-01-01 00:00:00 +0000
+++ src/network/nethost_interface.h	2017-10-25 10:15:57 +0000
@@ -0,0 +1,93 @@
+/*
+ * Copyright (C) 2008-2017 by the Widelands Development Team
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License
+ * as published by the Free Software Foundation; either version 2
+ * of the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+ *
+ */
+
+#ifndef WL_NETWORK_NETHOST_INTERFACE_H
+#define WL_NETWORK_NETHOST_INTERFACE_H
+
+#include "network/network.h"
+
+/**
+ * A NetHost manages the client connections of a network game in
+ * which this computer participates as a server.
+ * This class provides the interface all NetHost implementation have to follow.
+ * Currently two implementations exists: A "real" NetHost for local games and a
+ * NetHostProxy which relays commands over a relay server.
+ */
+class NetHostInterface {
+public:
+	/// IDs used to enumerate the clients.
+	using ConnectionId = uint8_t;
+
+	/**
+	 * Closes the server.
+	 */
+	virtual ~NetHostInterface() {
+	}
+
+	/**
+	 * Returns whether the given client is connected.
+	 * \param The id of the client to check.
+	 * \return \c true if the connection is open, \c false otherwise.
+	 */
+	virtual bool is_connected(ConnectionId id) const = 0;
+
+	/**
+	 * Closes the connection to the given client.
+	 * \param id The id of the client to close the connection to.
+	 */
+	virtual void close(ConnectionId id) = 0;
+
+	/**
+	 * Tries to accept a new client.
+	 * \param new_id The connection id of the new client will be stored here.
+	 * \return \c true if a client has connected, \c false otherwise.
+	 *   The given id is only modified when \c true is returned.
+	 *   Calling this on a closed server will return false.
+	 *   The returned id is always greater than 0.
+	 */
+	virtual bool try_accept(ConnectionId* new_id) = 0;
+
+	/**
+	 * Tries to receive a packet.
+	 * \param id The connection id of the client that should be received.
+	 * \param packet A packet that should be overwritten with the received data.
+	 * \return \c true if a packet is available, \c false otherwise.
+	 *   The given packet is only modified when \c true is returned.
+	 *   Calling this on a closed connection will return false.
+	 */
+	virtual bool try_receive(ConnectionId id, RecvPacket* packet) = 0;
+
+	/**
+	 * Sends a packet.
+	 * Calling this on a closed connection will silently fail.
+	 * \param id The connection id of the client that should be sent to.
+	 * \param packet The packet to send.
+	 */
+	virtual void send(ConnectionId id, const SendPacket& packet) = 0;
+
+	/**
+	 * Sends a packet to a group of clients.
+	 * Calling this on a closed connection will silently fail.
+	 * \param ids The connection ids of the clients that should be sent to.
+	 * \param packet The packet to send.
+	 */
+	virtual void send(const std::vector<ConnectionId>& ids, const SendPacket& packet) = 0;
+};
+
+#endif  // end of include guard: WL_NETWORK_NETHOST_INTERFACE_H