Merge pull request #305 from vcmi/networkImprovements

Network and multiplayer improvements

client/CMT.cpp

@@ -1273,8 +1273,11 @@ static void mainLoop()
 
 void startGame(StartInfo * options, CConnection *serv/* = nullptr*/)
 {
+	if(!CServerHandler::DO_NOT_START_SERVER)
+	{
 		serverAlive.waitWhileTrue();
 		serverAlive.setn(true);
+	}
 
 	if(vm.count("onlyAI"))
 	{

client/CPreGame.cpp

@@ -83,7 +83,7 @@ struct EvilHlpStruct
 
 	void reset()
 	{
-		vstd::clear_pointer(serv);
+//		vstd::clear_pointer(serv);
 		vstd::clear_pointer(sInfo);
 	}
 

client/Client.cpp

@@ -391,7 +391,7 @@ void CClient::newGame( CConnection *con, StartInfo *si )
 	else
 	{
 		serv = con;
-		networkMode = (con->connectionID == 1) ? HOST : GUEST;
+		networkMode = con->isHost() ? HOST : GUEST;
 	}
 
 	CConnection &c = *serv;
@@ -693,7 +693,7 @@ void CClient::stopConnection()
 {
 	terminate = true;
 
-	if (serv) //request closing connection
+	if (serv && serv->isHost()) //request closing connection
 	{
 		logNetwork->infoStream() << "Connection has been requested to be closed.";
 		boost::unique_lock<boost::mutex>(*serv->wmx);
@@ -701,6 +701,12 @@ void CClient::stopConnection()
 		sendRequest(&close_server, PlayerColor::NEUTRAL);
 		logNetwork->infoStream() << "Sent closing signal to the server";
 	}
+	else
+	{
+		LeaveGame leave_Game;
+		sendRequest(&leave_Game, PlayerColor::NEUTRAL);
+		logNetwork->infoStream() << "Sent leaving signal to the server";
+	}
 
 	if(connectionHandler)//end connection handler
 	{
@@ -708,16 +714,13 @@ void CClient::stopConnection()
 			connectionHandler->join();
 
 		logNetwork->infoStream() << "Connection handler thread joined";
-
-		delete connectionHandler;
-		connectionHandler = nullptr;
+		vstd::clear_pointer(connectionHandler);
 	}
 
 	if (serv) //and delete connection
 	{
 		serv->close();
-		delete serv;
-		serv = nullptr;
+		vstd::clear_pointer(serv);
 		logNetwork->warnStream() << "Our socket has been closed.";
 	}
 }
@@ -966,11 +969,14 @@ void CServerHandler::waitForServer()
 	th.update();
 
 #ifndef VCMI_ANDROID
+	if(shared)
+	{
 		intpr::scoped_lock<intpr::interprocess_mutex> slock(shared->sr->mutex);
 		while(!shared->sr->ready)
 		{
 			shared->sr->cond.wait(slock);
 		}
+	}
 #else
 	logNetwork->infoStream() << "waiting for server";
 	while (!androidTestServerReadyFlag.load())
@@ -988,8 +994,12 @@ void CServerHandler::waitForServer()
 CConnection * CServerHandler::connectToServer()
 {
 #ifndef VCMI_ANDROID
+	if(shared)
+	{
 		if(!shared->sr->ready)
 			waitForServer();
+		port = boost::lexical_cast<std::string>(shared->sr->port);
+	}
 #else
 	waitForServer();
 #endif
@@ -1015,6 +1025,9 @@ CServerHandler::CServerHandler(bool runServer /*= false*/)
 	verbose = true;
 
 #ifndef VCMI_ANDROID
+	if(DO_NOT_START_SERVER)
+		return;
+
 	boost::interprocess::shared_memory_object::remove("vcmi_memory"); //if the application has previously crashed, the memory may not have been removed. to avoid problems - try to destroy it
 	try
 	{
@@ -1022,6 +1035,7 @@ CServerHandler::CServerHandler(bool runServer /*= false*/)
 	}
 	catch(...)
 	{
+		vstd::clear_pointer(shared);
 		logNetwork->error("Cannot open interprocess memory.");
 		handleException();
 		throw;
@@ -1040,7 +1054,12 @@ void CServerHandler::callServer()
 #ifndef VCMI_ANDROID
 	setThreadName("CServerHandler::callServer");
 	const std::string logName = (VCMIDirs::get().userCachePath() / "server_log.txt").string();
-	const std::string comm = VCMIDirs::get().serverPath().string() + " --port=" + port + " > \"" + logName + '\"';
+	const std::string comm = VCMIDirs::get().serverPath().string()
+		+ " --port=" + port
+		+ " --run-by-client"
+		+ (shared ? " --use-shm" : "")
+		+ " > \"" + logName + '\"';
+
 	int result = std::system(comm.c_str());
 	if (result == 0)
 	{
@@ -1075,6 +1094,7 @@ CConnection * CServerHandler::justConnectToServer(const std::string &host, const
 			ret = new CConnection(	host.size() ? host : settings["server"]["server"].String(),
 									realPort,
 									NAME);
+			ret->connectionID = 1; // TODO: Refactoring for the server so IDs set outside of CConnection
 		}
 		catch(...)
 		{

lib/Interprocess.h

@@ -19,19 +19,22 @@
 struct ServerReady
 {
 	bool ready;
+	uint16_t port; //ui16?
 	boost::interprocess::interprocess_mutex  mutex;
 	boost::interprocess::interprocess_condition  cond;
 
 	ServerReady()
 	{
 		ready = false;
+		port = 0;
 	}
 
-	void setToTrueAndNotify()
+	void setToTrueAndNotify(uint16_t Port)
 	{
 		{
 			boost::unique_lock<boost::interprocess::interprocess_mutex> lock(mutex); 
 			ready = true;
+			port = Port;
 		}
 		cond.notify_all();
 	}

lib/NetPacks.h

@@ -152,6 +152,21 @@ struct PlayerBlocked : public CPackForClient
 	}
 };
 
+struct PlayerCheated : public CPackForClient
+{
+	PlayerCheated() : losingCheatCode(false), winningCheatCode(false) {}
+	DLL_LINKAGE void applyGs(CGameState *gs);
+
+	PlayerColor player;
+	bool losingCheatCode;
+	bool winningCheatCode;
+
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{
+		h & player & losingCheatCode & winningCheatCode;
+	}
+};
+
 struct YourTurn : public CPackForClient
 {
 	YourTurn(){}
@@ -1774,6 +1789,13 @@ struct CloseServer : public CPackForServer
 	{}
 };
 
+struct LeaveGame : public CPackForServer
+{
+	bool applyGh(CGameHandler *gh);
+	template <typename Handler> void serialize(Handler &h, const int version)
+	{}
+};
+
 struct EndTurn : public CPackForServer
 {
 	bool applyGh(CGameHandler *gh);

lib/NetPacksLib.cpp

@@ -1840,6 +1840,12 @@ DLL_LINKAGE void BattleSetStackProperty::applyGs(CGameState *gs)
 	}
 }
 
+DLL_LINKAGE void PlayerCheated::applyGs(CGameState *gs)
+{
+	gs->getPlayer(player)->enteredLosingCheatCode = losingCheatCode;
+	gs->getPlayer(player)->enteredWinningCheatCode = winningCheatCode;
+}
+
 DLL_LINKAGE void YourTurn::applyGs(CGameState *gs)
 {
 	gs->currentPlayer = player;

lib/registerTypes/RegisterTypes.h

@@ -210,6 +210,7 @@ void registerTypesClientPacks1(Serializer &s)
 	s.template registerType<CPackForClient, PackageApplied>();
 	s.template registerType<CPackForClient, SystemMessage>();
 	s.template registerType<CPackForClient, PlayerBlocked>();
+	s.template registerType<CPackForClient, PlayerCheated>();
 	s.template registerType<CPackForClient, YourTurn>();
 	s.template registerType<CPackForClient, SetResources>();
 	s.template registerType<CPackForClient, SetPrimSkill>();
@@ -314,6 +315,7 @@ void registerTypesServerPacks(Serializer &s)
 {
 	s.template registerType<CPack, CPackForServer>();
 	s.template registerType<CPackForServer, CloseServer>();
+	s.template registerType<CPackForServer, LeaveGame>();
 	s.template registerType<CPackForServer, EndTurn>();
 	s.template registerType<CPackForServer, DismissHero>();
 	s.template registerType<CPackForServer, MoveHero>();

lib/serializer/Connection.cpp

@@ -191,8 +191,7 @@ void CConnection::close()
 	if(socket)
 	{
 		socket->close();
-		delete socket;
-		socket = nullptr;
+		vstd::clear_pointer(socket);
 	}
 }
 
@@ -201,6 +200,11 @@ bool CConnection::isOpen() const
 	return socket && connected;
 }
 
+bool CConnection::isHost() const
+{
+	return connectionID == 1;
+}
+
 void CConnection::reportState(CLogger * out)
 {
 	out->debugStream() << "CConnection";

lib/serializer/Connection.h

@@ -74,6 +74,7 @@ public:
 
 	void close();
 	bool isOpen() const;
+	bool isHost() const;
 	template<class T>
 	CConnection &operator&(const T&);
 	virtual ~CConnection(void);

server/CGameHandler.cpp

@@ -1031,6 +1031,24 @@ void CGameHandler::handleConnection(std::set<PlayerColor> players, CConnection &
 {
 	setThreadName("CGameHandler::handleConnection");
 
+	auto handleDisconnection = [&](const std::exception & e)
+	{
+		assert(!c.connected); //make sure that connection has been marked as broken
+		logGlobal->error(e.what());
+		conns -= &c;
+		for(auto playerConn : connections)
+		{
+			if(playerConn.second == &c)
+			{
+				PlayerCheated pc;
+				pc.player = playerConn.first;
+				pc.losingCheatCode = true;
+				sendAndApply(&pc);
+				checkVictoryLossConditionsForPlayer(playerConn.first);
+			}
+		}
+	};
+
 	try
 	{
 		while(1)//server should never shut connection first //was: while(!end2)
@@ -1042,6 +1060,8 @@ void CGameHandler::handleConnection(std::set<PlayerColor> players, CConnection &
 
 			{
 				boost::unique_lock<boost::mutex> lock(*c.rmx);
+				if(!c.connected)
+					throw clientDisconnectedException();
 				c >> player >> requestID >> pack; //get the package
 
 				if (!pack)
@@ -1060,6 +1080,11 @@ void CGameHandler::handleConnection(std::set<PlayerColor> players, CConnection &
 			//prepare struct informing that action was applied
 			auto sendPackageResponse = [&](bool succesfullyApplied)
 			{
+				//dont reply to disconnected client
+				//TODO: this must be implemented as option of CPackForServer
+				if(dynamic_cast<LeaveGame *>(pack) || dynamic_cast<CloseServer *>(pack))
+					return;
+
 				PackageApplied applied;
 				applied.player = player;
 				applied.result = succesfullyApplied;
@@ -1095,9 +1120,11 @@ void CGameHandler::handleConnection(std::set<PlayerColor> players, CConnection &
 	}
 	catch(boost::system::system_error &e) //for boost errors just log, not crash - probably client shut down connection
 	{
-		assert(!c.connected); //make sure that connection has been marked as broken
-		logGlobal->error(e.what());
-		end2 = true;
+		handleDisconnection(e);
+	}
+	catch(clientDisconnectedException & e)
+	{
+		handleDisconnection(e);
 	}
 	catch(...)
 	{
@@ -2631,6 +2658,9 @@ void CGameHandler::sendToAllClients(CPackForClient * info)
 	logNetwork->trace("Sending to all clients a package of type %s", typeid(*info).name());
 	for (auto & elem : conns)
 	{
+		if(!elem->isOpen())
+			continue;
+
 		boost::unique_lock<boost::mutex> lock(*(elem)->wmx);
 		*elem << info;
 	}
@@ -2703,11 +2733,32 @@ void CGameHandler::close()
 	{
 		exit(0);
 	}
+	end2 = true;
 
-	//for (CConnection *cc : conns)
-	//	if (cc && cc->socket && cc->socket->is_open())
-	//		cc->socket->close();
-	//exit(0);
+	for (auto & elem : conns)
+	{
+		if(!elem->isOpen())
+			continue;
+
+		boost::unique_lock<boost::mutex> lock(*(elem)->wmx);
+		elem->close();
+		elem->connected = false;
+	}
+	exit(0);
+}
+
+void CGameHandler::playerLeftGame(int cid)
+{
+	for (auto & elem : conns)
+	{
+		if(elem->isOpen() && elem->connectionID == cid)
+		{
+			boost::unique_lock<boost::mutex> lock(*(elem)->wmx);
+			elem->close();
+			elem->connected = false;
+			break;
+		}
+	}
 }
 
 bool CGameHandler::arrangeStacks(ObjectInstanceID id1, ObjectInstanceID id2, ui8 what, SlotID p1, SlotID p2, si32 val, PlayerColor player)
@@ -6162,12 +6213,18 @@ void CGameHandler::handleCheatCode(std::string & cheat, PlayerColor player, cons
 	else if (cheat == "vcmisilmaril")
 	{
 		///Player wins
-		gs->getPlayer(player)->enteredWinningCheatCode = 1;
+		PlayerCheated pc;
+		pc.player = player;
+		pc.winningCheatCode = true;
+		sendAndApply(&pc);
 	}
 	else if (cheat == "vcmimelkor")
 	{
 		///Player looses
-		gs->getPlayer(player)->enteredLosingCheatCode = 1;
+		PlayerCheated pc;
+		pc.player = player;
+		pc.losingCheatCode = true;
+		sendAndApply(&pc);
 	}
 	else if (cheat == "vcmieagles" || cheat == "vcmiungoliant")
 	{

server/CGameHandler.h

@@ -223,6 +223,7 @@ public:
 	bool arrangeStacks( ObjectInstanceID id1, ObjectInstanceID id2, ui8 what, SlotID p1, SlotID p2, si32 val, PlayerColor player);
 	void save(const std::string &fname);
 	void close();
+	void playerLeftGame(int cid);
 	void handleTimeEvents();
 	void handleTownEvents(CGTownInstance *town, NewTurn &n);
 	bool complain(const std::string &problem); //sends message to all clients, prints on the logs and return true
@@ -298,4 +299,9 @@ private:
 	void checkVictoryLossConditionsForAll();
 };
 
+class clientDisconnectedException : public std::exception
+{
+
+};
+
 void makeStackDoNothing();

server/CVCMIServer.cpp

@@ -45,7 +45,6 @@ std::string NAME = GameConstants::VCMI_VERSION + std::string(" (") + NAME_AFFIX
 namespace intpr = boost::interprocess;
 #endif
 bool end2 = false;
-int port = 3030;
 
 boost::program_options::variables_map cmdLineOptions;
 
@@ -106,6 +105,10 @@ void CPregameServer::handleConnection(CConnection *cpc)
 				auto unlock = vstd::makeUnlockGuard(mx);
 				while(state == RUNNING) boost::this_thread::sleep(boost::posix_time::milliseconds(50));
 			}
+			else if(quitting) // Server must be stopped if host is leaving from lobby to avoid crash
+			{
+				end2 = true;
+			}
 		}
 	}
 	catch (const std::exception& e)
@@ -206,8 +209,11 @@ void CPregameServer::connectionAccepted(const boost::system::error_code& ec)
 		return;
 	}
 
+	try
+	{
 		logNetwork->info("We got a new connection! :)");
-	CConnection *pc = new CConnection(upcomingConnection, NAME);
+		std::string name = NAME;
+		CConnection *pc = new CConnection(upcomingConnection, name.append(" STATE_PREGAME"));
 		initConnection(pc);
 		upcomingConnection = nullptr;
 
@@ -220,6 +226,12 @@ void CPregameServer::connectionAccepted(const boost::system::error_code& ec)
 		pj->playerName = pc->name;
 		pj->connectionID = pc->connectionID;
 		toAnnounce.push_back(pj);
+	}
+	catch(std::exception& e)
+	{
+		upcomingConnection = nullptr;
+		logNetwork->info("I guess it was just my imagination!");
+	}
 
 	start_async_accept();
 }
@@ -314,9 +326,28 @@ void CPregameServer::startListeningThread(CConnection * pc)
 }
 
 CVCMIServer::CVCMIServer()
-: io(new boost::asio::io_service()), acceptor(new TAcceptor(*io, boost::asio::ip::tcp::endpoint(boost::asio::ip::tcp::v4(), port))), firstConnection(nullptr)
+	: port(3030), io(new boost::asio::io_service()), firstConnection(nullptr)
 {
 	logNetwork->trace("CVCMIServer created!");
+	if(cmdLineOptions.count("port"))
+		port = cmdLineOptions["port"].as<ui16>();
+	logNetwork->info("Port %d will be used", port);
+	try
+	{
+		acceptor = new TAcceptor(*io, boost::asio::ip::tcp::endpoint(boost::asio::ip::tcp::v4(), port));
+	}
+	catch(...)
+	{
+		logNetwork->info("Port %d is busy, trying to use random port instead", port);
+		if(cmdLineOptions.count("run-by-client") && !cmdLineOptions.count("use-shm"))
+		{
+			logNetwork->error("Cant pass port number to client without shared memory!", port);
+			exit(0);
+		}
+		acceptor = new TAcceptor(*io, boost::asio::ip::tcp::endpoint(boost::asio::ip::tcp::v4(), 0));
+		port = acceptor->local_endpoint().port();
+	}
+	logNetwork->info("Listening for connections at port %d", port);
 }
 CVCMIServer::~CVCMIServer()
 {
@@ -396,6 +427,8 @@ void CVCMIServer::start()
 #ifndef VCMI_ANDROID
 	ServerReady *sr = nullptr;
 	intpr::mapped_region *mr;
+	if(cmdLineOptions.count("use-shm"))
+	{
 		try
 		{
 			intpr::shared_memory_object smo(intpr::open_only,"vcmi_memory",intpr::read_write);
@@ -410,33 +443,41 @@ void CVCMIServer::start()
 			mr = new intpr::mapped_region(smo,intpr::read_write);
 			sr = new(mr->get_address())ServerReady();
 		}
+	}
 #endif
 
 	boost::system::error_code error;
-	logNetwork->info("Listening for connections at port %d", acceptor->local_endpoint().port());
+	for (;;)
+	{
+		try
+		{
 			auto s = new boost::asio::ip::tcp::socket(acceptor->get_io_service());
 			boost::thread acc(std::bind(vaccept,acceptor,s,&error));
-#ifndef VCMI_ANDROID
-	sr->setToTrueAndNotify();
-	delete mr;
-#else
+#ifdef VCMI_ANDROID
 			{ // in block to clean-up vm helper after use, because we don't need to keep this thread attached to vm
 				CAndroidVMHelper envHelper;
 				envHelper.callStaticVoidMethod(CAndroidVMHelper::NATIVE_METHODS_DEFAULT_CLASS, "onServerReady");
 				logNetwork->info("Sending server ready message to client");
 			}
+#else
+			if(cmdLineOptions.count("use-shm"))
+			{
+				sr->setToTrueAndNotify(port);
+				delete mr;
+			}
 #endif
 
 			acc.join();
 			if (error)
 			{
-		logNetwork->warnStream() << "Got connection but there is an error " << error;
+				logNetwork->warnStream()<<"Got connection but there is an error " << error;
 				return;
 			}
 			logNetwork->info("We've accepted someone... ");
-	firstConnection = new CConnection(s, NAME);
+			std::string name = NAME;
+			firstConnection = new CConnection(s, name.append(" STATE_WAITING"));
 			logNetwork->info("Got connection!");
-	while (!end2)
+			while(!end2)
 			{
 				ui8 mode;
 				*firstConnection >> mode;
@@ -459,6 +500,14 @@ void CVCMIServer::start()
 					break;
 				}
 			}
+			break;
+		}
+		catch(std::exception& e)
+		{
+			vstd::clear_pointer(firstConnection);
+			logNetwork->info("I guess it was just my imagination!");
+		}
+	}
 }
 
 void CVCMIServer::loadGame()
@@ -507,7 +556,9 @@ static void handleCommandOptions(int argc, char *argv[])
 	opts.add_options()
 		("help,h", "display help and exit")
 		("version,v", "display version information and exit")
-		("port", po::value<int>()->default_value(3030), "port at which server will listen to connections from client")
+		("run-by-client", "indicate that server launched by client on same machine")
+		("use-shm", "enable usage of shared memory")
+		("port", po::value<ui16>(), "port at which server will listen to connections from client")
 		("resultsFile", po::value<std::string>()->default_value("./results.txt"), "file to which the battle result will be appended. Used only in the DUEL mode.");
 
 	if(argc > 1)
@@ -584,12 +635,7 @@ int main(int argc, char** argv)
 	logConfig.configureDefault();
 	logGlobal->info(NAME);
 
-
 	handleCommandOptions(argc, argv);
-	if (cmdLineOptions.count("port"))
-		port = cmdLineOptions["port"].as<int>();
-	logNetwork->info("Port %d will be used.", port);
-
 	preinitDLL(console);
 	settings.init();
 	logConfig.configure();
@@ -630,10 +676,8 @@ int main(int argc, char** argv)
 	CAndroidVMHelper envHelper;
 	envHelper.callStaticVoidMethod(CAndroidVMHelper::NATIVE_METHODS_DEFAULT_CLASS, "killServer");
 #endif
-	delete VLC;
-	VLC = nullptr;
+	vstd::clear_pointer(VLC);
 	CResourceHandler::clear();
-
 	return 0;
 }
 

server/CVCMIServer.h

@@ -43,6 +43,7 @@ typedef boost::asio::basic_stream_socket < boost::asio::ip::tcp , boost::asio::s
 
 class CVCMIServer
 {
+	ui16 port;
 	boost::asio::io_service *io;
 	TAcceptor * acceptor;
 

server/NetPacksServer.cpp

@@ -65,6 +65,12 @@ bool CloseServer::applyGh( CGameHandler *gh )
 	return true;
 }
 
+bool LeaveGame::applyGh( CGameHandler *gh )
+{
+	gh->playerLeftGame(c->connectionID);
+	return true;
+}
+
 bool EndTurn::applyGh( CGameHandler *gh )
 {
 	PlayerColor player = GS(gh)->currentPlayer;