fix quest barrier and implement v3/bb file chunk acknowledge commands

src/Client.hh

@@ -135,9 +135,10 @@ struct Client {
   bool proxy_suppress_remote_login;
   bool proxy_zero_remote_guild_card;
 
-  // DOL file loading state
+  // File loading state
   uint32_t dol_base_addr;
   std::shared_ptr<DOLFileIndex::DOLFile> loading_dol_file;
+  std::unordered_map<std::string, std::shared_ptr<const std::string>> sending_files;
 
   Client(struct bufferevent* bev, GameVersion version, ServerBehavior server_behavior);
   ~Client();

src/CommandFormats.hh

@@ -628,7 +628,6 @@ struct S_WriteFile_13_A7 {
 // 13 (C->S): Confirm file write (V3/BB)
 // Client sends this in response to each 13 sent by the server. It appears these
 // are only sent by V3 and BB - PSO DC and PC do not send these.
-// This structure is for documentation only; newserv ignores these.
 
 // header.flag = file chunk index (same as in the 13/A7 sent by the server)
 struct C_WriteFileConfirmation_V3_BB_13_A7 {
@@ -870,10 +869,8 @@ struct S_OpenFile_BB_44_A6 {
   ptext<char, 0x18> name;
 } __packed__;
 
-// 44 (C->S): Confirm open file
+// 44 (C->S): Confirm open file (V3/BB)
 // Client sends this in response to each 44 sent by the server.
-// This structure is for documentation only; newserv ignores these.
-// TODO: Is this command sent by DC/PC clients?
 
 // header.flag = quest number (sort of - seems like the client just echoes
 // whatever the server sent in its header.flag field. Also quest numbers can be
@@ -1665,9 +1662,13 @@ struct S_QuestMenuEntry_BB_A2_A4 : S_QuestMenuEntry<char16_t, 0x7A> { } __packed
 // For .bin files, the flags field should be zero. For .pvr files, the flags
 // field should be 1. For .dat and .gba files, it seems the value in the flags
 // field does not matter.
+// Like the 44 command, the client->server form of this command is only used on
+// V3 and BB.
 
 // A7: Write download file
 // Same format as 13.
+// Like the 13 command, the client->server form of this command is only used on
+// V3 and BB.
 
 // A8: Invalid command
 

src/FileContentsCache.cc

@@ -15,7 +15,7 @@ FileContentsCache::File::File(
     const string& name,
     string&& data,
     uint64_t load_time)
-  : name(name), data(move(data)), load_time(load_time) { }
+  : name(name), data(new string(move(data))), load_time(load_time) { }
 
 shared_ptr<const FileContentsCache::File> FileContentsCache::replace(
     const string& name, string&& data, uint64_t t) {

src/FileContentsCache.hh

@@ -14,7 +14,7 @@ class FileContentsCache {
 public:
   struct File {
     std::string name;
-    std::string data;
+    shared_ptr<const std::string> data;
     uint64_t load_time;
 
     File() = delete;
@@ -68,29 +68,29 @@ public:
   template <typename T, typename NameT>
   GetObjResult<T> get_obj_or_load(NameT name) {
     auto res = this->get_or_load(name);
-    if (res.file->data.size() != sizeof(T)) {
+    if (res.file->data->size() != sizeof(T)) {
       throw runtime_error("cached string size is incorrect");
     }
-    return {*reinterpret_cast<const T*>(res.file->data.data()), res.file, res.generate_called};
+    return {*reinterpret_cast<const T*>(res.file->data->data()), res.file, res.generate_called};
   }
   template <typename T, typename NameT>
   GetObjResult<T> get_obj_or_throw(NameT name) {
     auto res = this->get_or_throw(name);
-    if (res.file->data.size() != sizeof(T)) {
+    if (res.file->data->size() != sizeof(T)) {
       throw runtime_error("cached string size is incorrect");
     }
-    return {*reinterpret_cast<const T*>(res.file->data.data()), res.file, res.generate_called};
+    return {*reinterpret_cast<const T*>(res.file->data->data()), res.file, res.generate_called};
   }
   template <typename T, typename NameT>
   GetObjResult<T> get_obj(NameT name, std::function<T(const std::string&)> generate) {
     uint64_t t = now();
     try {
       auto& f = this->name_to_file.at(name);
-      if (f->data.size() != sizeof(T)) {
+      if (f->data->size() != sizeof(T)) {
         throw runtime_error("cached string size is incorrect");
       }
       if (this->ttl_usecs && (t - f->load_time < this->ttl_usecs)) {
-        return {*reinterpret_cast<const T*>(f->data.data()), f, false};
+        return {*reinterpret_cast<const T*>(f->data->data()), f, false};
       }
     } catch (const out_of_range& e) { }
     T value = generate(name);
@@ -101,7 +101,7 @@ public:
   template <typename T, typename NameT>
   GetObjResult<T> replace_obj(NameT name, const T& value) {
     auto cached_value = this->replace(name, &value, sizeof(value));
-    return {*reinterpret_cast<const T*>(cached_value->data.data()), cached_value, false};
+    return {*reinterpret_cast<const T*>(cached_value->data->data()), cached_value, false};
   }
 
 private:

src/ReceiveCommands.cc

@@ -145,7 +145,7 @@ static void send_client_to_proxy_server(shared_ptr<ServerState> s, shared_ptr<Cl
     try {
       string key = string_printf("proxy_remote_guild_card_number:%" PRIX32, c->license->serial_number);
       const auto& entry = client_options_cache.get_or_throw(key);
-      session->remote_guild_card_number = stoul(entry->data, nullptr, 10);
+      session->remote_guild_card_number = stoul(*entry->data, nullptr, 10);
     } catch (const out_of_range&) { }
   }
 
@@ -158,7 +158,7 @@ static void send_proxy_destinations_menu(shared_ptr<ServerState> s, shared_ptr<C
   try {
     string key = string_printf("proxy_remote_guild_card_number:%" PRIX32, c->license->serial_number);
     const auto& entry = client_options_cache.get_or_throw(key);
-    uint32_t proxy_remote_guild_card_number = stoul(entry->data, nullptr, 10);
+    uint32_t proxy_remote_guild_card_number = stoul(*entry->data, nullptr, 10);
     string info_str = string_printf("Your remote Guild\nCard number is\noverridden as\n$C6%" PRIu32, proxy_remote_guild_card_number);
     send_ship_info(c, decode_sjis(info_str));
   } catch (const out_of_range&) { }
@@ -1771,14 +1771,10 @@ static void on_menu_selection(shared_ptr<ServerState> s, shared_ptr<Client> c,
             continue;
           }
 
-          // TODO: It looks like blasting all the chunks to the client at once
+          send_open_quest_file(l->clients[x], bin_basename + ".bin",
-          // can cause GC clients to crash in rare cases. Find a way to slow
+              bin_basename, bin_contents, QuestFileType::ONLINE);
-          // this down (perhaps by only sending each new chunk when they
+          send_open_quest_file(l->clients[x], dat_basename + ".dat",
-          // acknowledge the previous chunk with a 13 command).
+              dat_basename, dat_contents, QuestFileType::ONLINE);
-          send_quest_file(l->clients[x], bin_basename + ".bin", bin_basename,
-              *bin_contents, QuestFileType::ONLINE);
-          send_quest_file(l->clients[x], dat_basename + ".dat", dat_basename,
-              *dat_contents, QuestFileType::ONLINE);
 
           // There is no such thing as command AC on PSO V2 - quests just start
           // immediately when they're done downloading. (This is also the case
@@ -1804,10 +1800,10 @@ static void on_menu_selection(shared_ptr<ServerState> s, shared_ptr<Client> c,
         if (!is_ep3) {
           q = q->create_download_quest();
         }
-        send_quest_file(c, quest_name, bin_basename, *q->bin_contents(),
+        send_open_quest_file(c, quest_name, bin_basename, q->bin_contents(),
             is_ep3 ? QuestFileType::EPISODE_3 : QuestFileType::DOWNLOAD);
         if (dat_contents) {
-          send_quest_file(c, quest_name, dat_basename, *q->dat_contents(),
+          send_open_quest_file(c, quest_name, dat_basename, q->dat_contents(),
               is_ep3 ? QuestFileType::EPISODE_3 : QuestFileType::DOWNLOAD);
         }
       }
@@ -2134,7 +2130,42 @@ static void on_gba_file_request(shared_ptr<ServerState>, shared_ptr<Client> c,
   static FileContentsCache gba_file_cache(300 * 1000 * 1000);
   auto f = gba_file_cache.get_or_load("system/gba/" + filename).file;
 
-  send_quest_file(c, "", filename, f->data, QuestFileType::GBA_DEMO);
+  send_open_quest_file(c, "", filename, f->data, QuestFileType::GBA_DEMO);
+}
+
+static void send_file_chunk(
+    shared_ptr<Client> c,
+    const string& filename,
+    size_t chunk_index,
+    bool is_download_quest) {
+  shared_ptr<const string> data;
+  try {
+    data = c->sending_files.at(filename);
+  } catch (const out_of_range&) {
+    return;
+  }
+
+  size_t chunk_offset = chunk_index * 0x400;
+  if (chunk_offset >= data->size()) {
+    c->log.info("Done sending file %s", filename.c_str());
+    c->sending_files.erase(filename);
+  } else {
+    const void* chunk_data = data->data() + (chunk_index * 0x400);
+    size_t chunk_size = min<size_t>(data->size() - chunk_offset, 0x400);
+    send_quest_file_chunk(c, filename, chunk_index, chunk_data, chunk_size, is_download_quest);
+  }
+}
+
+static void on_ack_open_file(shared_ptr<ServerState>, shared_ptr<Client> c,
+    uint16_t command, uint32_t, const string& data) { // 44 A6
+  const auto& cmd = check_size_t<C_OpenFileConfirmation_44_A6>(data);
+  send_file_chunk(c, cmd.filename, 0, (command == 0xA6));
+}
+
+static void on_ack_write_file(shared_ptr<ServerState>, shared_ptr<Client> c,
+    uint16_t command, uint32_t flag, const string& data) { // 13 A7
+  const auto& cmd = check_size_t<C_WriteFileConfirmation_V3_BB_13_A7>(data);
+  send_file_chunk(c, cmd.filename, flag + 1, (command == 0xA7));
 }
 
 
@@ -3468,7 +3499,7 @@ static on_command_t handlers[0x100][6] = {
   /* 10 */ {on_checksums_done_patch, on_menu_selection,          on_menu_selection,               on_menu_selection,           on_menu_selection,           on_menu_selection,              }, /* 10 */
   /* 11 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 11 */
   /* 12 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 12 */
-  /* 13 */ {nullptr,                 on_ignored_command,         on_ignored_command,              on_ignored_command,          on_ignored_command,          on_ignored_command,             }, /* 13 */
+  /* 13 */ {nullptr,                 on_ignored_command,         on_ignored_command,              on_ack_write_file,           on_ack_write_file,           on_ack_write_file,              }, /* 13 */
   /* 14 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 14 */
   /* 15 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 15 */
   /* 16 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 16 */
@@ -3519,7 +3550,7 @@ static on_command_t handlers[0x100][6] = {
   /* 41 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 41 */
   /* 42 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 42 */
   /* 43 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 43 */
-  /* 44 */ {nullptr,                 on_ignored_command,         on_ignored_command,              on_ignored_command,          on_ignored_command,          on_ignored_command,             }, /* 44 */
+  /* 44 */ {nullptr,                 on_ignored_command,         on_ignored_command,              on_ack_open_file,            on_ack_open_file,            on_ack_open_file,               }, /* 44 */
   /* 45 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 45 */
   /* 46 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 46 */
   /* 47 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* 47 */
@@ -3620,8 +3651,8 @@ static on_command_t handlers[0x100][6] = {
   /* A3 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* A3 */
   /* A4 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* A4 */
   /* A5 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* A5 */
-  /* A6 */ {nullptr,                 nullptr,                    nullptr,                         on_ignored_command,          on_ignored_command,          nullptr,                        }, /* A6 */
+  /* A6 */ {nullptr,                 nullptr,                    nullptr,                         on_ack_open_file,            on_ack_open_file,            nullptr,                        }, /* A6 */
-  /* A7 */ {nullptr,                 nullptr,                    nullptr,                         on_ignored_command,          on_ignored_command,          nullptr,                        }, /* A7 */
+  /* A7 */ {nullptr,                 nullptr,                    nullptr,                         on_ack_write_file,           on_ack_write_file,           nullptr,                        }, /* A7 */
   /* A8 */ {nullptr,                 nullptr,                    nullptr,                         nullptr,                     nullptr,                     nullptr,                        }, /* A8 */
   /* A9 */ {nullptr,                 on_ignored_command,         on_ignored_command,              on_ignored_command,          on_ignored_command,          on_ignored_command,             }, /* A9 */
   /* AA */ {nullptr,                 nullptr,                    on_update_quest_statistics,      on_update_quest_statistics,  on_update_quest_statistics,  on_update_quest_statistics,     }, /* AA */

src/SendCommands.cc

@@ -489,16 +489,16 @@ void send_stream_file_index_bb(shared_ptr<Client> c) {
     string key = "system/blueburst/" + filename;
     auto cache_res = bb_stream_files_cache.get_or_load(key);
     auto& e = entries.emplace_back();
-    e.size = cache_res.file->data.size();
+    e.size = cache_res.file->data->size();
     // Computing the checksum can be slow, so we cache it along with the file
     // data. If the cache result was just populated, then it may be different,
     // so we always recompute the checksum in that case.
     if (cache_res.generate_called) {
-      e.checksum = crc32(cache_res.file->data.data(), e.size);
+      e.checksum = crc32(cache_res.file->data->data(), e.size);
       bb_stream_files_cache.replace_obj<uint32_t>(key + ".crc32", e.checksum);
     } else {
       auto compute_checksum = [&](const string&) -> uint32_t {
-        return crc32(cache_res.file->data.data(), e.size);
+        return crc32(cache_res.file->data->data(), e.size);
       };
       e.checksum = bb_stream_files_cache.get_obj<uint32_t>(key + ".crc32", compute_checksum).obj;
     }
@@ -513,13 +513,13 @@ void send_stream_file_chunk_bb(shared_ptr<Client> c, uint32_t chunk_index) {
   auto cache_result = bb_stream_files_cache.get("<BB stream file>", +[](const string&) -> string {
     size_t bytes = 0;
     for (const auto& name : stream_file_entries) {
-      bytes += bb_stream_files_cache.get_or_load("system/blueburst/" + name).file->data.size();
+      bytes += bb_stream_files_cache.get_or_load("system/blueburst/" + name).file->data->size();
     }
 
     string ret;
     ret.reserve(bytes);
     for (const auto& name : stream_file_entries) {
-      ret += bb_stream_files_cache.get_or_load("system/blueburst/" + name).file->data;
+      ret += *bb_stream_files_cache.get_or_load("system/blueburst/" + name).file->data;
     }
     return ret;
   });
@@ -528,11 +528,11 @@ void send_stream_file_chunk_bb(shared_ptr<Client> c, uint32_t chunk_index) {
   S_StreamFileChunk_BB_02EB chunk_cmd;
   chunk_cmd.chunk_index = chunk_index;
   size_t offset = sizeof(chunk_cmd.data) * chunk_index;
-  if (offset > contents.size()) {
+  if (offset > contents->size()) {
     throw runtime_error("client requested chunk beyond end of stream file");
   }
-  size_t bytes = min<size_t>(contents.size() - offset, sizeof(chunk_cmd.data));
+  size_t bytes = min<size_t>(contents->size() - offset, sizeof(chunk_cmd.data));
-  memcpy(chunk_cmd.data, contents.data() + offset, bytes);
+  memcpy(chunk_cmd.data, contents->data() + offset, bytes);
 
   size_t cmd_size = offsetof(S_StreamFileChunk_BB_02EB, data) + bytes;
   cmd_size = (cmd_size + 3) & ~3;
@@ -2197,9 +2197,9 @@ void send_quest_file_chunk(
     size_t chunk_index,
     const void* data,
     size_t size,
-    QuestFileType type) {
+    bool is_download_quest) {
   if (size > 0x400) {
-    throw invalid_argument("quest file chunks must be 1KB or smaller");
+    throw logic_error("quest file chunks must be 1KB or smaller");
   }
 
   S_WriteFile_13_A7 cmd;
@@ -2210,38 +2210,45 @@ void send_quest_file_chunk(
   }
   cmd.data_size = size;
 
-  send_command_t(c, (type == QuestFileType::ONLINE) ? 0x13 : 0xA7, chunk_index, cmd);
+  send_command_t(c, is_download_quest ? 0xA7 : 0x13, chunk_index, cmd);
 }
 
-void send_quest_file(shared_ptr<Client> c, const string& quest_name,
+void send_open_quest_file(shared_ptr<Client> c, const string& quest_name,
-    const string& basename, const string& contents, QuestFileType type) {
+    const string& basename, shared_ptr<const string> contents, QuestFileType type) {
 
   switch (c->version()) {
     case GameVersion::DC:
       send_quest_open_file_t<S_OpenFile_DC_44_A6>(
-          c, quest_name, basename, contents.size(), type);
+          c, quest_name, basename, contents->size(), type);
       break;
     case GameVersion::PC:
     case GameVersion::GC:
     case GameVersion::XB:
       send_quest_open_file_t<S_OpenFile_PC_V3_44_A6>(
-          c, quest_name, basename, contents.size(), type);
+          c, quest_name, basename, contents->size(), type);
       break;
     case GameVersion::BB:
       send_quest_open_file_t<S_OpenFile_BB_44_A6>(
-        c, quest_name, basename, contents.size(), type);
+          c, quest_name, basename, contents->size(), type);
       break;
     default:
       throw logic_error("cannot send quest files to this version of client");
   }
 
-  for (size_t offset = 0; offset < contents.size(); offset += 0x400) {
+  // For GC/XB/BB, we wait for acknowledgement commands before sending each
-    size_t chunk_bytes = contents.size() - offset;
+  // chunk. For DC/PC, we send the entire quest all at once.
-    if (chunk_bytes > 0x400) {
+  if ((c->version() == GameVersion::DC) || (c->version() == GameVersion::PC)) {
-      chunk_bytes = 0x400;
+    for (size_t offset = 0; offset < contents->size(); offset += 0x400) {
+      size_t chunk_bytes = contents->size() - offset;
+      if (chunk_bytes > 0x400) {
+        chunk_bytes = 0x400;
+      }
+      send_quest_file_chunk(c, basename.c_str(), offset / 0x400,
+          contents->data() + offset, chunk_bytes, (type != QuestFileType::ONLINE));
     }
-    send_quest_file_chunk(c, basename.c_str(), offset / 0x400,
+  } else {
-        contents.data() + offset, chunk_bytes, type);
+    c->sending_files.emplace(basename, contents);
+    c->log.info("Opened file %s", basename.c_str());
   }
 }
 
@@ -2268,7 +2275,9 @@ void send_quest_barrier_if_all_clients_ready(shared_ptr<Lobby> l) {
 
   // Check if any client is still loading
   for (x = 0; x < l->max_clients; x++) {
-    l->clients[x]->disconnect_hooks.erase(QUEST_BARRIER_DISCONNECT_HOOK_NAME);
+    if (l->clients[x]) {
+      l->clients[x]->disconnect_hooks.erase(QUEST_BARRIER_DISCONNECT_HOOK_NAME);
+    }
   }
 }
 

src/SendCommands.hh

@@ -336,9 +336,19 @@ enum class QuestFileType {
   GBA_DEMO,
 };
 
-void send_quest_file(std::shared_ptr<Client> c, const std::string& quest_name,
+void send_open_quest_file(
-    const std::string& basename, const std::string& contents,
+    std::shared_ptr<Client> c,
+    const std::string& quest_name,
+    const std::string& basename,
+    std::shared_ptr<const std::string> contents,
     QuestFileType type);
+void send_quest_file_chunk(
+    shared_ptr<Client> c,
+    const string& filename,
+    size_t chunk_index,
+    const void* data,
+    size_t size,
+    bool is_download_quest);
 void send_quest_barrier_if_all_clients_ready(std::shared_ptr<Lobby> l);
 
 void send_card_auction_if_all_clients_ready(

src/ServerState.cc

@@ -541,8 +541,7 @@ shared_ptr<const string> ServerState::load_bb_file(
   try {
     auto ret = cache.get_or_load("system/blueburst/" + effective_bb_directory_filename);
     static_game_data_log.info("Loaded %s", effective_bb_directory_filename.c_str());
-    // TODO: It's also not great that we copy the data here... sigh
+    return ret.file->data;
-    return shared_ptr<string>(new string(ret.file->data));
   } catch (const exception& e) {
     static_game_data_log.info("%s missing from system/blueburst", effective_bb_directory_filename.c_str());
     static_game_data_log.error("%s not found in any source", patch_index_filename.c_str());