Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Fixup ready block #148

Closed
wants to merge 5 commits into from

1 participant

@cbeck88
Collaborator

*** Experimental ***

gfgtdf reports a bug, where when a reloaded game is hosted, if a reserved side is replaced manually with a network player, the ready button blocks "waiting for faction..." even though no one is choosing a faction.

This is relevant to my earlier commit: 186e66c

I believe that I tested this and it worked at an earlier version, nevertheless after reviewing the code I decided that I should try to make exactly the same test on the host as is used by the client when deciding whether to show the faction selection dialog, instead of what I was using which now seems to me as a hack. Specifically that means I should test whether "allow_changes = true".

Here's where the plot thickens. That doesn't fix the issue, and indeed debugging output inserted in the subsequent commits here shows that the host and clients don't actually agree about the value of "allow_changes" for the joining player! I have also made server-side debugging output to show exactly what the game config being passed to the clients is. It looks that it might be an issue that one side is perhaps reading from "snapshot" and the other from replay_start or some such thing? Anyways if anyone has an idea why they might be out of sync regarding "allow_changes" it would be helpful, I'm going to put this aside for a few days and perhaps come back to it.

Also: possibly relevant to fa5b916

cbeck88 added some commits
@cbeck88 cbeck88 fixing block game until all sides ready ada42f5
@cbeck88 cbeck88 add debugging output to server and waiting client
server shows level when the game starts, on debug channel
mp client which is waiting shows allow_changes value
38a2302
@cbeck88 cbeck88 server debugging output: show level cfg when adding player aaf98f9
@cbeck88 cbeck88 add debugging output to mp clients when add new player 43603e0
@cbeck88 cbeck88 fix bug #21916, ready blocked at inappropriate times
the problem was that params.saved_game was checked at the wrong time,
it was checked at the time of making a new config, but not in the
constructor. the result is that the host and clients are out of sync
about whether allow_changes is true for a given side.
2c4e797
@cbeck88
Collaborator

I actually rebased this and merged it later.

@cbeck88 cbeck88 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Apr 17, 2014
  1. @cbeck88
  2. @cbeck88

    add debugging output to server and waiting client

    cbeck88 authored
    server shows level when the game starts, on debug channel
    mp client which is waiting shows allow_changes value
  3. @cbeck88
  4. @cbeck88
  5. @cbeck88

    fix bug #21916, ready blocked at inappropriate times

    cbeck88 authored
    the problem was that params.saved_game was checked at the wrong time,
    it was checked at the time of making a new config, but not in the
    constructor. the result is that the host and clients are out of sync
    about whether allow_changes is true for a given side.
