update game join procedure implementation

src/Client.hh

@@ -190,6 +190,7 @@ public:
   std::unordered_set<uint32_t> blocked_senders;
   std::unique_ptr<PlayerDispDataDCPCV3> v1_v2_last_reported_disp;
   std::shared_ptr<Parsed6x70Data> last_reported_6x70;
+  std::unordered_set<uint16_t> expected_game_state_sync_commands; // (command_num << 8) | target_client_id
   // These are null unless the client is within the trade sequence (D0-D4 or EE commands)
   std::unique_ptr<PendingItemTrade> pending_item_trade;
   std::unique_ptr<PendingCardTrade> pending_card_trade;

src/CommandFormats.hh

@@ -1139,17 +1139,37 @@ struct C_CharacterData_BB_61_98 {
 // 64 (S->C): Join game
 // Internal name: RcvStartGame3
 
-// This is sent to the joining player; the other players get a 65 instead. Note that (except on Episode 3) this command
-// does not include the player's disp or inventory data. The clients in the game are responsible for sending that data
-// to each other during the join process with 60/62/6C/6D commands.
-
-// After receiving a 64 command, the client starts the game loading procedure, during which it will completely ignore
-// other 64 or 65 commands, and will delay processing of all other commands except 1D until loading is done. If more
-// than 0x10000 bytes of commands are sent during loading, any commands that don't fit in the buffer are lost.
-
 // Curiously, this command is named RcvStartGame3 internally, while 0E is named RcvStartGame. The string RcvStartGame2
 // appears in the DC versions, but it seems the relevant code was deleted - there are no references to the string.
 
+// The game joining procedure goes as follows:
+// 1. The server sends 64 to the joining player, and 65 to all other players in the game. This pauses the game and
+//    brings up the "please wait" message box for all players. The joining player unloads the lobby assets and begins
+//    loading Pioneer 2 assets. On v2 and later, the joining player sends 8A near the beginning of this procedure.
+//    During this time, the joining player will completely ignore other 64 or 65 commands, and will delay processing of
+//    all other commands except 1D until loading is done. If more than 0x10000 bytes of commands are sent during
+//    loading, any commands that don't fit in the buffer are lost.
+// 2. If the joining player is not the only player in the game:
+//    a. If the leader is DC v1 or later, the leader sends 6x6D (item state).
+//    b. The leader sends 6x6B (enemy state).
+//    c. The leader sends 6x6C (object state).
+//    d. If the leader is DC NTE or DC 11/2000, the leader sends 6x6D (item state).
+//    e. The leader sends 6x6E (set flag state).
+//    f. If the leader is DC v1 or later, the leader sends 6x6F (quest flag state).
+//    g. If the leader is DC v1 or later, the leader sends 6x71 (construct player).
+//    h. All players except the joining player send 6x70 to the joining player. (This is not synchronized; non-leader
+//       players do not wait for any of the above things to happen, so their 6x70 commands may be interleaved with the
+//       preceding commands from the leader, or may arrive after the following 6x72.) Character data is sent in a
+//       different format here (6x70 instead of 65) because it contains ephemeral fields that the server doesn't know
+//       about - things like current HP, state, game flags, player flags, etc. which are not present in 65.
+//    i. If the leader is not BB, the leader sends 6x72 to all players, which resumes the game. If the leader is BB,
+//       the server is responsible for sending 6x72, and should do so here. (There is no sequence-breaking risk here,
+//       since the 6x72 sent to other players will always be ordered after the 65 from the server, so they will always
+//       send a 6x70 containing their player state at the time the game was paused.)
+// 3. Once the joining player has fully loaded, the player processes all the commands sent during loading, which sets
+//    up the game state and constructs all the other players. Once the local player is constructed, the joining player
+//    sends 6F, which notifies the server that it can unlock the game and allow more players to join.
+
 // Header flag = entry count
 template <typename LobbyDataT>
 struct S_JoinGameT_DC_PC {
@@ -1521,10 +1541,10 @@ struct C_ConnectionInfo_DCNTE_8A {
 // header.flag is a success flag. If it's zero, the client shows an error message and disconnects. Otherwise, the
 // client responds with an 8B command.
 
-// 8A (C->S): Request lobby/game name (except DC NTE)
+// 8A (C->S): Request lobby/game name (DCv2 and later)
 // No arguments.
 
-// 8A (S->C): Lobby/game name (except DC NTE)
+// 8A (S->C): Lobby/game name (DCv2 and later)
 // Contents is a string containing the lobby or game name. All versions after DCv1 send an 8A command to request the
 // team name after joining a game. The response is used to handle the team_name token in quest strings, and appears in
 // some Challenge Mode information windows.
@@ -2623,7 +2643,7 @@ struct C_GuildCardDataRequest_BB_03DC {
 // DD (S->C): Send quest state to joining player (BB)
 // When a player joins a game with a quest already in progress, the server should send this command to the leader.
 // No arguments except header.flag, which is the client ID that the leader should send quest state to. The leader will
-// then send a series of target commands (62/6D) that the server can forward to the joining player.
+// then send 6x6D, 6x6B, 6x6C, and 6x6E, in that order, targeted at the client specified in header.flag.
 
 // DE (S->C): Rare monster list (BB)
 

src/ReceiveCommands.cc

@@ -3259,6 +3259,12 @@ static void on_joinable_quest_loaded(shared_ptr<Client> c) {
   // happens when the response to the ping (1D) is received, so we don't need the game join command queue in that case.
   if (leader_c->version() == Version::BB_V4) {
     send_command(leader_c, 0xDD, c->lobby_client_id);
+    l->log.info_f("Expecting {} to send 6x6B, 6x6C, 6x6D, and 6x6E to {}, and 6x72 to all",
+        leader_c->channel->name, c->channel->name);
+    leader_c->expected_game_state_sync_commands.emplace(0x6B00 | (c->lobby_client_id));
+    leader_c->expected_game_state_sync_commands.emplace(0x6C00 | (c->lobby_client_id));
+    leader_c->expected_game_state_sync_commands.emplace(0x6D00 | (c->lobby_client_id));
+    leader_c->expected_game_state_sync_commands.emplace(0x6E00 | (c->lobby_client_id));
     c->log.info_f("Creating game join command queue");
     c->game_join_command_queue = make_unique<deque<Client::JoinCommand>>();
   } else {
@@ -4998,6 +5004,16 @@ static asio::awaitable<void> on_6F(shared_ptr<Client> c, Channel::Message& msg)
     }
   }
 
+  // DC NTE creates players in the invisible state by default; if the joiner is not DC NTE, it won't send 6x23 to make
+  // itself visible, so we have to do it
+  for (const auto& lc : l->clients) {
+    if (lc && (lc != c) && is_pre_v1(lc->version())) {
+      G_EntityIDHeader cmd = {
+          translate_subcommand_number(lc->version(), Version::BB_V4, 0x23), 0x01, c->lobby_client_id};
+      send_command_t(lc, 0x60, 0x00, cmd);
+    }
+  }
+
   if (l->ep3_server && l->ep3_server->battle_finished) {
     auto s = l->require_server_state();
     l->log.info_f("Deleting Episode 3 server state");
@@ -5021,7 +5037,6 @@ static asio::awaitable<void> on_6F(shared_ptr<Client> c, Channel::Message& msg)
     send_text_message_fmt(c, "Rare seed: {:08X}/{:c}\nVariations:{}\n", l->random_seed, type_ch, variations_str);
   }
 
-  bool should_resume_game = true;
   if (c->version() == Version::BB_V4) {
     send_set_exp_multiplier(l);
     send_update_team_reward_flags(c);
@@ -5045,7 +5060,6 @@ static asio::awaitable<void> on_6F(shared_ptr<Client> c, Channel::Message& msg)
       send_open_quest_file(c, dat_filename, dat_filename, "", vq->meta.quest_number, QuestFileType::ONLINE, vq->dat_contents);
       c->set_flag(Client::Flag::LOADING_RUNNING_JOINABLE_QUEST);
       c->log.info_f("LOADING_RUNNING_JOINABLE_QUEST flag set");
-      should_resume_game = false;
 
     } else if ((msg.command == 0x016F) && c->check_flag(Client::Flag::LOADING_RUNNING_JOINABLE_QUEST)) {
       c->clear_flag(Client::Flag::LOADING_RUNNING_JOINABLE_QUEST);
@@ -5054,13 +5068,6 @@ static asio::awaitable<void> on_6F(shared_ptr<Client> c, Channel::Message& msg)
     send_rare_enemy_index_list(c, l->map_state->bb_rare_enemy_indexes);
   }
 
-  // We should resume the game if:
-  // - command is 016F and a joinable quest is in progress
-  // - command is 006F and a joinable quest is NOT in progress
-  if (should_resume_game) {
-    send_resume_game(l, c);
-  }
-
   // Handle initial commands for spectator teams
   auto watched_lobby = l->watched_lobby.lock();
   if (l->battle_player && l->check_flag(Lobby::Flag::START_BATTLE_PLAYER_IMMEDIATELY)) {

src/ReceiveSubcommands.cc

@@ -299,9 +299,19 @@ static asio::awaitable<void> on_debug_info(shared_ptr<Client>, SubcommandMessage
   co_return;
 }
 
-static asio::awaitable<void> on_forward_check_game_loading(shared_ptr<Client> c, SubcommandMessage& msg) {
+void check_expected_loading_command(shared_ptr<Client> c, const SubcommandMessage& msg) {
+  const auto& base_header = msg.check_size_t<G_UnusedHeader>(0xFFFF);
+  uint8_t subcommand = translate_subcommand_number(Version::BB_V4, c->version(), base_header.subcommand);
+  if (!c->expected_game_state_sync_commands.erase((subcommand << 8) | (msg.flag & 0xFF))) {
+    throw std::runtime_error("client sent unexpected game state sync command");
+  }
+}
+
+static asio::awaitable<void> on_forward_check_game_loading_expected(shared_ptr<Client> c, SubcommandMessage& msg) {
   auto l = c->require_lobby();
-  if (l->is_game() && l->any_client_loading()) {
+  if (l->is_game()) {
+    check_expected_loading_command(c, msg);
+    msg.check_size_t<G_UnusedHeader>().unused = 0;
     forward_subcommand(c, msg);
   }
   co_return;
@@ -492,6 +502,8 @@ static shared_ptr<Client> get_sync_target(
 }
 
 static asio::awaitable<void> on_sync_joining_player_compressed_state(shared_ptr<Client> c, SubcommandMessage& msg) {
+  check_expected_loading_command(c, msg);
+
   auto target = get_sync_target(c, msg.command, msg.flag, false); // Checks l->is_game
   if (!target) {
     co_return;
@@ -702,6 +714,15 @@ static asio::awaitable<void> on_sync_joining_player_compressed_state(shared_ptr<
       } else {
         send_game_set_state(target);
       }
+
+      // If the sender is the leader and is pre-V1, and the target is V1 or later, we need to synthesize a 6x71 command
+      // to tell the target to construct its TObjPlayer. (If both are pre-V1, the target won't expect this command; if
+      // both are V1 or later, the leader will send this command itself.)
+      if (is_pre_v1(c->version()) && !is_pre_v1(target->version())) {
+        G_UnusedHeader cmd = {0x71, 0x01, 0x0000};
+        send_command_t(target, 0x62, target->lobby_client_id, cmd);
+      }
+
       break;
     }
 
@@ -712,8 +733,9 @@ static asio::awaitable<void> on_sync_joining_player_compressed_state(shared_ptr<
 
 template <typename CmdT>
 static asio::awaitable<void> on_sync_joining_player_quest_flags_t(shared_ptr<Client> c, SubcommandMessage& msg) {
-  const auto& cmd = msg.check_size_t<CmdT>();
+  check_expected_loading_command(c, msg);
 
+  const auto& cmd = msg.check_size_t<CmdT>();
   if (!command_is_private(msg.command)) {
     co_return;
   }
@@ -1211,9 +1233,9 @@ uint32_t Parsed6x70Data::get_player_flags(bool is_v3) const {
       : Parsed6x70Data::convert_player_flags(this->player_flags, is_v3);
 }
 
-static asio::awaitable<void> on_sync_joining_player_disp_and_inventory(
-    shared_ptr<Client> c, SubcommandMessage& msg) {
+static asio::awaitable<void> on_sync_joining_player_disp_and_inventory(shared_ptr<Client> c, SubcommandMessage& msg) {
   auto s = c->require_server_state();
+  check_expected_loading_command(c, msg);
 
   // In V1/V2 games, this command sometimes is sent after the new client has finished loading, so we don't check
   // l->any_client_loading() here.
@@ -1222,28 +1244,18 @@ static asio::awaitable<void> on_sync_joining_player_disp_and_inventory(
     co_return;
   }
 
-  // If the sender is the leader and is pre-V1, and the target is V1 or later, we need to synthesize a 6x71 command to
-  // tell the target all state has been sent. (If both are pre-V1, the target won't expect this command; if both are V1
-  // or later, the leader will send this command itself.)
-  Version target_v = target->version();
-  Version c_v = c->version();
-  if (is_pre_v1(c_v) && !is_pre_v1(target_v)) {
-    static const be_uint32_t data = 0x71010000;
-    send_command(target, 0x62, target->lobby_client_id, &data, sizeof(data));
-  }
-
   bool is_client_customisation = c->check_flag(Client::Flag::IS_CLIENT_CUSTOMIZATION);
-  switch (c_v) {
+  switch (c->version()) {
     case Version::DC_NTE:
       c->last_reported_6x70.reset(new Parsed6x70Data(
           msg.check_size_t<G_SyncPlayerDispAndInventory_DCNTE_6x70>(),
-          c->login->account->account_id, c_v, is_client_customisation));
+          c->login->account->account_id, c->version(), is_client_customisation));
       c->last_reported_6x70->clear_dc_protos_unused_item_fields();
       break;
     case Version::DC_11_2000:
       c->last_reported_6x70.reset(new Parsed6x70Data(
           msg.check_size_t<G_SyncPlayerDispAndInventory_DC112000_6x70>(),
-          c->login->account->account_id, c->language(), c_v, is_client_customisation));
+          c->login->account->account_id, c->language(), c->version(), is_client_customisation));
       c->last_reported_6x70->clear_dc_protos_unused_item_fields();
       break;
     case Version::DC_V1:
@@ -1252,8 +1264,8 @@ static asio::awaitable<void> on_sync_joining_player_disp_and_inventory(
     case Version::PC_V2:
       c->last_reported_6x70.reset(new Parsed6x70Data(
           msg.check_size_t<G_SyncPlayerDispAndInventory_DC_PC_6x70>(),
-          c->login->account->account_id, c_v, is_client_customisation));
-      if (c_v == Version::DC_V1) {
+          c->login->account->account_id, c->version(), is_client_customisation));
+      if (c->version() == Version::DC_V1) {
         c->last_reported_6x70->clear_v1_unused_item_fields();
       }
       break;
@@ -1263,17 +1275,17 @@ static asio::awaitable<void> on_sync_joining_player_disp_and_inventory(
     case Version::GC_EP3:
       c->last_reported_6x70.reset(new Parsed6x70Data(
           msg.check_size_t<G_SyncPlayerDispAndInventory_GC_6x70>(),
-          c->login->account->account_id, c_v, is_client_customisation));
+          c->login->account->account_id, c->version(), is_client_customisation));
       break;
     case Version::XB_V3:
       c->last_reported_6x70.reset(new Parsed6x70Data(
           msg.check_size_t<G_SyncPlayerDispAndInventory_XB_6x70>(),
-          c->login->account->account_id, c_v, is_client_customisation));
+          c->login->account->account_id, c->version(), is_client_customisation));
       break;
     case Version::BB_V4:
       c->last_reported_6x70.reset(new Parsed6x70Data(
           msg.check_size_t<G_SyncPlayerDispAndInventory_BB_6x70>(),
-          c->login->account->account_id, c_v, is_client_customisation));
+          c->login->account->account_id, c->version(), is_client_customisation));
       break;
     default:
       throw logic_error("6x70 command from unknown game version");
@@ -1281,6 +1293,15 @@ static asio::awaitable<void> on_sync_joining_player_disp_and_inventory(
 
   c->pos = c->last_reported_6x70->base.pos;
   send_game_player_state(target, c, false);
+
+  // On BB, the server is expected to send 6x72 rather than the client. We just do it at the same time the client did
+  // on previous versions (the leader sends it immediately after its own 6x70).
+  if (c->version() == Version::BB_V4) {
+    auto l = c->require_lobby();
+    if (c->lobby_client_id == l->leader_id) {
+      send_resume_game(l, target);
+    }
+  }
 }
 
 static asio::awaitable<void> on_forward_check_client(shared_ptr<Client> c, SubcommandMessage& msg) {
@@ -1619,7 +1640,6 @@ static asio::awaitable<void> on_change_floor_6x1F(shared_ptr<Client> c, Subcomma
     if (c->check_flag(Client::Flag::LOADING)) {
       c->clear_flag(Client::Flag::LOADING);
       c->log.info_f("LOADING flag cleared");
-      send_resume_game(c->require_lobby(), c);
       c->require_lobby()->assign_inventory_and_bank_item_ids(c, true);
     }
 
@@ -5653,8 +5673,8 @@ const vector<SubcommandDefinition> subcommand_definitions{
     /* 6x6E */ {0x5F, 0x66, 0x6E, on_sync_joining_player_compressed_state},
     /* 6x6F */ {NONE, NONE, 0x6F, on_sync_joining_player_quest_flags},
     /* 6x70 */ {0x60, 0x67, 0x70, on_sync_joining_player_disp_and_inventory},
-    /* 6x71 */ {NONE, NONE, 0x71, on_forward_check_game_loading},
-    /* 6x72 */ {0x61, 0x68, 0x72, on_forward_check_game_loading},
+    /* 6x71 */ {NONE, NONE, 0x71, on_forward_check_game_loading_expected},
+    /* 6x72 */ {0x61, 0x68, 0x72, on_forward_check_game_loading_expected},
     /* 6x73 */ {NONE, NONE, 0x73, on_forward_check_game_quest},
     /* 6x74 */ {0x62, 0x69, 0x74, on_word_select, SDF::ALWAYS_FORWARD_TO_WATCHERS},
     /* 6x75 */ {NONE, NONE, 0x75, on_set_quest_flag},

src/SendCommands.cc

@@ -2545,6 +2545,7 @@ void send_arrow_update(shared_ptr<Lobby> l) {
 }
 
 void send_unblock_join(shared_ptr<Client> c) {
+  // Pre-V1 clients don't have 6x71 at all
   if (!is_pre_v1(c->version())) {
     static const be_uint32_t data = 0x71010000;
     send_command(c, 0x60, 0x00, &data, sizeof(be_uint32_t));
@@ -2553,9 +2554,16 @@ void send_unblock_join(shared_ptr<Client> c) {
 
 void send_resume_game(shared_ptr<Lobby> l, shared_ptr<Client> ready_client) {
   for (auto lc : l->clients) {
-    if (lc && (lc != ready_client) && !is_pre_v1(lc->version())) {
-      static const be_uint32_t data = 0x72010000;
-      send_command(lc, 0x60, 0x00, &data, sizeof(be_uint32_t));
+    if (lc && (lc != ready_client)) {
+      G_UnusedHeader cmd = {0x00, 0x01, 0x0000};
+      if (lc->version() == Version::DC_NTE) {
+        cmd.subcommand = 0x61;
+      } else if (lc->version() == Version::DC_11_2000) {
+        cmd.subcommand = 0x68;
+      } else {
+        cmd.subcommand = 0x72;
+      }
+      send_command_t(lc, 0x60, 0x00, cmd);
     }
   }
 }

src/ServerState.cc

@@ -192,6 +192,26 @@ void ServerState::send_lobby_join_notifications(shared_ptr<Lobby> l, shared_ptr<
       send_player_join_notification(watcher_c, watcher_l, joining_client);
     }
   }
+
+  if (l->is_game()) {
+    for (auto lc : l->clients) {
+      if (!lc || (lc == joining_client)) {
+        continue;
+      }
+      if (lc->lobby_client_id == l->leader_id) {
+        l->log.info_f("Expecting {} to send game state to {}", lc->channel->name, joining_client->channel->name);
+        lc->expected_game_state_sync_commands.emplace(0x6B00 | (joining_client->lobby_client_id));
+        lc->expected_game_state_sync_commands.emplace(0x6C00 | (joining_client->lobby_client_id));
+        lc->expected_game_state_sync_commands.emplace(0x6D00 | (joining_client->lobby_client_id));
+        lc->expected_game_state_sync_commands.emplace(0x6E00 | (joining_client->lobby_client_id));
+        lc->expected_game_state_sync_commands.emplace(0x6F00 | (joining_client->lobby_client_id));
+        lc->expected_game_state_sync_commands.emplace(0x7100 | (joining_client->lobby_client_id));
+        lc->expected_game_state_sync_commands.emplace(0x7200);
+      }
+      l->log.info_f("Expecting {} to send 6x70 to {}", lc->channel->name, joining_client->channel->name);
+      lc->expected_game_state_sync_commands.emplace(0x7000 | (joining_client->lobby_client_id));
+    }
+  }
 }
 
 shared_ptr<Lobby> ServerState::find_lobby(uint32_t lobby_id) {