From 5df98fb691c6c8d3574b601baa745bf3f472d95b Mon Sep 17 00:00:00 2001
From: Martin Michelsen <fuzziqersoftware+g@gmail.com>
Date: Mon, 27 May 2024 10:00:04 -0700
Subject: [PATCH] speed up quest loading

---
 src/ReceiveCommands.cc | 45 ++++++++++++++++++------------------------
 src/SendCommands.cc    | 31 ++++++++++++++++++-----------
 src/SendCommands.hh    |  2 ++
 3 files changed, 41 insertions(+), 37 deletions(-)

diff --git a/src/ReceiveCommands.cc b/src/ReceiveCommands.cc
index c73f3cdb..60aa3228 100644
--- a/src/ReceiveCommands.cc
+++ b/src/ReceiveCommands.cc
@@ -2970,39 +2970,32 @@ static void on_D7_GC(shared_ptr<Client> c, uint16_t, uint32_t, string& data) {
   }
 }
 
-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;
+static void on_13_A7_GC(shared_ptr<Client> c, uint16_t command, uint32_t flag, string& data) {
+  // See comment in send_open_quest_file about why we only have logic here for
+  // GC clients, and ignore this command on all other versions
+  const auto& cmd = check_size_t<C_WriteFileConfirmation_V3_BB_13_A7>(data);
+  bool is_download_quest = (command == 0xA7);
+  string filename = cmd.filename.decode();
+  size_t chunk_to_send = flag + GC_QUEST_LOAD_MAX_CHUNKS_IN_FLIGHT;
+
+  shared_ptr<const string> file_data;
   try {
-    data = c->sending_files.at(filename);
+    file_data = c->sending_files.at(filename);
   } catch (const out_of_range&) {
     return;
   }
 
-  size_t chunk_offset = chunk_index * 0x400;
-  if (chunk_offset >= data->size()) {
+  size_t chunk_offset = chunk_to_send * 0x400;
+  if (chunk_offset >= file_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);
+    const void* chunk_data = file_data->data() + (chunk_to_send * 0x400);
+    size_t chunk_size = min<size_t>(file_data->size() - chunk_offset, 0x400);
+    send_quest_file_chunk(c, filename, chunk_to_send, chunk_data, chunk_size, is_download_quest);
   }
 }
 
