widelands-dev team mailing list archive
-
widelands-dev team
-
Mailing list archive
-
Message #16765
Re: [Merge] lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands
Diff comments:
>
> === modified file 'src/network/gameclient.cc'
> --- src/network/gameclient.cc 2019-04-29 16:22:08 +0000
> +++ src/network/gameclient.cc 2019-05-01 07:38:30 +0000
> @@ -91,8 +91,115 @@
>
> /// Backlog of chat messages
> std::vector<ChatMessage> chatmessages;
> +
> +
> + void send_hello();
> + void send_player_command(Widelands::PlayerCommand*);
> +
> + bool run_map_menu(GameClient* This, bool internet);
> + void run_game(InteractiveGameBase* igb, UI::ProgressWindow*, bool internet);
> +
> + InteractiveGameBase* init_game(GameClient* This, UI::ProgressWindow*);
> +
> };
>
> +void GameClientImpl::send_hello() {
> + SendPacket s;
> + s.unsigned_8(NETCMD_HELLO);
> + s.unsigned_8(NETWORK_PROTOCOL_VERSION);
> + s.string(localplayername);
> + s.string(build_id());
> + net->send(s);
> +}
> +
> +void GameClientImpl::send_player_command(Widelands::PlayerCommand* pc) {
> + SendPacket s;
> + s.unsigned_8(NETCMD_PLAYERCOMMAND);
> + s.signed_32(game->get_gametime());
> + pc->serialize(s);
> + net->send(s);
> +}
> +
> +/**
> + * Show and run() the fullscreen menu for setting map and mapsettings.
> + *
> + * @return true to indicate that run is done.
> + */
> +bool GameClientImpl::run_map_menu(GameClient* This, bool internet) {
> + FullscreenMenuLaunchMPG lgm(This, This);
> + lgm.set_chat_provider(*This);
> + modal = &lgm;
> + FullscreenMenuBase::MenuTarget code = lgm.run<FullscreenMenuBase::MenuTarget>();
> + modal = nullptr;
> + if (code == FullscreenMenuBase::MenuTarget::kBack) {
> + // if this is an internet game, tell the metaserver that client is back in the lobby.
> + if (internet) {
> + InternetGaming::ref().set_game_done();
> + }
> + return true;
> + }
> + return false;
> +}
> +
> +/**
> + * Show Pogressdialog and load map or saved game.
Pogressdialog -> progress dialog
> + */
> +InteractiveGameBase* GameClientImpl::init_game(GameClient* This, UI::ProgressWindow* loader) {
> + modal = loader;
> + std::vector<std::string> tipstext;
> + tipstext.push_back("general_game");
> + tipstext.push_back("multiplayer");
> + try {
> + tipstext.push_back(This->get_players_tribe());
Remove the This-> bit.
Get the tribe and check whether it exists before you push it, then you won't have to do programming by exception.
> + } catch (GameClient::NoTribe) {
> + }
> + GameTips tips(*loader, tipstext);
> +
> + loader->step(_("Preparing game"));
> +
> + game->set_game_controller(This);
This -> this
> + uint8_t const pn = settings.playernum + 1;
> + game->save_handler().set_autosave_filename(
> + (boost::format("%s_netclient%u") % kAutosavePrefix % static_cast<unsigned int>(pn)).str());
> + InteractiveGameBase* igb;
> + if (pn > 0)
add {}
> + igb = new InteractivePlayer(*game, g_options.pull_section("global"), pn, true);
> + else
> + igb = new InteractiveSpectator(*game, g_options.pull_section("global"), true);
> + game -> set_ibase(igb);
> + igb->set_chat_provider(*This);
> + if (settings.savegame) { // new map
> + game->init_newgame(loader, settings);
> + } else { // savegame
> + game->init_savegame(loader, settings);
> + }
> + return igb;
> +}
> +
> +
> +/**
> + * Run the actual game and cleanup when done.
> + */
> +void GameClientImpl::run_game(InteractiveGameBase* igb, UI::ProgressWindow* loader, bool internet) {
> + time.reset(game->get_gametime());
> + lasttimestamp = game->get_gametime();
> + lasttimestamp_realtime = SDL_GetTicks();
> +
> + modal = igb;
> + game->run(
> + loader,
> + settings.savegame ?
> + Widelands::Game::Loaded :
> + settings.scenario ? Widelands::Game::NewMPScenario : Widelands::Game::NewNonScenario,
> + "", false, (boost::format("netclient_%d") % static_cast<int>(settings.usernum)).str());
> +
> + // if this is an internet game, tell the metaserver that the game is done.
> + if (internet)
Add {}
> + InternetGaming::ref().set_game_done();
> + modal = nullptr;
> + game = nullptr;
> +}
> +
> GameClient::GameClient(const std::pair<NetAddress, NetAddress>& host,
> const std::string& playername,
> bool internet,
> @@ -141,85 +248,32 @@
> delete d;
> }
>
> +
> +
> void GameClient::run() {
> - SendPacket s;
> - s.unsigned_8(NETCMD_HELLO);
> - s.unsigned_8(NETWORK_PROTOCOL_VERSION);
> - s.string(d->localplayername);
> - s.string(build_id());
> - d->net->send(s);
>
> + d->send_hello();
> d->settings.multiplayer = true;
>
> // Fill the list of possible system messages
> NetworkGamingMessages::fill_map();
> - {
> - FullscreenMenuLaunchMPG lgm(this, this);
> - lgm.set_chat_provider(*this);
> - d->modal = &lgm;
> - FullscreenMenuBase::MenuTarget code = lgm.run<FullscreenMenuBase::MenuTarget>();
> - d->modal = nullptr;
> - if (code == FullscreenMenuBase::MenuTarget::kBack) {
> - // if this is an internet game, tell the metaserver that client is back in the lobby.
> - if (internet_)
> - InternetGaming::ref().set_game_done();
> - return;
> - }
> - }
> +
> + if (d->run_map_menu(this, internet_))
Add {}
> + return; // did not select a Map ...
>
> d->server_is_waiting = true;
>
> + bool writeSyncStreams = g_options.pull_section("global").get_bool("write_syncstreams", true);
We use underscores in variable names rather then CamelCase: write_sync_sctrams
> Widelands::Game game;
> - game.set_write_syncstream(g_options.pull_section("global").get_bool("write_syncstreams", true));
> + game.set_write_syncstream(writeSyncStreams);
>
> try {
> std::unique_ptr<UI::ProgressWindow> loader_ui(new UI::ProgressWindow());
> - d->modal = loader_ui.get();
> - std::vector<std::string> tipstext;
> - tipstext.push_back("general_game");
> - tipstext.push_back("multiplayer");
> - try {
> - tipstext.push_back(get_players_tribe());
> - } catch (NoTribe) {
> - }
> - GameTips tips(*loader_ui.get(), tipstext);
> -
> - loader_ui->step(_("Preparing game"));
>
> d->game = &game;
> - game.set_game_controller(this);
> - uint8_t const pn = d->settings.playernum + 1;
> - game.save_handler().set_autosave_filename(
> - (boost::format("%s_netclient%u") % kAutosavePrefix % static_cast<unsigned int>(pn)).str());
> - InteractiveGameBase* igb;
> - if (pn > 0)
> - igb = new InteractivePlayer(game, g_options.pull_section("global"), pn, true);
> - else
> - igb = new InteractiveSpectator(game, g_options.pull_section("global"), true);
> - game.set_ibase(igb);
> - igb->set_chat_provider(*this);
> - if (!d->settings.savegame) { // new map
> - game.init_newgame(loader_ui.get(), d->settings);
> - } else { // savegame
> - game.init_savegame(loader_ui.get(), d->settings);
> - }
> - d->time.reset(game.get_gametime());
> - d->lasttimestamp = game.get_gametime();
> - d->lasttimestamp_realtime = SDL_GetTicks();
> -
> - d->modal = igb;
> - game.run(
> - loader_ui.get(),
> - d->settings.savegame ?
> - Widelands::Game::Loaded :
> - d->settings.scenario ? Widelands::Game::NewMPScenario : Widelands::Game::NewNonScenario,
> - "", false, (boost::format("netclient_%d") % static_cast<int>(d->settings.usernum)).str());
> -
> - // if this is an internet game, tell the metaserver that the game is done.
> - if (internet_)
> - InternetGaming::ref().set_game_done();
> - d->modal = nullptr;
> - d->game = nullptr;
> + InteractiveGameBase* igb = d->init_game(this, loader_ui.get());
> + d->run_game(igb, loader_ui.get(), internet_);
> +
> } catch (...) {
> WLApplication::emergency_save(game);
> d->game = nullptr;
> @@ -254,25 +309,28 @@
> }
> }
>
> -void GameClient::send_player_command(Widelands::PlayerCommand& pc) {
> +/**
> + * Send PlayerCommand to server.
> + *
> + * @param pc will always be deleted in the end.
> + */
> +void GameClient::send_player_command(Widelands::PlayerCommand* pc) {
> assert(d->game);
> - if (pc.sender() != d->settings.playernum + 1) {
> - delete &pc;
> - return;
> +
> + // TODDO(Klaus Halfmann)should this be an assert?
asserts will only be checked in debug builds, so we still need this check for security. You could add an assert though to guard against programming mistakes.
> + if (pc->sender() == d->settings.playernum + 1) // allow command for current player only
> + {
> + log("[Client]: send playercommand at time %i\n", d->game->get_gametime());
> +
> + d->send_player_command(pc);
> +
> + d->lasttimestamp = d->game->get_gametime();
> + d->lasttimestamp_realtime = SDL_GetTicks();
> + } else {
> + log("[Client]: Playercommand is not for curret player? %i\n", pc -> sender());
curret -> current
> }
>
> - log("[Client]: send playercommand at time %i\n", d->game->get_gametime());
> -
> - SendPacket s;
> - s.unsigned_8(NETCMD_PLAYERCOMMAND);
> - s.signed_32(d->game->get_gametime());
> - pc.serialize(s);
> - d->net->send(s);
> -
> - d->lasttimestamp = d->game->get_gametime();
> - d->lasttimestamp_realtime = SDL_GetTicks();
> -
> - delete &pc;
> + delete pc;
Make pc a unique_ptr and it will delete itself when it's no longer used.
> }
>
> int32_t GameClient::get_frametime() {
> @@ -544,6 +602,331 @@
> }
> }
>
> +void GameClient::handle_disconnect(RecvPacket& packet) {
> + uint8_t number = packet.unsigned_8();
> + std::string reason = packet.string();
> + if (number == 1)
Add {}
> + disconnect(reason, "", false);
> + else {
> + std::string arg = packet.string();
> + disconnect(reason, arg, false);
> + }
> +}
> +
> +/**
> + * Hello from the other side
> + */
> +void GameClient::handle_hello(RecvPacket& packet) {
> + if (d->settings.usernum == -2) // TODDO(Klaus Halfmann): what magic number is this?
I don't know. I do not like the raw number either, so we should search for this and give it a name.
> + throw ProtocolException(NETCMD_HELLO);
> + uint8_t const version = packet.unsigned_8();
> + if (version != NETWORK_PROTOCOL_VERSION)
Add {}
> + throw DisconnectException("DIFFERENT_PROTOCOL_VERS");
> + d->settings.usernum = packet.unsigned_32();
> + d->settings.playernum = -1;
> +}
> +
> +/**
> + * Give a pong for a ping
> + */
> +void GameClient::handle_ping(RecvPacket&) {
> + SendPacket s;
> + s.unsigned_8(NETCMD_PONG);
> + d->net->send(s);
> +
> + log("[Client] Pong!\n");
> +}
> +
> +/**
> + * New Map name was send.
send -> sent
> + */
> +void GameClient::handle_setting_map(RecvPacket& packet) {
> + d->settings.mapname = packet.string();
> + d->settings.mapfilename = g_fs->FileSystem::fix_cross_file(packet.string());
> + d->settings.savegame = packet.unsigned_8() == 1;
> + d->settings.scenario = packet.unsigned_8() == 1;
> + log("[Client] SETTING_MAP '%s' '%s'\n", d->settings.mapname.c_str(),
> + d->settings.mapfilename.c_str());
> +
> + // New map was set, so we clean up the buffer of a previously requested file
> + file_.reset(nullptr);
> +}
> +
> +/**
> + *
> + */
> +void GameClient::handle_new_file(RecvPacket& packet) {
> + std::string path = g_fs->FileSystem::fix_cross_file(packet.string());
> + uint32_t bytes = packet.unsigned_32();
> + std::string md5 = packet.string();
> +
> + // Check whether the file or a file with that name already exists
> + if (g_fs->file_exists(path)) {
> + // If the file is a directory, we have to rename the file and replace it with the version
> + // of thehost. If it is a ziped file, we can check, whether the host and the client have
thehost -> the host
ziped -> zipped
> + // got the same file.
> + if (!g_fs->is_directory(path)) {
> + FileRead fr;
> + fr.open(*g_fs, path);
> + if (bytes == fr.get_size()) {
> + std::unique_ptr<char[]> complete(new char[bytes]);
> + if (!complete)
Add {} - below too
> + throw wexception("Out of memory");
> +
> + fr.data_complete(complete.get(), bytes);
> + // TODO(Klaus Halfmann): compute MD5 on the fly in FileRead...
> + SimpleMD5Checksum md5sum;
> + md5sum.data(complete.get(), bytes);
> + md5sum.finish_checksum();
> + std::string localmd5 = md5sum.get_checksum().str();
> + if (localmd5 == md5)
> + // everything is alright we now have the file.
> + return;
> + }
> + }
> + // Don't overwrite the file, better rename the original one
> + try {
> + g_fs->fs_rename(path, backup_file_name(path));
> + } catch (const FileError& e) {
> + log("file error in GameClient::handle_packet: case NETCMD_FILE_PART: "
> + "%s\n",
> + e.what());
> + // TODO(Arty): What now? It just means the next step will fail
> + // or possibly result in some corrupt file
> + }
> + }
> +
> + // Yes we need the file!
> + SendPacket s;
> + s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
> + d->net->send(s);
> +
> + file_.reset(new NetTransferFile());
> + file_->bytes = bytes;
> + file_->filename = path;
> + file_->md5sum = md5;
> + size_t position = path.rfind(g_fs->file_separator(), path.size() - 2);
> + if (position != std::string::npos) {
> + path.resize(position);
> + g_fs->ensure_directory_exists(path);
> + }
> +}
> +
> +/**
> + *
> + */
> +void GameClient::handle_file_part(RecvPacket& packet) {
> + // Only go on, if we are waiting for a file part at the moment. It can happen, that an
> + // "unrequested" part is send by the server if the map was changed just a moment ago
> + // and there was an outstanding request from the client.
> + if (!file_)
> + return; // silently ignore
> +
> + uint32_t part = packet.unsigned_32();
> + uint32_t size = packet.unsigned_32();
> +
> + // Send an answer
> + SendPacket s;
> + s.unsigned_8(NETCMD_FILE_PART);
> + s.unsigned_32(part);
> + s.string(file_->md5sum);
> + d->net->send(s);
> +
> + FilePart fp;
> +
> + char buf[NETFILEPARTSIZE];
> + assert(size <= NETFILEPARTSIZE);
> +
> + // TODO(Klaus Halfmann): read directcly into FilePart?
> + if (packet.data(buf, size) != size)
> + log("Readproblem. Will try to go on anyways\n");
> + memcpy(fp.part, &buf[0], size);
> + file_->parts.push_back(fp);
> +
> + // Write file to disk as soon as all parts arrived
> + uint32_t left = (file_->bytes - NETFILEPARTSIZE * part);
> + if (left <= NETFILEPARTSIZE) {
> + FileWrite fw;
> + left = file_->bytes;
> + uint32_t i = 0;
> + // Put all data together
> + while (left > 0) {
> + uint32_t writeout = (left > NETFILEPARTSIZE) ? NETFILEPARTSIZE : left;
> + fw.data(file_->parts[i].part, writeout, FileWrite::Pos::null());
> + left -= writeout;
> + ++i;
> + }
> + // Now really write the file
> + fw.write(*g_fs, file_->filename.c_str());
> +
> + // Check for consistence
> + FileRead fr;
> + fr.open(*g_fs, file_->filename);
> +
> + std::unique_ptr<char[]> complete(new char[file_->bytes]);
> +
> + fr.data_complete(complete.get(), file_->bytes);
> + SimpleMD5Checksum md5sum;
> + md5sum.data(complete.get(), file_->bytes);
> + md5sum.finish_checksum();
> + std::string localmd5 = md5sum.get_checksum().str();
> + if (localmd5 != file_->md5sum) {
> + // Something went wrong! We have to rerequest the file.
> + s.reset();
> + s.unsigned_8(NETCMD_NEW_FILE_AVAILABLE);
> + d->net->send(s);
> + // Notify the players
> + s.reset();
> + s.unsigned_8(NETCMD_CHAT);
> + s.string(_("/me 's file failed md5 checksumming."));
> + d->net->send(s);
> + try {
> + g_fs->fs_unlink(file_->filename);
> + } catch (const FileError& e) {
> + log("file error in GameClient::handle_packet: case NETCMD_FILE_PART: "
> + "%s\n",
> + e.what());
> + }
> + }
> + // Check file for validity
> + bool invalid = false;
> + if (d->settings.savegame) {
> + // Saved game check - does Widelands recognize the file as saved game?
> + Widelands::Game game;
> + try {
> + Widelands::GameLoader gl(file_->filename, game);
> + } catch (...) {
> + invalid = true;
> + }
> + } else {
> + // Map check - does Widelands recognize the file as map?
> + Widelands::Map map;
> + std::unique_ptr<Widelands::MapLoader> ml = map.get_correct_loader(file_->filename);
> + if (!ml)
> + invalid = true;
> + }
> + if (invalid) {
> + try {
> + g_fs->fs_unlink(file_->filename);
> + // Restore original file, if there was one before
> + if (g_fs->file_exists(backup_file_name(file_->filename)))
> + g_fs->fs_rename(backup_file_name(file_->filename), file_->filename);
> + } catch (const FileError& e) {
> + log("file error in GameClient::handle_packet: case NETCMD_FILE_PART: "
> + "%s\n",
> + e.what());
> + }
> + s.reset();
> + s.unsigned_8(NETCMD_CHAT);
> + s.string(_("/me checked the received file. Although md5 check summing succeeded, "
> + "I can not handle the file."));
> + d->net->send(s);
> + }
> + }
> +}
> +
> +/**
> + *
> + */
> +void GameClient::handle_setting_tribes(RecvPacket& packet) {
> + d->settings.tribes.clear();
> + for (uint8_t i = packet.unsigned_8(); i; --i) {
> + Widelands::TribeBasicInfo info = Widelands::get_tribeinfo(packet.string());
> +
> + // Get initializations (we have to do this locally, for translations)
> + LuaInterface lua;
> + info.initializations.clear();
> + for (uint8_t j = packet.unsigned_8(); j > 0; --j) {
> + std::string const initialization_script = packet.string();
> + std::unique_ptr<LuaTable> t = lua.run_script(initialization_script);
> + t->do_not_warn_about_unaccessed_keys();
> + info.initializations.push_back(Widelands::TribeBasicInfo::Initialization(
> + initialization_script, t->get_string("descname"), t->get_string("tooltip")));
> + }
> + d->settings.tribes.push_back(info);
> + }
> +}
> +
> +/**
> + *
> + */
> +void GameClient::handle_setting_allplayers(RecvPacket& packet) {
> + d->settings.players.resize(packet.unsigned_8());
> + for (uint8_t i = 0; i < d->settings.players.size(); ++i) {
> + receive_one_player(i, packet);
> + }
> + // Map changes are finished here
> + Notifications::publish(NoteGameSettings(NoteGameSettings::Action::kMap));
> +}
> +
> +/**
> + *
> + */
> +void GameClient::handle_playercommand(RecvPacket& packet) {
> + if (!d->game)
> + throw DisconnectException("PLAYERCMD_WO_GAME");
> +
> + int32_t const time = packet.signed_32();
> + Widelands::PlayerCommand& plcmd = *Widelands::PlayerCommand::deserialize(packet);
> + plcmd.set_duetime(time);
> + d->game->enqueue_command(&plcmd);
> + d->time.receive(time);
> +}
> +
> +/**
> + *
> + */
> +void GameClient::handle_syncrequest(RecvPacket& packet) {
> + if (!d->game)
> + throw DisconnectException("SYNCREQUEST_WO_GAME");
> + int32_t const time = packet.signed_32();
> + d->time.receive(time);
> + d->game->enqueue_command(new CmdNetCheckSync(time, [this] { sync_report_callback(); }));
> + d->game->report_sync_request();
> +}
> +
> +/**
> + *
> + */
> +void GameClient::handle_chat(RecvPacket& packet) {
> + ChatMessage c("");
> + c.playern = packet.signed_16();
> + c.sender = packet.string();
> + c.msg = packet.string();
> + if (packet.unsigned_8())
> + c.recipient = packet.string();
> + d->chatmessages.push_back(c);
> + Notifications::publish(c);
> +}
> +
> +/**
> + *
> + */
> +void GameClient::handle_system_message(RecvPacket& packet) {
> + const std::string code = packet.string();
> + const std::string arg1 = packet.string();
> + const std::string arg2 = packet.string();
> + const std::string arg3 = packet.string();
> + ChatMessage c(NetworkGamingMessages::get_message(code, arg1, arg2, arg3));
> + c.playern = UserSettings::none(); // == System message
> + // c.sender remains empty to indicate a system message
> + d->chatmessages.push_back(c);
> + Notifications::publish(c);
> +}
> +
> +/**
> + *
> + */
> +void GameClient::handle_desync(RecvPacket&) {
> + log("[Client] received NETCMD_INFO_DESYNC. Trying to salvage some "
> + "information for debugging.\n");
> + if (d->game) {
> + d->game->save_syncstream(true);
> + // We don't know our playernumber, so report as -1
> + d->game->report_desync(-1);
> + }
> +}
> +
> /**
> * Handle one packet received from the host.
> *
>
> === modified file 'src/network/gameclient.h'
> --- src/network/gameclient.h 2019-04-29 16:22:08 +0000
> +++ src/network/gameclient.h 2019-05-01 07:38:30 +0000
> @@ -125,8 +141,11 @@
> bool sendreason = true,
> bool showmsg = true);
>
> +
> + GameClientImpl* d;
Turn this into a unique_ptr
> +
> + /** File that is eventually transferred via the netowrk if not found at the other side */
netowrk -> network
> std::unique_ptr<NetTransferFile> file_;
> - GameClientImpl* d;
> bool internet_;
> };
>
--
https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743
Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands.
References