This page is out of date. Refresh to see the latest.
View
15 src/multiplayer_connect_engine.cpp
@@ -31,8 +31,8 @@ static lg::log_domain log_config("config");
#define ERR_CF LOG_STREAM(err, log_config)
static lg::log_domain log_mp_connect_engine("mp/connect/engine");
-#define DBG_MP LOG_STREAM(debug, log_mp_connect_engine)
#define LOG_MP LOG_STREAM(info, log_mp_connect_engine)
+#define DBG_MP LOG_MP
static lg::log_domain log_network("network");
#define LOG_NW LOG_STREAM(info, log_network)
@@ -692,12 +692,11 @@ std::pair<bool, bool> connect_engine::process_network_data(const config& data,
LOG_CF << "client has taken a valid position\n";
- bool was_reserved = (side_engines_[side_taken]->controller() == CNTR_RESERVED);
import_user(data, false, side_taken);
- side_engines_[side_taken]->set_waiting_to_choose_status(!was_reserved);
-
update_and_send_diff();
+ side_engines_[side_taken]->set_waiting_to_choose_status(side_engines_[side_taken]->allow_changes()); //wait for them to choose faction if allowed
+ LOG_MP << "waiting to choose status = " << side_engines_[side_taken]->allow_changes() << std::endl;
result.second = false;
LOG_NW << "sent player data\n";
@@ -840,7 +839,7 @@ side_engine::side_engine(const config& cfg, connect_engine& parent_engine,
current_controller_index_(0),
controller_options_(),
allow_player_(cfg["allow_player"].to_bool(true)),
- allow_changes_(cfg["allow_changes"].to_bool(true)),
+ allow_changes_(!parent_.params_.saved_game && cfg["allow_changes"].to_bool(true)),
controller_lock_(cfg["controller_lock"].to_bool(
parent_.force_lock_settings_)),
index_(index),
@@ -851,7 +850,7 @@ side_engine::side_engine(const config& cfg, connect_engine& parent_engine,
current_player_(cfg["current_player"]),
player_id_(cfg["player_id"]),
ai_algorithm_(),
- waiting_to_choose_faction_(allow_changes_ && allow_player_),
+ waiting_to_choose_faction_(allow_changes_),
chose_random_(cfg["chose_random"].to_bool(false)),
flg_(parent_.era_factions_, cfg_, parent_.force_lock_settings_,
parent_.params_.saved_game, color_)
@@ -862,6 +861,8 @@ side_engine::side_engine(const config& cfg, connect_engine& parent_engine,
current_player_ = lexical_cast<std::string>(side_cntr);
}
+ LOG_MP << "Made a side engine: side_cntr = " << side_cntr << " allow_changes = " << allow_changes_ << std::endl;
+
update_controller_options();
// Tweak the controllers.
@@ -1016,7 +1017,7 @@ config side_engine::new_config() const
}
res["name"] = res["user_description"];
- res["allow_changes"] = !parent_.params_.saved_game && allow_changes_;
+ res["allow_changes"] = allow_changes_;
res["chose_random"] = chose_random_;
if (!parent_.params_.saved_game) {
View
1  src/multiplayer_connect_engine.hpp
@@ -193,6 +193,7 @@ class side_engine
void set_ai_algorithm(const std::string& ai_algorithm)
{ ai_algorithm_ = ai_algorithm; }
bool allow_player() const { return allow_player_; }
+ bool allow_changes() const { return allow_changes_; }
bool waiting_to_choose_faction() const { return waiting_to_choose_faction_; }
void set_waiting_to_choose_status(bool status) { waiting_to_choose_faction_ = status;}
bool chose_random() const { return chose_random_;}
View
11 src/multiplayer_wait.cpp
@@ -38,6 +38,7 @@ static lg::log_domain log_network("network");
static lg::log_domain log_enginerefac("enginerefac");
#define LOG_RG LOG_STREAM(info, log_enginerefac)
+#define DBG_RG LOG_STREAM(debug, log_enginerefac)
namespace {
@@ -225,6 +226,10 @@ void wait::join_game(bool observe)
return;
}
+ DBG_RG << "*** wait::join_game: downloaded level data, here it is: " << std::endl;
+ DBG_RG << level_.debug() << std::endl;
+ DBG_RG << "***" << std::endl;
+
if (first_scenario_) {
state_ = game_state();
state_.classification().campaign_type = "multiplayer";
@@ -258,6 +263,10 @@ void wait::join_game(bool observe)
int side_num = -1, nb_sides = 0;
BOOST_FOREACH(const config &sd, level_.child_range("side"))
{
+ DBG_RG << "wait::join_game: considering a side: " << std::endl;
+ DBG_RG << "***" << std::endl;
+ DBG_RG << sd.debug() << std::endl;
+ DBG_RG << "***" << std::endl;
if (sd["controller"] == "reserved" && sd["current_player"] == preferences::login())
{
side_choice = &sd;
@@ -287,6 +296,8 @@ void wait::join_game(bool observe)
//if the client is allowed to choose their team, instead of having
//it set by the server, do that here.
+ LOG_RG << "multiplayer_wait: allow_changes = " << allow_changes << std::endl;
+
if(allow_changes) {
events::event_context context;
View
10 src/server/game.cpp
@@ -212,9 +212,10 @@ void game::perform_controller_tweaks() {
void game::start_game(const player_map::const_iterator starter) {
const simple_wml::node::child_list & sides = level_.root().children("side");
- DBG_GAME << "****\n Starting game. sides = " << std::endl;
- DBG_GAME << debug_sides_info() << std::endl;
- DBG_GAME << "****" << std::endl;
+ LOG_GAME << "****\n Starting game. sides = " << std::endl;
+ LOG_GAME << debug_sides_info() << std::endl;
+ DBG_GAME << "**** level = " << std::endl << level_.output() << std::endl;
+ LOG_GAME << "****" << std::endl;
started_ = true;
@@ -1120,8 +1121,11 @@ bool game::add_player(const network::connection player, bool observer) {
<< ". (socket: " << player << ")\n";
user->second.mark_available(id_, name_);
user->second.set_status((observer) ? player::OBSERVING : player::PLAYING);
+ DBG_GAME << "**** A player has joined" << std::endl;
DBG_GAME << debug_player_info();
// Send the user the game data.
+ DBG_GAME << "**** level = " << std::endl << level_.output() << std::endl;
+ DBG_GAME << "****" << std::endl;
if (!wesnothd::send_to_one(level_, player)) return false;
if(started_) {
Something went wrong with that request. Please try again.