handle one 6x63 data race

src/CommandFormats.hh

@@ -4676,7 +4676,9 @@ struct G_ActivateMagEffect_6x61 {
 // 6x62: Unknown
 // This command has a handler, but it does nothing even on DC NTE.
 
-// 6x63: Destroy floor item (used when too many items have been dropped)
+// 6x63: Destroy floor item
+// This is sent by the leader to destroy a floor item when there are 50 or more
+// items already on the ground on the current floor.
 
 struct G_DestroyFloorItem_6x5C_6x63 {
   G_UnusedHeader header;

src/Lobby.hh

@@ -91,8 +91,7 @@ struct Lobby : public std::enable_shared_from_this<Lobby> {
     SERVER_DUPLICATE = 4,
   };
 
-  std::weak_ptr<ServerState>
+  std::weak_ptr<ServerState> server_state;
-      server_state;
   PrefixedLogger log;
 
   uint32_t lobby_id;

src/ReceiveCommands.cc

@@ -3243,6 +3243,12 @@ static void on_61_98(shared_ptr<Client> c, uint16_t command, uint32_t flag, stri
 
   c->update_channel_name();
 
+  // If the player is BB and has just left a game, sync their save file to the
+  // client to make sure it's up to date
+  if ((c->version() == Version::BB_V4) && (command == 0x98)) {
+    send_complete_player_bb(c);
+  }
+
   if (command == 0x61) {
     if (c->pending_character_export) {
       unique_ptr<Client::PendingCharacterExport> pending_export = std::move(c->pending_character_export);

src/ReceiveSubcommands.cc

@@ -3724,29 +3724,52 @@ static void on_destroy_floor_item(shared_ptr<Client> c, uint8_t command, uint8_t
   }
 
   auto s = c->require_server_state();
-  auto fi = l->remove_item(cmd.floor, cmd.item_id, 0xFF);
+  shared_ptr<Lobby::FloorItem> fi;
-  auto name = s->describe_item(c->version(), fi->data, false);
+  try {
-  l->log.info("Player %hhu destroyed floor item %08" PRIX32 " (%s)", c->lobby_client_id, cmd.item_id.load(), name.c_str());
+    fi = l->remove_item(cmd.floor, cmd.item_id, 0xFF);
+  } catch (const out_of_range&) {
+  }
 
-  // Only forward to players for whom the item was visible
+  if (!fi) {
-  for (size_t z = 0; z < l->clients.size(); z++) {
+    // There are generally two data races that could occur here. Either the
-    auto lc = l->clients[z];
+    // player attempted to evict the item at the same time the server did (that
-    if (lc && fi->visible_to_client(z)) {
+    // is, the client's and server's 6x63 commands crossed paths on the
-      if (lc->version() != c->version()) {
+    // network), or the player attempted to evict an item that was already
-        G_DestroyFloorItem_6x5C_6x63 out_cmd = cmd;
+    // picked up. The former case is easy to handle; we can just ignore the
-        switch (lc->version()) {
+    // command. The latter case is more difficult - we have to know which
-          case Version::DC_NTE:
+    // player picked up the item and send a 6x2B command to the sender, to sync
-            out_cmd.header.subcommand = is_6x5C ? 0x4E : 0x55;
+    // their item state with the server's again. We can't just look through the
-            break;
+    // players' inventories to find the item ID, since item IDs can be
-          case Version::DC_V1_11_2000_PROTOTYPE:
+    // destroyed when stackable items or Meseta are picked up.
-            out_cmd.header.subcommand = is_6x5C ? 0x55 : 0x5C;
+    // TODO: We don't actually handle the evict/pickup conflict case. This case
-            break;
+    // is probably quite rare, but we should eventually handle it.
-          default:
+    l->log.info("Player %hhu attempted to destroy floor item %08" PRIX32 ", but it is missing",
-            out_cmd.header.subcommand = is_6x5C ? 0x5C : 0x63;
+        c->lobby_client_id, cmd.item_id.load());
+
+  } else {
+    auto name = s->describe_item(c->version(), fi->data, false);
+    l->log.info("Player %hhu destroyed floor item %08" PRIX32 " (%s)", c->lobby_client_id, cmd.item_id.load(), name.c_str());
+
+    // Only forward to players for whom the item was visible
+    for (size_t z = 0; z < l->clients.size(); z++) {
+      auto lc = l->clients[z];
+      if (lc && fi->visible_to_client(z)) {
+        if (lc->version() != c->version()) {
+          G_DestroyFloorItem_6x5C_6x63 out_cmd = cmd;
+          switch (lc->version()) {
+            case Version::DC_NTE:
+              out_cmd.header.subcommand = is_6x5C ? 0x4E : 0x55;
+              break;
+            case Version::DC_V1_11_2000_PROTOTYPE:
+              out_cmd.header.subcommand = is_6x5C ? 0x55 : 0x5C;
+              break;
+            default:
+              out_cmd.header.subcommand = is_6x5C ? 0x5C : 0x63;
+          }
+          send_command_t(lc, command, flag, out_cmd);
+        } else {
+          send_command_t(lc, command, flag, cmd);
         }
-        send_command_t(lc, command, flag, out_cmd);
-      } else {
-        send_command_t(lc, command, flag, cmd);
       }
     }
   }