-static void on_44_A6_V3_BB(shared_ptr<Client> c, uint16_t command, uint32_t, string& data) {
-  const auto& cmd = check_size_t<C_OpenFileConfirmation_44_A6>(data);
-  send_file_chunk(c, cmd.filename.decode(), 0, (command == 0xA6));
-}
-
-static void on_13_A7_V3_BB(shared_ptr<Client> c, uint16_t command, uint32_t flag, string& data) {
-  const auto& cmd = check_size_t<C_WriteFileConfirmation_V3_BB_13_A7>(data);
-  send_file_chunk(c, cmd.filename.decode(), flag + 1, (command == 0xA7));
-}
-
 static void on_61_98(shared_ptr<Client> c, uint16_t command, uint32_t flag, string& data) {
   auto s = c->require_server_state();
   auto player = c->character();
@@ -5322,7 +5315,7 @@ static on_command_t handlers[0x100][NUM_NON_PATCH_VERSIONS] = {
 /* 10 */ {on_10,         on_10,         on_10,         on_10,          on_10,       on_10,       on_10,          on_10,          on_10,          on_10,          on_10,          on_10},
 /* 11 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* 12 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
-/* 13 */ {on_ignored,    on_ignored,    on_ignored,    on_ignored,     on_ignored,  on_ignored,  on_13_A7_V3_BB, on_13_A7_V3_BB, on_13_A7_V3_BB, on_13_A7_V3_BB, on_13_A7_V3_BB, on_13_A7_V3_BB},
+/* 13 */ {on_ignored,    on_ignored,    on_ignored,    on_ignored,     on_ignored,  on_ignored,  on_13_A7_GC,    on_13_A7_GC,    on_13_A7_GC,    on_13_A7_GC,    on_ignored,     on_ignored},
 /* 14 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* 15 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* 16 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
@@ -5374,7 +5367,7 @@ static on_command_t handlers[0x100][NUM_NON_PATCH_VERSIONS] = {
 /* 41 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* 42 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* 43 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
-/* 44 */ {on_ignored,    on_ignored,    on_ignored,    on_ignored,     on_ignored,  on_ignored,  on_44_A6_V3_BB, on_44_A6_V3_BB, on_44_A6_V3_BB, on_44_A6_V3_BB, on_44_A6_V3_BB, on_44_A6_V3_BB},
+/* 44 */ {on_ignored,    on_ignored,    on_ignored,    on_ignored,     on_ignored,  on_ignored,  on_ignored,     on_ignored,     on_ignored,     on_ignored,     on_ignored,     on_ignored},
 /* 45 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* 46 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* 47 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
@@ -5478,8 +5471,8 @@ static on_command_t handlers[0x100][NUM_NON_PATCH_VERSIONS] = {
 /* A3 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* A4 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* A5 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
-/* A6 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     on_44_A6_V3_BB, on_44_A6_V3_BB, on_44_A6_V3_BB, on_44_A6_V3_BB, on_44_A6_V3_BB, nullptr},
-/* A7 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     on_13_A7_V3_BB, on_13_A7_V3_BB, on_13_A7_V3_BB, on_13_A7_V3_BB, on_13_A7_V3_BB, nullptr},
+/* A6 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     on_ignored,     on_ignored,     on_ignored,     on_ignored,     on_ignored,     nullptr},
+/* A7 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     on_13_A7_GC,    on_13_A7_GC,    on_13_A7_GC,    on_13_A7_GC,    on_ignored,     nullptr},
 /* A8 */ {nullptr,       nullptr,       nullptr,       nullptr,        nullptr,     nullptr,     nullptr,        nullptr,        nullptr,        nullptr,        nullptr,        nullptr},
 /* A9 */ {on_A9,         on_A9,         on_A9,         on_A9,          on_A9,       on_A9,       on_A9,          on_A9,          on_A9,          on_A9,          on_A9,          on_A9},
 /* AA */ {nullptr,       nullptr,       nullptr,       nullptr,        on_AA,       on_AA,       on_AA,          on_AA,          on_AA,          on_AA,          on_AA,          on_AA},
diff --git a/src/SendCommands.cc b/src/SendCommands.cc
index b04e295f..ad6b4fe8 100644
--- a/src/SendCommands.cc
+++ b/src/SendCommands.cc
@@ -3852,18 +3852,27 @@ void send_open_quest_file(
       throw logic_error("cannot send quest files to this version of client");
   }
 
-  // For GC/XB/BB, we wait for acknowledgement commands before sending each
-  // chunk. For DC/PC, we send the entire quest all at once.
-  if (is_v1_or_v2(c->version()) && (c->version() != Version::GC_NTE)) {
-    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, filename.c_str(), offset / 0x400,
-          contents->data() + offset, chunk_bytes, (type != QuestFileType::ONLINE));
+  // On most versions, we can trust the TCP stack to do the right thing when we
+  // send a lot of data at once, but on GC, the client will crash if too much
+  // quest data is sent at once. This is likely a bug in the TCP stack, since
+  // the client should apply backpressure to avoid bad situations, but we have
+  // to deal with it here instead.
+  size_t total_chunks = (contents->size() + 0x3FF) / 0x400;
+  size_t chunks_to_send = is_gc(c->version()) ? min<size_t>(GC_QUEST_LOAD_MAX_CHUNKS_IN_FLIGHT, total_chunks) : total_chunks;
+
+  for (size_t z = 0; z < chunks_to_send; z++) {
+    size_t offset = z * 0x400;
+    size_t chunk_bytes = contents->size() - offset;
+    if (chunk_bytes > 0x400) {
+      chunk_bytes = 0x400;
     }
-  } else {
+    send_quest_file_chunk(c, filename.c_str(), offset / 0x400,
+        contents->data() + offset, chunk_bytes, (type != QuestFileType::ONLINE));
+  }
+
+  // If there are still chunks to send, track the file so the chunk
+  // acknowledgement handler (13 or A7) cna know what to send next
+  if (chunks_to_send < total_chunks) {
     c->sending_files.emplace(filename, contents);
     c->log.info("Opened file %s", filename.c_str());
   }
diff --git a/src/SendCommands.hh b/src/SendCommands.hh
index 63ab429d..def92060 100644
--- a/src/SendCommands.hh
+++ b/src/SendCommands.hh
@@ -21,6 +21,8 @@ extern const std::unordered_set<uint32_t> v2_crypt_initial_client_commands;
 extern const std::unordered_set<uint32_t> v3_crypt_initial_client_commands;
 extern const std::unordered_set<std::string> bb_crypt_initial_client_commands;
 
+constexpr size_t GC_QUEST_LOAD_MAX_CHUNKS_IN_FLIGHT = 4;
+
 // TODO: Many of these functions should take a Channel& instead of a
 // shared_ptr<Client>. Refactor functions appropriately.