fix ghost items in persistent games

src/Lobby.cc

@@ -123,7 +123,7 @@ void Lobby::FloorItemManager::clear() {
 }
 
 uint32_t Lobby::FloorItemManager::reassign_all_item_ids(uint32_t next_item_id) {
-  unordered_map<uint32_t, shared_ptr<FloorItem>> old_items;
+  ::map<uint32_t, shared_ptr<FloorItem>> old_items;
   old_items.swap(this->items);
   for (auto& queue : this->queue_for_client) {
     queue.clear();

src/Lobby.hh

@@ -36,7 +36,9 @@ struct Lobby : public std::enable_shared_from_this<Lobby> {
   struct FloorItemManager {
     PrefixedLogger log;
     uint64_t next_drop_number;
-    std::unordered_map<uint32_t, std::shared_ptr<FloorItem>> items; // Keyed on item_id
+    // It's important that this is a map and not an unordered_map. See the
+    // comment in send_game_item_state for more details.
+    std::map<uint32_t, std::shared_ptr<FloorItem>> items; // Keyed on item_id
     std::array<std::map<uint64_t, std::shared_ptr<FloorItem>>, 12> queue_for_client;
 
     FloorItemManager(uint32_t lobby_id, uint8_t floor);

src/SendCommands.cc

@@ -2381,8 +2381,16 @@ void send_game_item_state(shared_ptr<Client> c) {
 
   for (size_t floor = 0; floor < 0x10; floor++) {
     const auto& m = l->floor_item_managers.at(floor);
-    for (const auto& it : m.queue_for_client.at(c->lobby_client_id)) {
+    // It's important that these are added in increasing order of item_id (hence
+    // why items is a map and not an unordered_map), since the game uses binary
+    // search to find floor items when picking them up. If items aren't in the
+    // correct order, the game may fail to find an item when attempting to pick
+    // it up, causing "ghost items" which are visible but can't be picked up.
+    for (const auto& it : m.items) {
       const auto& item = it.second;
+      if (!item->visible_to_client(c->lobby_client_id)) {
+        continue;
+      }
 
       FloorItem fi;
       fi.floor = floor;