[Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/refac-netcode into lp:widelands has been updated. Status: Needs review => Merged For more details, see: https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798 -- Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/refac-netcode. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
Review: Approve Testing done. @bunnybot merge -- https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/refac-netcode. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
Thanks, is done. I used the *Impl structs to hide that we are using SDLNet, but admittedly it doesn't really matter. -- https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/refac-netcode. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
Review: Approve Reviewed the code. lgtm, I left a few nits in the code in the last commit. Grep for NOCOM please. -- https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798 Your team Widelands Developers is subscribed to branch lp:~widelands-dev/widelands/refac-netcode. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
Yes, I wasn't sure about that one. Will download and do a bit of testing. -- https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refac-netcode into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
Thanks for the review! I included nearly all of them in the commit, except for you "Game or Net"-comment which was already correct. -- https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refac-netcode into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
Re: [Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
ublished 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_H
> +#define WL_NETWORK_NETHOST_H
> +
> +#include
> +
> +#include "network/network.h"
> +
> +class NetHostImpl;
> +
> +/**
> + * GameClient manages the lifetime of a network game in which this computer
Update the description
> + * participates as a client.
> + *
> + * This includes running the game setup screen and the actual game after
> + * launch, as well as dealing with the actual network protocol.
> + */
> +class NetHost {
> + public:
> +
> + using ConId = uint32_t;
ConnectionId?
> +
> + /**
> + * Tries to listen on the given port.
> + * @param port The port to listen on.
> + * @return A pointer to a listening \c NetHost object or an
> invalid pointer if the connection failed.
invalid pointer => nullptr
> + */
> + static std::unique_ptr listen(const uint16_t port);
> +
> + /**
> + * Closes the server.
> + */
> + ~NetHost();
> +
> + /**
> + * Returns whether the server is started and is listening.
> + * @return \c true if the server is listening, \c false
> otherwise.
> + */
> + 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(ConId id) const;
> +
> + /**
> + * Stops listening for connections.
> + */
> + void stop_listening();
> +
> + /**
> + * Closes the connection to the given client.
> + * @param id The id of the client to close the connection to.
> + */
> + void close(ConId 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 0.
greater than 0
> + */
> + bool try_accept(ConId& 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(ConId id, RecvPacket& packet);
If the packet's contents are changed, it should be a pointer rather than a
reference -> makes the code easier to read. Same for similar functions.
> +
> + /**
> + * Sends a packet.
> + * Calling this on a closed connection will silently fail.
> + * @param id The connection id of the client that should be
> send to.
send -> sent
> + * @param packet The packet to send.
> + */
> + void send(ConId id, const SendPacket& packet);
> +
> + private:
> + NetHost(const uint16_t port);
> +
> + NetHostImpl *d;
> +};
> +
> +#endif // end of include guard: WL_NETWORK_NETHOST_H
>
> === modified file 'src/network/network.h'
> --- src/network/network.h 2017-01-25 18:55:59 +
> +++ src/network/network.h 2017-05-10 05:43:49 +
> @@ -43,7 +43,7 @@
> };
>
> /**
> - * This non-gamelogic command is used by \ref NetHost and \ref NetClient
> + * This non-gamelogic command is used by \ref GameHost and \ref GameClient
Game or Net?
> * to schedule taking a synchronization hash.
> */
> struct CmdNetCheckSync : public Widelands::Command {
--
https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/refac-netcode into lp:widelands.
___
Mailing list: https://launchpad.net/~widelands-dev
Post to : widelands-dev@lists.launchpad.net
Unsubscribe : https://launchpad.net/~widelands-dev
More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
Continuous integration builds have changed state: Travis build 2164. State: passed. Details: https://travis-ci.org/widelands/widelands/builds/230634393. Appveyor build 1999. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_refac_netcode-1999. -- https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refac-netcode into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
Continuous integration builds have changed state: Travis build 2161. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/230495286. Appveyor build 1996. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_refac_netcode-1996. -- https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refac-netcode into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
The proposal to merge lp:~widelands-dev/widelands/refac-netcode into lp:widelands has been updated. Description changed to: Hint: You don't need to know anything about networking to review this. Moving some code around to hide most SDLNet specific calls inside two classes. This branch should not change anything on the functionality but is a preparation for a future replacement of SDLNet with boost.asio. Renamed the classes NetHost/NetClient to GameHost/GameClient and introduced new classes NetHost/NetClient with the "real" network calls (the SDLNet function calls). Also changed some related code segments in the hope of improving code quality. For more details, see: https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798 -- Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refac-netcode into lp:widelands. ___ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp
[Widelands-dev] [Merge] lp:~widelands-dev/widelands/refac-netcode into lp:widelands
Notabilis has proposed merging lp:~widelands-dev/widelands/refac-netcode into
lp:widelands.
Requested reviews:
Widelands Developers (widelands-dev)
Related bugs:
Bug #1689087 in widelands: "Implementing a relay server"
https://bugs.launchpad.net/widelands/+bug/1689087
For more details, see:
https://code.launchpad.net/~widelands-dev/widelands/refac-netcode/+merge/323798
Hint: You don't need to know anything about networking to review this.
Moving some code around to hide most SDLNet specific calls inside two classes.
This branch should not change anything on the functionality but is a
preparation for a future replacement of SDLNet with boost.asio.
--
Your team Widelands Developers is requested to review the proposed merge of
lp:~widelands-dev/widelands/refac-netcode into lp:widelands.
=== modified file 'src/network/CMakeLists.txt'
--- src/network/CMakeLists.txt 2017-02-28 12:59:39 +
+++ src/network/CMakeLists.txt 2017-05-09 19:19:42 +
@@ -6,6 +6,10 @@
internet_gaming_messages.cc
internet_gaming_messages.h
internet_gaming_protocol.h
+gameclient.cc
+gameclient.h
+gamehost.cc
+gamehost.h
netclient.cc
netclient.h
nethost.cc
=== renamed file 'src/network/netclient.cc' => 'src/network/gameclient.cc'
--- src/network/netclient.cc 2017-02-10 14:12:36 +
+++ src/network/gameclient.cc 2017-05-09 19:19:42 +
@@ -17,7 +17,7 @@
*
*/
-#include "network/netclient.h"
+#include "network/gameclient.h"
#include
@@ -53,19 +53,12 @@
#include "wui/interactive_player.h"
#include "wui/interactive_spectator.h"
-struct NetClientImpl {
+struct GameClientImpl {
GameSettings settings;
std::string localplayername;
- /// The socket that connects us to the host
- TCPsocket sock;
-
- /// Socket set used for selection
- SDLNet_SocketSet sockset;
-
- /// Deserializer acts as a buffer for packets (reassembly/splitting up)
- Deserializer deserializer;
+ std::unique_ptr net;
/// Currently active modal panel. Receives an end_modal on disconnect
UI::Panel* modal;
@@ -96,18 +89,16 @@
std::vector chatmessages;
};
-NetClient::NetClient(IPaddress* const svaddr, const std::string& playername, bool internet)
- : d(new NetClientImpl), internet_(internet) {
- d->sock = SDLNet_TCP_Open(svaddr);
- if (d->sock == nullptr)
+GameClient::GameClient(const std::string& host, const uint16_t port, const std::string& playername, bool internet)
+ : d(new GameClientImpl), internet_(internet) {
+ d->net = NetClient::connect(host, port);
+ if (!d->net || !d->net->is_connected()) {
throw WLWarning(_("Could not establish connection to host"),
_("Widelands could not establish a connection to the given "
"address.\n"
"Either no Widelands server was running at the supposed port or\n"
"the server shut down as you tried to connect."));
-
- d->sockset = SDLNet_AllocSocketSet(1);
- SDLNet_TCP_AddSocket(d->sockset, d->sock);
+ }
d->settings.playernum = UserSettings::not_connected();
d->settings.usernum = -2;
@@ -122,22 +113,21 @@
d->settings.win_condition_script = d->settings.win_condition_scripts.front();
}
-NetClient::~NetClient() {
- if (d->sock != nullptr)
+GameClient::~GameClient() {
+ assert(d->net != nullptr);
+ if (d->net->is_connected())
disconnect("CLIENT_LEFT_GAME", "", true, false);
- SDLNet_FreeSocketSet(d->sockset);
-
delete d;
}
-void NetClient::run() {
+void GameClient::run() {
SendPacket s;
s.unsigned_8(NETCMD_HELLO);
s.unsigned_8(NETWORK_PROTOCOL_VERSION);
s.string(d->localplayername);
s.string(build_id());
- s.send(d->sock);
+ d->net->send(s);
d->settings.multiplayer = true;
@@ -221,7 +211,7 @@
}
}
-void NetClient::think() {
+void GameClient::think() {
handle_network();
if (d->game) {
@@ -242,7 +232,7 @@
}
}
-void NetClient::send_player_command(Widelands::PlayerCommand& pc) {
+void GameClient::send_player_command(Widelands::PlayerCommand& pc) {
assert(d->game);
if (pc.sender() != d->settings.playernum + 1) {
delete
@@ -255,7 +245,7 @@
s.unsigned_8(NETCMD_PLAYERCOMMAND);
s.signed_32(d->game->get_gametime());
pc.serialize(s);
- s.send(d->sock);
+ d->net->send(s);
d->lasttimestamp = d->game->get_gametime();
d->lasttimestamp_realtime = SDL_GetTicks();
@@ -263,15 +253,15 @@
delete
}
-int32_t NetClient::get_frametime() {
+int32_t GameClient::get_frametime() {
return d->time.time() - d->game->get_gametime();
}
-GameController::GameType NetClient::get_game_type() {
+GameController::GameType GameClient::get_game_type() {
return GameController::GameType::NETCLIENT;
}
-void NetClient::report_result(uint8_t player_nr,
+void GameCl
