reformat more files

2025-12-20 20:44:32 -08:00
parent a9fa138213
commit a462a774f5
21 changed files with 447 additions and 669 deletions
@@ -67,8 +67,8 @@ string AFSArchive::generate_t(const vector<string>& files) {
  w.put_u32b(0x41465300); // 'AFS\0'
  w.put<U32T<BE>>(files.size());
-  // It seems entries are aligned to 0x800-byte boundaries, and the file's
+  // It seems entries are aligned to 0x800-byte boundaries, and the file's header is always 0x80000 (!) bytes, most of
-  // header is always 0x80000 (!) bytes, most of which is unused
+  // which is unused
  uint32_t data_offset = 0x80000;
  for (const auto& file : files) {
    w.put<U32T<BE>>(data_offset);
@@ -32,10 +32,7 @@ shared_ptr<DCNTELicense> DCNTELicense::from_json(const phosg::JSON& json) {
 }
 phosg::JSON DCNTELicense::json() const {
-  return phosg::JSON::dict({
+  return phosg::JSON::dict({{"SerialNumber", this->serial_number}, {"AccessKey", this->access_key}});
      {"SerialNumber", this->serial_number},
      {"AccessKey", this->access_key},
  });
 }
 shared_ptr<V1V2License> V1V2License::from_json(const phosg::JSON& json) {
@@ -52,10 +49,7 @@ shared_ptr<V1V2License> V1V2License::from_json(const phosg::JSON& json) {
 }
 phosg::JSON V1V2License::json() const {
-  return phosg::JSON::dict({
+  return phosg::JSON::dict({{"SerialNumber", this->serial_number}, {"AccessKey", this->access_key}});
      {"SerialNumber", this->serial_number},
      {"AccessKey", this->access_key},
  });
 }
 shared_ptr<GCLicense> GCLicense::from_json(const phosg::JSON& json) {
@@ -101,11 +95,7 @@ shared_ptr<XBLicense> XBLicense::from_json(const phosg::JSON& json) {
 }
 phosg::JSON XBLicense::json() const {
-  return phosg::JSON::dict({
+  return phosg::JSON::dict({{"GamerTag", this->gamertag}, {"UserID", this->user_id}, {"AccountID", this->account_id}});
      {"GamerTag", this->gamertag},
      {"UserID", this->user_id},
      {"AccountID", this->account_id},
  });
 }
 shared_ptr<BBLicense> BBLicense::from_json(const phosg::JSON& json) {
@@ -128,10 +118,7 @@ shared_ptr<BBLicense> BBLicense::from_json(const phosg::JSON& json) {
 }
 phosg::JSON BBLicense::json() const {
-  return phosg::JSON::dict({
+  return phosg::JSON::dict({{"UserName", this->username}, {"Password", this->password}});
      {"UserName", this->username},
      {"Password", this->password},
  });
 }
 Account::Account(const phosg::JSON& json)
@@ -412,7 +399,8 @@ string Account::str() const {
 void Account::save() const {
  if (!this->is_temporary) {
    auto json = this->json();
-    string json_data = json.serialize(phosg::JSON::SerializeOption::FORMAT | phosg::JSON::SerializeOption::HEX_INTEGERS);
+    string json_data = json.serialize(
        phosg::JSON::SerializeOption::FORMAT | phosg::JSON::SerializeOption::HEX_INTEGERS);
    string filename = std::format("system/licenses/{:010}.json", this->account_id);
    phosg::save_file(filename, json_data);
  }
@@ -656,7 +644,11 @@ shared_ptr<Login> AccountIndex::from_gc_credentials_locked(
 }
 shared_ptr<Login> AccountIndex::from_gc_credentials(
-    uint32_t serial_number, const string& access_key, const string* password, const string& character_name, bool allow_create) {
+    uint32_t serial_number,
    const string& access_key,
    const string* password,
    const string& character_name,
    bool allow_create) {
  if (serial_number == 0) {
    throw no_username();
  }
@@ -750,7 +742,8 @@ shared_ptr<Login> AccountIndex::from_bb_credentials_locked(const string& usernam
  return login;
 }
-shared_ptr<Login> AccountIndex::from_bb_credentials(const string& username, const string* password, bool allow_create) {
+shared_ptr<Login> AccountIndex::from_bb_credentials(
    const string& username, const string* password, bool allow_create) {
  if (username.empty() || (password && password->empty())) {
    throw no_username();
  }
@@ -72,8 +72,7 @@ struct Account {
    ADMINISTRATOR               = 0x000000FF,
    ROOT                        = 0x7FFFFFFF,
    IS_SHARED_ACCOUNT           = 0x80000000,
-    // NOTE: When adding or changing license flags, don't forget to change the
+    // NOTE: When adding or changing license flags, don't forget to change the documentation in the shell's help text.
    // documentation in the shell's help text.
    UNUSED_BITS                 = 0x70FFFF00,
    // clang-format on
  };
@@ -149,8 +148,7 @@ struct Login {
  bool account_was_created = false;
  // This field will never be null
  std::shared_ptr<Account> account;
-  // Exactly one of the following will be non-null, representing the license
+  // Exactly one of the following will be non-null, representing the license that the client logged in with
  // that the client logged in with
  std::shared_ptr<DCNTELicense> dc_nte_license;
  std::shared_ptr<V1V2License> dc_license;
  std::shared_ptr<V1V2License> pc_license;
@@ -210,22 +208,12 @@ public:
  std::shared_ptr<Account> from_account_id(uint32_t account_id) const;
  std::shared_ptr<Login> from_dc_nte_credentials(
-      const std::string& serial_number,
+      const std::string& serial_number, const std::string& access_key, bool allow_create);
      const std::string& access_key,
      bool allow_create);
  std::shared_ptr<Login> from_dc_credentials(
-      uint32_t serial_number,
+      uint32_t serial_number, const std::string& access_key, const std::string& character_name, bool allow_create);
-      const std::string& access_key,
+  std::shared_ptr<Login> from_pc_nte_credentials(uint32_t guild_card_number, bool allow_create);
      const std::string& character_name,
      bool allow_create);
  std::shared_ptr<Login> from_pc_nte_credentials(
      uint32_t guild_card_number,
      bool allow_create);
  std::shared_ptr<Login> from_pc_credentials(
-      uint32_t serial_number,
+      uint32_t serial_number, const std::string& access_key, const std::string& character_name, bool allow_create);
      const std::string& access_key,
      const std::string& character_name,
      bool allow_create);
  std::shared_ptr<Login> from_gc_credentials(
      uint32_t serial_number,
      const std::string& access_key,
@@ -233,14 +221,9 @@ public:
      const std::string& character_name,
      bool allow_create);
  std::shared_ptr<Login> from_xb_credentials(
-      const std::string& gamertag,
+      const std::string& gamertag, uint64_t user_id, uint64_t account_id, bool allow_create);
      uint64_t user_id,
      uint64_t account_id,
      bool allow_create);
  std::shared_ptr<Login> from_bb_credentials(
-      const std::string& username,
+      const std::string& username, const std::string* password, bool allow_create);
      const std::string* password,
      bool allow_create);
  std::shared_ptr<Account> create_temporary_account_for_shared_account(
      std::shared_ptr<const Account> src_a, const std::string& variation_data) const;
@@ -248,8 +231,6 @@ public:
 protected:
  bool force_all_temporary;
  // This class must be thread-safe because it's used by both the patch server
  // and game server threads
  mutable std::shared_mutex lock;
  std::unordered_map<uint32_t, std::shared_ptr<Account>> by_account_id;
  std::unordered_map<std::string, std::shared_ptr<Account>> by_dc_nte_serial_number;
@@ -262,23 +243,16 @@ protected:
  void add_locked(std::shared_ptr<Account> a);
  std::shared_ptr<Login> from_dc_nte_credentials_locked(
-      const std::string& serial_number,
+      const std::string& serial_number, const std::string& access_key);
      const std::string& access_key);
  std::shared_ptr<Login> from_dc_credentials_locked(
-      uint32_t serial_number,
+      uint32_t serial_number, const std::string& access_key, const std::string& character_name);
      const std::string& access_key,
      const std::string& character_name);
  std::shared_ptr<Login> from_pc_credentials_locked(
-      uint32_t serial_number,
+      uint32_t serial_number, const std::string& access_key, const std::string& character_name);
      const std::string& access_key,
      const std::string& character_name);
  std::shared_ptr<Login> from_gc_credentials_locked(
      uint32_t serial_number,
      const std::string& access_key,
      const std::string* password,
      const std::string& character_name);
  std::shared_ptr<Login> from_xb_credentials_locked(uint64_t user_id);
-  std::shared_ptr<Login> from_bb_credentials_locked(
+  std::shared_ptr<Login> from_bb_credentials_locked(const std::string& username, const std::string* password);
      const std::string& username,
      const std::string* password);
 };
@@ -267,20 +267,16 @@ public:
  // Returns {type: {constructor_addr: [(start_area, end_area), ...]}}
  template <typename EntryT>
-  map<uint32_t, map<uint32_t, vector<pair<size_t, size_t>>>>
+  map<uint32_t, map<uint32_t, vector<pair<size_t, size_t>>>> parse_dat_constructor_table_t(
-  parse_dat_constructor_table_t(
+      shared_ptr<const ResourceDASM::MemoryContext>& mem, const ParseDATConstructorTableSpec& spec) {
      shared_ptr<const ResourceDASM::MemoryContext>& mem,
      const ParseDATConstructorTableSpec& spec) {
    if (!mem) {
      throw runtime_error("no file selected");
    }
-    // On some of the x86 builds of the game (PCv2 and Xbox), the constructor
+    // On some of the x86 builds of the game (PCv2 and Xbox), the constructor tables aren't entirely static in the data
-    // tables aren't entirely static in the data sections - some parts are
+    // sections - some parts are written during static initialization instead. To handle this, we make a copy of the
-    // written during static initialization instead. To handle this, we make a
+    // immutable MemoryContext and run the static initialization functions using resource_dasm's emulator before
-    // copy of the immutable MemoryContext and run the static initialization
+    // parsing the constructor table.
    // functions using resource_dasm's emulator before parsing the constructor
    // table.
    shared_ptr<const ResourceDASM::MemoryContext> effective_mem = mem;
    if (!spec.x86_constructor_calls.empty()) {
      auto constructed_mem = make_shared<ResourceDASM::MemoryContext>(mem->duplicate());
@@ -455,9 +451,7 @@ public:
        }
      }
      line.push_back(' ');
-      line += is_enemies
+      line += is_enemies ? MapFile::name_for_enemy_type(type) : MapFile::name_for_object_type(type);
          ? MapFile::name_for_enemy_type(type)
          : MapFile::name_for_object_type(type);
      if ((formatted_lines.size() % 40) == 0) {
        formatted_lines.emplace_back(header_line);
@@ -732,9 +726,7 @@ public:
  }
  uint32_t find_be_to_le_data_match(
-      shared_ptr<const ResourceDASM::MemoryContext> dest_mem,
+      shared_ptr<const ResourceDASM::MemoryContext> dest_mem, uint32_t src_addr, uint32_t src_size) const {
      uint32_t src_addr,
      uint32_t src_size) const {
    if (src_size == 0) {
      src_size = 4;
    }
@@ -324,8 +324,8 @@ asio::awaitable<WebSocketMessage> HTTPClient::recv_websocket_message(size_t max_
    this->last_communication_time = phosg::now();
-    // If the current message is a control message, respond appropriately
+    // If the current message is a control message, respond appropriately (these can be sent in the middle of
-    // (these can be sent in the middle of fragmented messages)
+    // fragmented messages)
    uint8_t opcode = msg.header[0] & 0x0F;
    if (opcode & 0x08) {
      if (opcode == 0x0A) {
@@ -347,8 +347,8 @@ asio::awaitable<WebSocketMessage> HTTPClient::recv_websocket_message(size_t max_
      continue;
    }
-    // If there's an existing fragment, the current message's opcode should be
+    // If there's an existing fragment, the current message's opcode should be zero; if there's no pending message, it
-    // zero; if there's no pending message, it must not be zero
+    // must not be zero
    if (prev_msg_present == (opcode != 0)) {
      this->r.close();
      continue;
@@ -372,10 +372,9 @@ asio::awaitable<WebSocketMessage> HTTPClient::recv_websocket_message(size_t max_
      prev_msg.data += msg.data;
    }
-    // If the FIN bit is set, then the frame is complete - append the payload
+    // If the FIN bit is set, then the frame is complete - append the payload to any pending payloads and call the
-    // to any pending payloads and call the message handler. If the FIN bit
+    // message handler. If the FIN bit isn't set, we need to receive at least one continuation frame to complete the
-    // isn't set, we need to receive at least one continuation frame to
+    // message.
    // complete the message.
    if (prev_msg.header[0] & 0x80) {
      co_return prev_msg;
    }
@@ -38,9 +38,8 @@ struct HTTPRequest {
  std::unordered_multimap<std::string, std::string> query_params;
  std::string data;
-  // Header name should be entirely lowercase for this function. Returns
+  // Header name should be entirely lowercase for this function. Returns nullptr if the header doesn't exist; throws
-  // nullptr if the header doesn't exist; throws http_error(400) if multiple
+  // http_error(400) if multiple instances of it exist.
  // instances of it exist.
  const std::string* get_header(const std::string& name) const;
  const std::string* get_query_param(const std::string& name) const;
@@ -49,8 +48,7 @@ struct HTTPRequest {
 struct HTTPResponse {
  std::string http_version;
  int response_code = 200;
-  // Content-Length should NOT be specified in headers; it is automatically
+  // Content-Length should NOT be specified in headers; it is automatically added in async_write() if data isn't blank.
  // added in async_write() if data is not blank.
  std::unordered_multimap<std::string, std::string> headers;
  std::string data;
 };
@@ -244,9 +242,8 @@ protected:
    }
  }
-  // Attempts to switch the client to WebSockets. Returns true if this is done
+  // Attempts to switch the client to WebSockets. Returns true if this is done successfully (and the caller should then
-  // successfully (and the caller should then receive/send WebSocket messages),
+  // receive/send WebSocket messages), or false if this failed (and the caller should send an HTTP response).
  // or false if this failed (and the caller should send an HTTP response).
  asio::awaitable<bool> enable_websockets(std::shared_ptr<ClientT> c, const HTTPRequest& req) {
    if (req.method != HTTPRequest::Method::GET) {
      co_return false;
@@ -287,13 +284,10 @@ protected:
  // handle_request must do one of the following three things:
  // 1. Return an HTTP response.
-  // 2. Call enable_websockets, and if it returns true, return nullptr. After
+  // 2. Call enable_websockets, and if it returns true, return nullptr. After this point, handle_request will not be
-  //    this point, handle_request will not be called again for this client;
+  //    called again for this client; handle_websocket_message will be called instead when any WebSocket messages are
-  //    handle_websocket_message will be called instead when any WebSocket
+  //    received. If enable_websockets returns false, handle_request must still return an HTTP response.
-  //    messages are received. If enable_websockets returns false,
+  // 3. Throw an exception. In this case, the client receives an HTTP 500 response.
  //    handle_request must still return an HTTP response.
  // 3. Throw an exception. In this case, the client receives an HTTP 500
  //    response.
  virtual asio::awaitable<std::unique_ptr<HTTPResponse>> handle_request(std::shared_ptr<ClientT> c, HTTPRequest&& req) = 0;
  virtual asio::awaitable<void> handle_websocket_message(std::shared_ptr<ClientT>, WebSocketMessage&&) {
    co_return;
@@ -73,8 +73,7 @@ asio::awaitable<string> AsyncSocketReader::read_line(const char* delimiter, size
    throw runtime_error("line exceeds max length");
  }
-  // TODO: It's not great that we copy the data here. There's probably a more
+  // TODO: It's not great that we copy the data here. There's probably a more idiomatic and efficient way to do this.
  // idiomatic and efficient way to do this.
  string ret = this->pending_data.substr(0, delimiter_pos);
  this->pending_data = this->pending_data.substr(delimiter_pos + delimiter_size);
  co_return ret;
@@ -175,16 +175,15 @@ public:
  AsyncSocketReader& operator=(AsyncSocketReader&&) = delete;
  ~AsyncSocketReader() = default;
-  // Reads one line from the socket, buffering any extra data read. The
+  // Reads one line from the socket, buffering any extra data read. The delimiter is not included in the returned line.
-  // delimiter is not included in the returned line. max_length = 0 means no
+  // max_length = 0 means no maximum length is enforced.
  // maximum length is enforced.
  asio::awaitable<std::string> read_line(
      const char* delimiter = "\n", size_t max_length = 0);
  asio::awaitable<std::string> read_data(size_t size);
  asio::awaitable<void> read_data_into(void* data, size_t size);
-  // The caller cannot know what the socket's read state is, so this should
+  // The caller cannot know what the socket's read state is, so this should only be used when the caller intends to
-  // only be used when the caller intends to write to the socket, not read
+  // write to the socket, not read
  inline asio::ip::tcp::socket& get_socket() {
    return this->sock;
  }
@@ -215,8 +214,8 @@ public:
  void add(std::string&& data);
-  // When using add_reference, it is the caller's responsibility to ensure that
+  // When using add_reference, it is the caller's responsibility to ensure that the buffer is valid until *this is
-  // the buffer is valid until *this is destroyed or write() returns.
+  // destroyed or write() returns.
  void add_reference(const void* data, size_t size);
  asio::awaitable<void> write(asio::ip::tcp::socket& sock);
@@ -260,9 +259,8 @@ asio::awaitable<std::invoke_result_t<FnT, ArgTs...>> call_on_thread_pool(asio::t
  using ReturnT = std::invoke_result_t<FnT, ArgTs...>;
  auto bound = std::bind(std::forward<FnT>(f), std::forward<ArgTs>(args)...);
-  // We have to use a shared_ptr here in case call_on_thread_pool is canceled
+  // We have to use a shared_ptr here in case call_on_thread_pool is canceled (in that case, the posted callback will
-  // (in that case, the posted callback will try to use promise after the
+  // try to use promise after the call_on_thread_pool coroutine has been destroyed)
  // call_on_thread_pool coroutine has been destroyed)
  auto promise = std::make_shared<AsyncPromise<ReturnT>>();
  asio::post(pool, [bound = std::move(bound), promise]() mutable {
    try {
@@ -15,7 +15,6 @@ struct BMLHeaderT {
  U32T<BE> num_entries;
  parray<uint8_t, 0x38> unknown_a2;
 } __attribute__((packed));
 using BMLHeader = BMLHeaderT<false>;
 using BMLHeaderBE = BMLHeaderT<true>;
 check_struct_size(BMLHeader, 0x40);
@@ -31,7 +30,6 @@ struct BMLHeaderEntryT {
  U32T<BE> decompressed_gvm_size;
  parray<uint8_t, 0x0C> unknown_a2;
 } __attribute__((packed));
 using BMLHeaderEntry = BMLHeaderEntryT<false>;
 using BMLHeaderEntryBE = BMLHeaderEntryT<true>;
 check_struct_size(BMLHeaderEntry, 0x40);
@@ -32,7 +32,8 @@ void Channel::send(uint16_t cmd, uint32_t flag, bool silent) {
  this->send(cmd, flag, nullptr, 0, silent);
 }
-void Channel::send(uint16_t cmd, uint32_t flag, const std::vector<std::pair<const void*, size_t>> blocks, bool silent) {
+void Channel::send(
    uint16_t cmd, uint32_t flag, const std::vector<std::pair<const void*, size_t>> blocks, bool silent) {
  if (!this->connected()) {
    channel_exceptions_log.warning_f("Attempted to send command on closed channel; dropping data");
    return;
@@ -57,10 +58,7 @@ void Channel::send(uint16_t cmd, uint32_t flag, const std::vector<std::pair<cons
    case Version::GC_EP3:
    case Version::XB_V3: {
      PSOCommandHeaderDCV3 header;
-      if (this->crypt_out.get() &&
+      if (this->crypt_out.get() && !is_v1(this->version)) {
          (this->version != Version::DC_NTE) &&
          (this->version != Version::DC_11_2000) &&
          (this->version != Version::DC_V1)) {
        send_data_size = (sizeof(header) + size + 3) & ~3;
      } else {
        send_data_size = (sizeof(header) + size);
@@ -90,13 +88,11 @@ void Channel::send(uint16_t cmd, uint32_t flag, const std::vector<std::pair<cons
      break;
    }
    case Version::BB_V4: {
-      // BB has an annoying behavior here: command lengths must be multiples of
+      // BB has an annoying behavior here: command lengths must be multiples of 4, but the actual data length must be a
-      // 4, but the actual data length must be a multiple of 8. If the size
+      // multiple of 8. If the size field is not divisible by 8, 4 extra bytes are sent anyway. This behavior only
-      // field is not divisible by 8, 4 extra bytes are sent anyway. This
+      // applies when encryption is enabled - any commands sent before encryption is enabled have no size restrictions
-      // behavior only applies when encryption is enabled - any commands sent
+      // (except they must include a full header and must fit in the client's receive buffer), and no implicit extra
-      // before encryption is enabled have no size restrictions (except they
+      // bytes are sent.
      // must include a full header and must fit in the client's receive
      // buffer), and no implicit extra bytes are sent.
      PSOCommandHeaderBB header;
      if (this->crypt_out.get()) {
        send_data_size = (sizeof(header) + size + 7) & ~7;
@@ -115,8 +111,7 @@ void Channel::send(uint16_t cmd, uint32_t flag, const std::vector<std::pair<cons
      throw logic_error("unimplemented game version in send_command");
  }
-  // All versions of PSO I've seen (so far) have a receive buffer 0x7C00
+  // All versions of PSO I've seen (so far) have a receive buffer 0x7C00 bytes in size
  // bytes in size
  if (send_data_size > 0x7C00) {
    throw runtime_error("outbound command too large");
  }
@@ -132,8 +127,7 @@ void Channel::send(uint16_t cmd, uint32_t flag, const std::vector<std::pair<cons
      print_color_escape(stderr, phosg::TerminalFormat::FG_YELLOW, phosg::TerminalFormat::BOLD, phosg::TerminalFormat::END);
    }
    if (version == Version::BB_V4) {
-      command_data_log.info_f("Sending to {} (version=BB command={:04X} flag={:08X})",
+      command_data_log.info_f("Sending to {} (version=BB command={:04X} flag={:08X})", this->name, cmd, flag);
          this->name, cmd, flag);
    } else {
      command_data_log.info_f("Sending to {} (version={} command={:02X} flag={:02X})",
          this->name, phosg::name_for_enum(version), cmd, flag);
@@ -187,9 +181,8 @@ asio::awaitable<Channel::Message> Channel::recv() {
    throw runtime_error("header size field is smaller than header");
  }
-  // If encryption is enabled, BB pads commands to 8-byte boundaries, and this
+  // If encryption is enabled, BB pads commands to 8-byte boundaries, and this is not reflected in the size field. This
-  // is not reflected in the size field. This logic does not occur if encryption
+  // logic does not occur if encryption is not yet enabled.
  // is not yet enabled.
  size_t command_physical_size = (this->crypt_in.get() && (version == Version::BB_V4))
      ? ((command_logical_size + 7) & ~7)
      : command_logical_size;
@@ -198,10 +191,9 @@ asio::awaitable<Channel::Message> Channel::recv() {
  co_await this->recv_raw(command_data.data(), command_data.size());
  if (this->crypt_in.get()) {
-    // Some versions of PSO DC can send commands whose sizes are not a multiple
+    // Some versions of PSO DC can send commands whose sizes are not a multiple of 4, but the server is expected to
-    // of 4, but the server is expected to always use a multiple of 4 bytes when
+    // always use a multiple of 4 bytes when decrypting (the extra cipher bytes are lost). To emulate this behavior, we
-    // decrypting (the extra cipher bytes are lost). To emulate this behavior,
+    // have to round up the size for DC commands here.
    // we have to round up the size for DC commands here.
    size_t orig_size = command_data.size();
    command_data.resize((orig_size + 3) & (~3), 0);
    this->crypt_in->decrypt(command_data.data(), command_data.size());
@@ -60,9 +60,8 @@ public:
  // Returns whether the channel is connected or not.
  virtual bool connected() const = 0;
-  // Disconnects the channel. Any pending data will still be sent before the
+  // Disconnects the channel. Any pending data will still be sent before the underlying transport (e.g. socket) is
-  // underlying transport (e.g. socket) is closed, but further send calls will
+  // closed, but further send calls will do nothing.
  // do nothing.
  virtual void disconnect() = 0;
  // Sends a message with an automatically-constructed header.
@@ -76,8 +75,7 @@ public:
    this->send(cmd, flag, &data, sizeof(data), silent);
  }
-  // Sends a message with a pre-existing header (as the first few bytes in the
+  // Sends a message with a pre-existing header (as the first few bytes in the data)
  // data)
  void send(const void* data, size_t size, bool silent = false);
  void send(const std::string& data, bool silent = false);
@@ -96,27 +94,22 @@ protected:
  Channel& operator=(const Channel& other) = delete;
  Channel& operator=(Channel&& other) = delete;
-  // Sends raw data on the underlying transport. If the channel is already
+  // Sends raw data on the underlying transport. If the channel is already disconnected, silently drops the data.
  // disconnected, silently drops the data.
  virtual void send_raw(std::string&& data) = 0;
-  // Receives raw data on the underlying transport. Raises when the channel is
+  // Receives raw data on the underlying transport. Raises when the channel is disconnected.
  // disconnected.
  virtual asio::awaitable<void> recv_raw(void* data, size_t size) = 0;
 };
-// Standard channel type, used for most PSO clients. Represents an open TCP
+// Standard channel type, used for most PSO clients. Represents an open TCP socket.
 // socket.
 class SocketChannel : public Channel, public std::enable_shared_from_this<SocketChannel> {
 public:
  std::unique_ptr<asio::ip::tcp::socket> sock;
  asio::ip::tcp::endpoint local_addr;
  asio::ip::tcp::endpoint remote_addr;
-  // SocketChannel has a static constructor because it has an internal task,
+  // SocketChannel has a static constructor because it has an internal task, which is necessary to support flushing
-  // which is necessary to support flushing before disconnection (for example)
+  // before disconnection (for example) and also to make send_raw not a coroutine, which keeps the rest of the code
-  // and also to make send_raw not a coroutine, which keeps the rest of the
+  // cleaner.
  // code cleaner. The task needs to hold a shared_ptr to the SocketChannel
  // whilc it's open
  static std::shared_ptr<SocketChannel> create(std::shared_ptr<asio::io_context> io_context,
      std::unique_ptr<asio::ip::tcp::socket>&& sock,
      Version version,
@@ -25,7 +25,7 @@
 using namespace std;
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // Tools
 string str_for_flag_ranges(const vector<bool>& flags) {
@@ -60,7 +60,7 @@ string str_for_flag_ranges(const vector<bool>& flags) {
  return ret;
 }
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // Checks
 class precondition_failed {
@@ -165,7 +165,7 @@ struct Args {
  }
 };
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // Command definitions
 struct ChatCommandDefinition {
@@ -188,7 +188,7 @@ struct ChatCommandDefinition {
 unordered_map<string, const ChatCommandDefinition*> ChatCommandDefinition::all_defs;
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // All commands (in alphabetical order)
 ChatCommandDefinition cc_allevent(
@@ -265,8 +265,8 @@ ChatCommandDefinition cc_announce_rares(
      a.c->login->account->toggle_user_flag(Account::UserFlag::DISABLE_DROP_NOTIFICATION_BROADCAST);
      a.c->login->account->save();
-      send_text_message_fmt(a.c, "$C6Rare announcements\n{} for your\nitems",
+      bool enabled = a.c->login->account->check_user_flag(Account::UserFlag::DISABLE_DROP_NOTIFICATION_BROADCAST);
-          a.c->login->account->check_user_flag(Account::UserFlag::DISABLE_DROP_NOTIFICATION_BROADCAST) ? "disabled" : "enabled");
+      send_text_message_fmt(a.c, "$C6Rare announcements\n{} for your\nitems", enabled ? "disabled" : "enabled");
      co_return;
    });
@@ -370,9 +370,8 @@ ChatCommandDefinition cc_ban(
        throw precondition_failed("$C6You do not have\nsufficient privileges.");
      }
      if (a.c == target) {
-        // This shouldn't be possible because you need BAN_USER to get here,
+        // This shouldn't be possible because you need BAN_USER to get here, but the target can't have BAN_USER if we
-        // but the target can't have BAN_USER if we get here, so if a.c and
+        // get here, so if a.c and target are the same, one of the preceding conditions must be false.
        // target are the same, one of the preceding conditions must be false.
        throw logic_error("client attempts to ban themself");
      }
@@ -482,8 +481,7 @@ static asio::awaitable<void> server_command_bbchar_savechar(const Args& a, bool
    dest_account = a.c->login->account;
  }
-  // If the client isn't BB, request the player info. (If they are BB, the
+  // If the client isn't BB, request the player info. (If they are BB, the server already has it)
  // server already has it)
  GetPlayerInfoResult ch;
  if (a.c->version() == Version::BB_V4) {
    ch.character = a.c->character_file();
@@ -516,8 +514,7 @@ static asio::awaitable<void> server_command_bbchar_savechar(const Args& a, bool
    }
  } else {
-    // Client sent 61; generate a BB-format player from the information we have
+    // Client sent 61; generate a BB-format player from the information we have and save that instead
    // and save that instead
    if (ch.character) {
      auto bb_player = PSOBBCharacterFile::create_from_config(
          a.c->login->account->account_id,
@@ -528,9 +525,8 @@ static asio::awaitable<void> server_command_bbchar_savechar(const Args& a, bool
      bb_player->disp.visual.version = 4;
      bb_player->disp.visual.name_color_checksum = 0x00000000;
      bb_player->inventory = ch.character->inventory;
-      // Before V3, player stats can't be correctly computed from other fields
+      // Before V3, player stats can't be correctly computed from other fields because material usage isn't stored
-      // because material usage isn't stored anywhere. For these versions, we
+      // anywhere. For these versions, we have to trust the stats field from the player's data.
      // have to trust the stats field from the player's data.
      auto level_table = s->level_table(a.c->version());
      if (is_v1_or_v2(a.c->version())) {
        bb_player->disp.stats = ch.character->disp.stats;
@@ -1074,9 +1070,7 @@ ChatCommandDefinition cc_event(
 ChatCommandDefinition cc_exit(
    {"$exit"},
    +[](const Args& a) -> asio::awaitable<void> {
-      if (!(a.c->proxy_session
+      if (!(a.c->proxy_session ? a.c->proxy_session->is_in_game : a.c->require_lobby()->is_game())) {
                  ? a.c->proxy_session->is_in_game
                  : a.c->require_lobby()->is_game())) {
        // Client is in the lobby; send them to the login server (main menu)
        if (a.c->proxy_session) {
          if (is_v4(a.c->version())) {
@@ -1181,8 +1175,6 @@ ChatCommandDefinition cc_infhp(
 ChatCommandDefinition cc_inftime(
    {"$inftime"},
    +[](const Args& a) -> asio::awaitable<void> {
      // TODO: We could implement this in proxy sessions by rewriting the rules
      // struct from the server in various 6xB4 commands.
      a.check_is_proxy(false);
      a.check_is_game(true);
      a.check_is_ep3(true);
@@ -1301,9 +1293,8 @@ ChatCommandDefinition cc_kick(
        throw precondition_failed("$C6You do not have\nsufficient privileges.");
      }
      if (a.c == target) {
-        // This shouldn't be possible because you need KICK_USER to get here,
+        // This shouldn't be possible because you need KICK_USER to get here, but the target can't have KICK_USER if we
-        // but the target can't have KICK_USER if we get here, so if a.c and
+        // get here, so if a.c and target are the same, one of the preceding conditions must be false.
        // target are the same, one of the preceding conditions must be false.
        throw logic_error("client attempts to kick themself off");
      }
@@ -1332,13 +1323,10 @@ ChatCommandDefinition cc_killcount(
        throw precondition_failed("No equipped items\nhave kill counts");
      } else {
-        // Kill counts are only accurate on the server side at all times on BB.
+        // Kill counts are only accurate on the server side at all times on BB. On other versions, we update the
-        // On other versions, we update the server's view of the client's
+        // server's view of the client's inventory during games, but we can't track kills because the client doesn't
-        // inventory during games, but we can't track kills because the client
+        // inform the server whether it counted a kill for any individual enemy. So, on non-BB versions, the kill count
-        // doesn't inform the server whether it counted a kill for any
+        // is accurate at all times in the lobby (since kills can't occur there), or at the beginning of a game.
        // individual enemy. So, on non-BB versions, the kill count is accurate
        // at all times in the lobby (since kills can't occur there), or at the
        // beginning of a game.
        if ((a.c->version() == Version::BB_V4) || !a.c->require_lobby()->is_game()) {
          send_text_message(a.c, "As of now:");
        } else {
@@ -1361,9 +1349,8 @@ ChatCommandDefinition cc_lobby_info(
    +[](const Args& a) -> asio::awaitable<void> {
      if (a.c->proxy_session) {
        string msg;
-        // On non-masked-GC sessions (BB), there is no remote Guild Card number, so we
+        // On non-masked-GC sessions (BB), there is no remote Guild Card number, so we don't show it. (The user can see
-        // don't show it. (The user can see it in the pause menu, unlike in masked-GC
+        // it in the pause menu, unlike in masked-GC sessions like GC.)
        // sessions like GC.)
        if (a.c->proxy_session->remote_guild_card_number >= 0) {
          msg = std::format("$C7GC: $C6{}$C7\n", a.c->proxy_session->remote_guild_card_number);
        }
@@ -1612,8 +1599,8 @@ ChatCommandDefinition cc_loadchar(
        }
      } else {
-        // On v1 and v2, the client will assign its character data from the lobby
+        // On v1 and v2, the client will assign its character data from the lobby join command, so it suffices to just
-        // join command, so it suffices to just resend the join notification.
+        // resend the join notification.
        auto s = a.c->require_server_state();
        send_player_leave_notification(l, a.c->lobby_client_id);
        s->send_lobby_join_notifications(l, a.c);
@@ -1788,9 +1775,7 @@ ChatCommandDefinition cc_next(
      auto s = a.c->require_server_state();
      a.check_cheats_enabled_or_allowed(s->cheat_flags.warp);
-      auto episode = a.c->proxy_session
+      auto episode = a.c->proxy_session ? a.c->proxy_session->lobby_episode : a.c->require_lobby()->episode;
          ? a.c->proxy_session->lobby_episode
          : a.c->require_lobby()->episode;
      size_t limit = FloorDefinition::limit_for_episode(episode);
      if (limit > 0) {
        send_warp(a.c, (a.c->floor + 1) % limit, true);
@@ -1835,8 +1820,7 @@ ChatCommandDefinition cc_patch(
      co_await prepare_client_for_patches(a.c);
      try {
        auto s = a.c->require_server_state();
-        // Note: We can't look this up before prepare_client_for_patches
+        // Note: We can't look this up before prepare_client_for_patches because specific_version may not be set
        // because specific_version may not be set at that point
        auto fn = s->function_code_index->get_patch(patch_name, a.c->specific_version);
        co_await send_function_call(a.c, fn, label_writes);
      } catch (const out_of_range&) {
@@ -1874,9 +1858,7 @@ ChatCommandDefinition cc_ping(
      if (a.c->proxy_session) {
        a.c->proxy_session->server_ping_start_time = a.c->ping_start_time;
        C_GuildCardSearch_40 cmd = {
-            0x00010000,
+            0x00010000, a.c->proxy_session->remote_guild_card_number, a.c->proxy_session->remote_guild_card_number};
            a.c->proxy_session->remote_guild_card_number,
            a.c->proxy_session->remote_guild_card_number};
        a.c->proxy_session->server_channel->send(0x40, 0x00, &cmd, sizeof(cmd));
      }
      co_return;
@@ -1914,7 +1896,8 @@ ChatCommandDefinition cc_playrec(
          data = phosg::load_file(file_path_for_recording(filename, a.c->login->account->account_id, false));
        } catch (const phosg::cannot_open_file&) {
          try {
-            data = prs_decompress(phosg::load_file(file_path_for_recording(filename, a.c->login->account->account_id, true)));
+            data = prs_decompress(phosg::load_file(file_path_for_recording(
                filename, a.c->login->account->account_id, true)));
          } catch (const phosg::cannot_open_file&) {
            throw precondition_failed("$C4The recording does\nnot exist");
          }
@@ -2304,8 +2287,7 @@ ChatCommandDefinition cc_savechar(
 ChatCommandDefinition cc_saverec(
    {"$saverec"},
    +[](const Args& a) -> asio::awaitable<void> {
-      // TODO: We can probably support this on the proxy server, but it would
+      // TODO: We can support this on the proxy server, but it would only include CA commands from the local player
      // only include CA commands from the local player
      a.check_is_proxy(false);
      if (!a.c->ep3_prev_battle_record) {
        throw precondition_failed("$C4No finished\nrecording is\npresent");
@@ -2452,7 +2434,7 @@ ChatCommandDefinition cc_silence(
      auto s = a.c->require_server_state();
      auto target = s->find_client(&a.text);
      if (!target->login) {
-        // this should be impossible, but I'll bet it's not actually
+        // This should be impossible, but I'll bet it's not actually
        throw precondition_failed("$C6Client not logged in");
      }
@@ -2512,9 +2494,8 @@ ChatCommandDefinition cc_spec(
        throw logic_error("Episode 3 client in non-Episode 3 game");
      }
-      // In non-tournament games, only the leader can do this; in a tournament
+      // In non-tournament games, only the leader can do this; in a tournament match, the players don't have control
-      // match, the players don't have control over who the leader is, so we allow
+      // over who the leader is, so we allow all players to use this command
      // all players to use this command
      if (!l->tournament_match) {
        a.check_is_leader();
      }
@@ -2662,8 +2643,8 @@ ChatCommandDefinition cc_swa(
      a.check_is_game(true);
      a.c->toggle_flag(Client::Flag::SWITCH_ASSIST_ENABLED);
-      send_text_message_fmt(a.c, "$C6Switch assist {}",
+      bool enabled = a.c->check_flag(Client::Flag::SWITCH_ASSIST_ENABLED);
-          a.c->check_flag(Client::Flag::SWITCH_ASSIST_ENABLED) ? "enabled" : "disabled");
+      send_text_message_fmt(a.c, "$C6Switch assist {}", enabled ? "enabled" : "disabled");
      co_return;
    });
@@ -2775,15 +2756,12 @@ ChatCommandDefinition cc_switchchar(
      a.c->bb_character_index = index;
      a.c->bb_bank_character_index = index;
-      // TODO: This can trigger a client bug where the previous character's
+      // TODO: This can trigger a client bug where the previous character's name label object isn't deleted if the
-      // name label object isn't deleted if the leave and join notifications
+      // leave and join notifications are received on the same frame. This results in the receiving player seeing both
-      // are received on the same frame. This results in the receiving player
+      // labels over the new character, with the latest one appearing on top. We could fix this by requiring each
-      // seeing both labels over the new character, with the latest one
+      // recipient to reply to a ping between the two commands, similar to how the 64 and 6x6D commands are split
-      // appearing on top. We could fix this by requiring each recipient to
+      // during game joining, but implementing that here seems not worth the effort given the low likelihood and impact
-      // reply to a ping between the two commands, similar to how the 64 and
+      // of this bug.
      // 6x6D commands are split during game joining, but implementing that
      // here seems not worth the effort given the low likelihood and impact of
      // this bug.
      send_complete_player_bb(a.c);
      send_player_leave_notification(l, a.c->lobby_client_id);
      s->send_lobby_join_notifications(l, a.c);
@@ -2822,9 +2800,7 @@ ChatCommandDefinition cc_unset(
 ChatCommandDefinition cc_variations(
    {"$variations"},
    +[](const Args& a) -> asio::awaitable<void> {
-      // Note: This command is intentionally undocumented, since it's primarily used
+      // Note: This command is intentionally undocumented, since it's primarily used for testing
      // for testing. If we ever make it public, we should add some kind of user
      // feedback (currently it sends no message when it runs).
      a.check_is_proxy(false);
      a.check_is_game(false);
      auto s = a.c->require_server_state();
@@ -2854,9 +2830,7 @@ static void command_warp(const Args& a, bool is_warpall) {
    return;
  }
-  Episode episode = a.c->proxy_session
+  Episode episode = a.c->proxy_session ? a.c->proxy_session->lobby_episode : a.c->require_lobby()->episode;
      ? a.c->proxy_session->lobby_episode
      : a.c->require_lobby()->episode;
  size_t limit = FloorDefinition::limit_for_episode(episode);
  if (limit == 0) {
    return;
@@ -2922,15 +2896,16 @@ ChatCommandDefinition cc_what(
        throw precondition_failed("$C4No items are near you");
      } else {
        auto s = a.c->require_server_state();
-        string name = s->describe_item(a.c->version(), nearest_fi->data, ItemNameIndex::Flag::INCLUDE_PSO_COLOR_ESCAPES);
+        string name = s->describe_item(
            a.c->version(), nearest_fi->data, ItemNameIndex::Flag::INCLUDE_PSO_COLOR_ESCAPES);
        send_text_message(a.c, name);
      }
      co_return;
    });
 static void whatobj_whatene_fn(const Args& a, bool include_objs, bool include_enes) {
-  // TODO: This probably wouldn't be too hard to implement for proxy sessions.
+  // TODO: This probably wouldn't be too hard to implement for proxy sessions. We already have the map and most of the
-  // We already have the map and most of the lobby metadata (episode, etc.)
+  // lobby metadata (episode, etc.)
  a.check_is_proxy(false);
  a.check_is_game(true);
  auto l = a.c->require_lobby();
@@ -2997,9 +2972,8 @@ static void whatobj_whatene_fn(const Args& a, bool include_objs, bool include_en
    }
  }
-  // Since we check all objects first, nearest_ene will only be set if
+  // Since we check all objects first, nearest_ene will only be set if there is an enemy closer than all objects. So,
-  // there is an enemy closer than all objects. So, we print that if it's
+  // we print that if it's set, and print the object if not.
  // set, and print the object if not.
  if (nearest_ene) {
    const auto* set_entry = nearest_ene->super_ene->version(a.c->version()).set_entry;
    string type_name = MapFile::name_for_enemy_type(set_entry->base_type, a.c->version(), area);
@@ -3116,10 +3090,9 @@ ChatCommandDefinition cc_nativecall(
    +[](const Args& a) -> asio::awaitable<void> {
      a.check_debug_enabled();
-      // TODO: $nativecall is not implemented on x86 (yet) because there are
+      // TODO: $nativecall is not implemented on x86 (yet) because there are multiple calling conventions used within
-      // multiple calling conventions used within the executable (at least on
+      // the executable (at least on Xbox and BB), so we would need a way to specify which calling convention to use,
-      // Xbox and BB), so we would need a way to specify which calling
+      // which would be annoying
      // convention to use, which would be annoying
      if (is_x86(a.c->version())) {
        throw precondition_failed("Command not supported\non x86 clients");
      }
@@ -3156,7 +3129,7 @@ ChatCommandDefinition cc_nativecall(
      co_return;
    });
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // Dispatch methods
 struct SplitCommand {
@@ -3174,15 +3147,13 @@ struct SplitCommand {
  }
 };
-// This function is called every time any player sends a chat beginning with a
+// This function is called every time any player sends a chat message beginning with $. It is this function's
-// dollar sign. It is this function's responsibility to see if the chat is a
+// responsibility to see if the chat is a command, and to execute the command and block the chat if it is.
 // command, and to execute the command and block the chat if it is.
 asio::awaitable<void> on_chat_command(std::shared_ptr<Client> c, const std::string& text, bool check_permissions) {
  SplitCommand cmd(text);
-  // This function is only called by on_06 if it looks like a chat command
+  // This function is only called by on_06 if it looks like a chat command (starts with $, or @ on 11/2000), so we just
-  // (starts with $, or @ on 11/2000), so we just normalize all commands to $
+  // normalize all commands to $ here
  // here
  if (!cmd.name.empty() && cmd.name[0] == '@') {
    cmd.name[0] = '$';
  }
@@ -41,7 +41,6 @@ struct ChoiceSearchConfigT {
    return ret;
  }
 } __attribute__((packed));
 using ChoiceSearchConfig = ChoiceSearchConfigT<false>;
 using ChoiceSearchConfigBE = ChoiceSearchConfigT<true>;
 check_struct_size(ChoiceSearchConfig, 0x18);
@@ -25,8 +25,7 @@ static atomic<uint64_t> next_id(1);
 void Client::set_flags_for_version(Version version, int64_t sub_version) {
  this->set_flag(Flag::PROXY_CHAT_COMMANDS_ENABLED);
-  // BB shares some sub_version values with GC Episode 3, so we handle it
+  // BB shares some sub_version values with GC Episode 3, so we handle it separately first.
  // separately first.
  if (version == Version::BB_V4) {
    this->set_flag(Flag::NO_D6);
    this->set_flag(Flag::SAVE_ENABLED);
@@ -72,8 +71,8 @@ void Client::set_flags_for_version(Version version, int64_t sub_version) {
          break;
        case Version::GC_V3:
        case Version::GC_EP3:
-          // Some of these versions have send_function_call and some don't; we
+          // Some of these versions have send_function_call and some don't; we have to set these flags later when we
-          // have to set these flags later when we get sub_version
+          // get sub_version
          break;
        case Version::XB_V3:
          // TODO: Do all versions of XB need this flag? US does, at least.
@@ -142,8 +141,8 @@ void Client::set_flags_for_version(Version version, int64_t sub_version) {
      this->set_flag(Flag::SEND_FUNCTION_CALL_ACTUALLY_RUNS_CODE);
      this->set_flag(Flag::ENCRYPTED_SEND_FUNCTION_CALL);
      this->set_flag(Flag::SEND_FUNCTION_CALL_NO_CACHE_PATCH);
-      // sub_version can't be used to tell JP final and Trial Edition apart; we
+      // sub_version can't be used to tell JP final and Trial Edition apart; we instead look at header.flag in the 61
-      // instead look at header.flag in the 61 command and set the version then.
+      // command and set the version then.
      break;
    case 0x41: // GC Ep3 US (and BB, but BB is handled above)
    case 0x42: // GC Ep3 EU 50Hz
@@ -191,9 +190,8 @@ Client::Client(
      should_update_play_time(false) {
  this->update_channel_name();
-  // Don't print data sent to patch clients to the logs. The patch server
+  // Don't print data sent to patch clients to the logs. The patch server protocol is fully understood and data logs
-  // protocol is fully understood and data logs for patch clients are generally
+  // for patch clients are generally more annoying than helpful at this point.
  // more annoying than helpful at this point.
  auto s = server->get_state();
  if (is_patch(this->version()) && s->hide_download_commands) {
    this->channel->terminal_recv_color = phosg::TerminalFormat::END;
@@ -212,9 +210,8 @@ Client::Client(
  this->reschedule_save_game_data_timer();
  this->reschedule_ping_and_timeout_timers();
-  // Don't print data sent to patch clients to the logs. The patch server
+  // Don't print data sent to patch clients to the logs. The patch server protocol is fully understood and data logs
-  // protocol is fully understood and data logs for patch clients are generally
+  // for patch clients are generally more annoying than helpful at this point.
  // more annoying than helpful at this point.
  if ((s->hide_download_commands) &&
      ((this->version() == Version::PC_PATCH) || (this->version() == Version::BB_PATCH))) {
    this->channel->terminal_recv_color = phosg::TerminalFormat::END;
@@ -298,9 +295,8 @@ void Client::reschedule_ping_and_timeout_timers() {
 }
 void Client::convert_account_to_temporary_if_nte() {
-  // If the session is a prototype version and the account was created and we
+  // If the session is a prototype version and the account was created and we should use a temporary account instead,
-  // should use a temporary account instead, delete the permanent account and
+  // delete the permanent account and replace it with a temporary account.
  // replace it with a temporary account.
  auto s = this->require_server_state();
  if (s->use_temp_accounts_for_prototypes && this->login->account_was_created && is_any_nte(this->version())) {
    this->log.info_f("Client is a prototype version and the account was created during this session; converting permanent account to temporary account");
@@ -353,8 +349,7 @@ shared_ptr<const TeamIndex::Team> Client::team() const {
    return nullptr;
  }
-  // The team membership is valid, but the player name may be different; update
+  // The team membership is valid, but the player name may be different; update the team membership if needed
  // the team membership if needed
  if (p) {
    auto& m = member_it->second;
    string name = p->disp.name.decode(this->language());
@@ -601,8 +596,8 @@ void Client::save_character_file() {
    throw logic_error("no character file loaded");
  }
  if (this->should_update_play_time) {
-    // This is slightly inaccurate, since fractions of a second are truncated
+    // This is slightly inaccurate, since fractions of a second are truncated off each time we save. I'm lazy, so
-    // off each time we save. I'm lazy, so insert shrug emoji here.
+    // insert shrug emoji here
    uint64_t t = phosg::now();
    uint64_t seconds = (t - this->last_play_time_update) / 1000000;
    this->character_data->play_time_seconds += seconds;
@@ -642,8 +637,7 @@ void Client::create_battle_overlay(shared_ptr<const BattleRules> rules, shared_p
    this->overlay_character_data->inventory.remove_all_items_of_type(3);
  }
  if (rules->replace_char) {
-    // TODO: Shouldn't we clear other material usage here? It looks like the
+    // TODO: Shouldn't we clear other material usage here? Looks like the original code doesn't, but that seems wrong.
    // original code doesn't, but that seems wrong.
    this->overlay_character_data->inventory.hp_from_materials = 0;
    this->overlay_character_data->inventory.tp_from_materials = 0;
@@ -678,7 +672,8 @@ void Client::create_battle_overlay(shared_ptr<const BattleRules> rules, shared_p
  }
 }
-void Client::create_challenge_overlay(Version version, size_t template_index, shared_ptr<const LevelTable> level_table) {
+void Client::create_challenge_overlay(
    Version version, size_t template_index, shared_ptr<const LevelTable> level_table) {
  auto p = this->character_file(true, false);
  const auto& tpl = get_challenge_template_definition(version, p->disp.visual.class_flags, template_index);
@@ -700,9 +695,11 @@ void Client::create_challenge_overlay(Version version, size_t template_index, sh
  level_table->reset_to_base(overlay->disp.stats, overlay->disp.visual.char_class);
  level_table->advance_to_level(overlay->disp.stats, tpl.level, overlay->disp.visual.char_class);
  const auto& stats_delta = level_table->stats_delta_for_level(
      overlay->disp.visual.char_class, overlay->disp.stats.level);
  overlay->disp.stats.esp = 40;
  overlay->disp.stats.unknown_a3 = 10.0;
-  overlay->disp.stats.experience = level_table->stats_delta_for_level(overlay->disp.visual.char_class, overlay->disp.stats.level).experience;
+  overlay->disp.stats.experience = stats_delta.experience;
  overlay->disp.stats.meseta = 0;
  overlay->clear_all_material_usage();
  for (size_t z = 0; z < 0x13; z++) {
@@ -762,10 +759,9 @@ std::shared_ptr<PlayerBank> Client::bank_file(bool allow_load) {
      this->bank_data->load(f.get());
      this->log.info_f("Loaded bank data from {}", filename);
    } catch (const phosg::cannot_open_file&) {
-      // If there isn't a psobank file, use the loaded character data if the
+      // If there isn't a psobank file, use the loaded character data if the bank character index matches the current
-      // bank character index matches the current character index (that is, we
+      // character index (that is, we should use the current character's bank); otherwise, load the corresponding
-      // should use the current character's bank); otherwise, load the
+      // character and parse the bank from that character file
      // corresponding character and parse the bank from that character file
      if (this->bb_bank_character_index == this->bb_character_index) {
        this->bank_data = std::make_shared<PlayerBank>(this->character_file(true, false)->bank);
        this->log.info_f("Using bank data from loaded character");
@@ -891,8 +887,7 @@ void Client::load_all_files() {
      this->character_data = psochar.character_file;
      this->log.info_f("Loaded character data from {}", char_filename);
-      // If there was no .psosys file, use the system file from the .psochar
+      // If there was no .psosys file, use the system file from the .psochar file instead
      // file instead
      if (!this->system_data) {
        if (!psochar.system_file) {
          throw logic_error("account system data not present, and also not loaded from psochar file");
@@ -920,8 +915,7 @@ void Client::load_all_files() {
    }
  }
-  // If any of the above files are still missing, try to load from .nsa/.nsc
+  // If any of the above files are still missing, try to load from .nsa/.nsc files instead
  // files instead
  if (!this->system_data || (!this->character_data && (this->bb_character_index >= 0)) || !this->guild_card_data) {
    string nsa_filename = this->legacy_account_filename();
    shared_ptr<LegacySavedAccountDataBB> nsa_data;
@@ -987,9 +981,8 @@ void Client::load_all_files() {
    }
  }
-  // The system and Guild Card files can be auto-created if they can't be
+  // The system and Guild Card files can be auto-created if they can't be loaded. After this, system_data and
-  // loaded. After this, system_data and guild_card_data are always non-null,
+  // guild_card_data are always non-null, but character_data may still be null
  // but character_data may still be null
  if (!this->system_data) {
    this->system_data = make_shared<PSOBBBaseSystemFile>();
    auto s = this->require_server_state();
@@ -1024,8 +1017,8 @@ void Client::load_all_files() {
    this->login->account->save();
    this->last_play_time_update = phosg::now();
    if (this->bb_character_index >= 0) {
-      // Note that bank_file() can't recur infinitely here because
+      // Note that bank_file() can't recur infinitely here because character_file is already set; it will not call
-      // character_file is already set; it will not call load_all_files() again
+      // load_all_files() again
      this->bank_file()->enforce_stack_limits(stack_limits);
    }
  }
@@ -189,8 +189,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;
-  // These are null unless the client is within the trade sequence (D0-D4 or EE
+  // These are null unless the client is within the trade sequence (D0-D4 or EE commands)
  // commands)
  std::unique_ptr<PendingItemTrade> pending_item_trade;
  std::unique_ptr<PendingCardTrade> pending_card_trade;
  uint32_t telepipe_lobby_id = 0;
@@ -203,12 +202,10 @@ public:
  uint8_t schtserv_response_register = 0;
  uint32_t next_exp_value = 0;
  bool can_chat = true;
-  // NOTE: If you add any new optional promises here, make sure to also add
+  // NOTE: If you add any new optional promises here, make sure to also add them to cancel_pending_promises.
-  // them to cancel_pending_promises.
+  // NOTE: Entries in this queue can be nullptr; that represents a B2 command sent by the remote server during a proxy
-  // NOTE: Entries in this queue can be nullptr; that represents a B2 command
+  // session. We can't just omit those from the queue entirely, because if we did, we could end up sending the wrong B3
-  // sent by the remote server during a proxy session. We can't just omit those
+  // response back.
  // from the queue entirely, because if we did, we could end up sending the
  // wrong B3 response back.
  std::deque<std::shared_ptr<AsyncPromise<C_ExecuteCodeResult_B3>>> function_call_response_queue;
  std::shared_ptr<AsyncPromise<GetPlayerInfoResult>> character_data_ready_promise;
  std::shared_ptr<AsyncPromise<void>> enable_save_promise;
@@ -216,10 +213,7 @@ public:
  // File loading state
  std::unordered_map<std::string, std::shared_ptr<const std::string>> sending_files;
-  Client(
+  Client(std::shared_ptr<GameServer> server, std::shared_ptr<Channel> channel, ServerBehavior server_behavior);
      std::shared_ptr<GameServer> server,
      std::shared_ptr<Channel> channel,
      ServerBehavior server_behavior);
  ~Client();
  void update_channel_name();
@@ -258,8 +252,6 @@ public:
  void convert_account_to_temporary_if_nte();
  void sync_config();
  std::shared_ptr<ServerState> require_server_state() const;
  std::shared_ptr<Lobby> require_lobby() const;
@@ -353,9 +345,8 @@ public:
  void cancel_pending_promises();
 private:
-  // The overlay character data is used in battle and challenge modes, when
+  // The overlay character data is used in battle and challenge modes, when character data is temporarily replaced
-  // character data is temporarily replaced in-game. In other play modes and in
+  // in-game. In other play modes and in lobbies, overlay_character_data is null.
  // lobbies, overlay_character_data is null.
  std::shared_ptr<PSOBBBaseSystemFile> system_data;
  std::shared_ptr<PSOBBCharacterFile> overlay_character_data;
  std::shared_ptr<PSOBBCharacterFile> character_data;
@@ -86,26 +86,17 @@ struct VectorXYZF {
  inline VectorXYZF rotate_x(double angle) const {
    double s = sin(angle);
    double c = cos(angle);
-    return VectorXYZF{
+    return VectorXYZF{this->x, this->y * c - this->z * s, this->y * s + this->z * c};
        this->x,
        this->y * c - this->z * s,
        this->y * s + this->z * c};
  }
  inline VectorXYZF rotate_y(double angle) const {
    double s = sin(angle);
    double c = cos(angle);
-    return VectorXYZF{
+    return VectorXYZF{this->x * c + this->z * s, this->y, -this->x * s + this->z * c};
        this->x * c + this->z * s,
        this->y,
        -this->x * s + this->z * c};
  }
  inline VectorXYZF rotate_z(double angle) const {
    double s = sin(angle);
    double c = cos(angle);
-    return VectorXYZF{
+    return VectorXYZF{this->x * c - this->y * s, this->x * s + this->y * c, this->z};
        this->x * c - this->y * s,
        this->x * s + this->y * c,
        this->z};
  }
  inline std::string str() const {
@@ -141,23 +132,20 @@ check_struct_size(ArrayRefBE, 8);
 template <bool BE>
 struct RELFileFooterT {
  static constexpr bool IsBE = BE;
-  // Relocations is a list of words (le_uint16_t on DC/PC/XB/BB, be_uint16_t on
+  // Relocations is a list of words (le_uint16_t on DC/PC/XB/BB, be_uint16_t on GC) containing the number of
-  // GC) containing the number of doublewords (uint32_t) to skip for each
+  // doublewords (uint32_t) to skip for each relocation. The relocation pointer starts at the beginning of the file
-  // relocation. The relocation pointer starts at the beginning of the file
+  // data, and advances by the value of one relocation word (times 4) before each relocation. At each relocated
-  // data, and advances by the value of one relocation word (times 4) before
+  // doubleword, the address of the first byte of the file is added to the existing value.
-  // each relocation. At each relocated doubleword, the address of the first
+  //
-  // byte of the file is added to the existing value.
+  // For example, if the file data contains the following data (where R specifies doublewords to relocate):
  // For example, if the file data contains the following data (where R
  // specifies doublewords to relocate):
  //   RR RR RR RR ?? ?? ?? ?? ?? ?? ?? ?? RR RR RR RR
  //   RR RR RR RR ?? ?? ?? ?? RR RR RR RR
  // then the relocation words should be 0000, 0003, 0001, and 0002.
-  // If there is a small number of relocations, they may be placed in the unused
+  //
-  // fields of this structure to save space and/or confuse reverse engineers.
+  // If there is a small number of relocations, they may be placed in the unused fields of this structure to save space
-  // The game never accesses the last 12 bytes of this structure unless
+  // and/or confuse reverse engineers. The game never accesses the last 12 bytes of this structure unless
-  // relocations_offset points there, so those 12 bytes may also be omitted
+  // relocations_offset points there, so those 12 bytes may also be omitted entirely in some situations (e.g. in the B2
-  // entirely in situations (e.g. in the B2 command, without changing code_size,
+  // command, without changing code_size, so code_size would technically extend beyond the end of the B2 command).
  // so code_size would technically extend beyond the end of the B2 command).
  U32T<BE> relocations_offset = 0;
  U32T<BE> num_relocations = 0;
  parray<U32T<BE>, 2> unused1;
@@ -266,7 +266,7 @@ void CommonItemSet::Table::print(FILE* stream) const {
        this->special_mult[z], this->special_percent[z]);
  }
-  phosg::fwrite_fmt(stream, "  Tool class table:\n");
+  phosg::fwrite_fmt(stream, "Tool class table:\n");
  phosg::fwrite_fmt(stream, "  CS     A1     A2     A3     A4     A5     A6     A7     A8     A9    A10\n");
  for (size_t tool_class = 0; tool_class < this->tool_class_prob_table.size(); tool_class++) {
    phosg::fwrite_fmt(stream, "  {:02X}", tool_class);
@@ -298,7 +298,7 @@ void CommonItemSet::Table::print(FILE* stream) const {
      "MEGID   ",
  };
-  phosg::fwrite_fmt(stream, "  Technique table:\n");
+  phosg::fwrite_fmt(stream, "Technique table:\n");
  phosg::fwrite_fmt(stream, "  TECH                   A1            A2            A3            A4            A5            A6            A7            A8            A9           A10\n");
  for (size_t tech_num = 0; tech_num < this->technique_index_prob_table.size(); tech_num++) {
    phosg::fwrite_fmt(stream, "  {:02X}:{}", tech_num, technique_names[tech_num]);
@@ -316,21 +316,21 @@ void CommonItemSet::Table::print(FILE* stream) const {
    fputc('\n', stream);
  }
-  phosg::fwrite_fmt(stream, "  Armor/shield type bias: {}\n", this->armor_or_shield_type_bias);
+  phosg::fwrite_fmt(stream, "Armor/shield type bias: {}\n", this->armor_or_shield_type_bias);
-  phosg::fwrite_fmt(stream, "  Armor/shield type index table:\n");
+  phosg::fwrite_fmt(stream, "Armor/shield type index table:\n");
  phosg::fwrite_fmt(stream, "  TY  PROB\n");
  for (size_t z = 0; z < 5; z++) {
    phosg::fwrite_fmt(stream, "  {:02X}  {:3}%\n", z, this->armor_shield_type_index_prob_table[z]);
  }
-  phosg::fwrite_fmt(stream, "  Armor/shield slot count table:\n");
+  phosg::fwrite_fmt(stream, "Armor/shield slot count table:\n");
  phosg::fwrite_fmt(stream, "  #S  PROB\n");
  for (size_t z = 0; z < 5; z++) {
    phosg::fwrite_fmt(stream, "  {:02X}  {:3}%\n", z, this->armor_slot_count_prob_table[z]);
  }
-  phosg::fwrite_fmt(stream, "  Unit maximum stars table:\n");
+  phosg::fwrite_fmt(stream, "Unit maximum stars table:\n");
  phosg::fwrite_fmt(stream, "  AR   #*\n");
  for (size_t z = 0; z < 10; z++) {
    phosg::fwrite_fmt(stream, "  {:02X}  {:3}\n", z, this->unit_max_stars_table[z]);
@@ -534,7 +534,10 @@ void CommonItemSet::print(FILE* stream) const {
          try {
            auto table = this->get_table(episode, mode, difficulty, section_id);
            phosg::fwrite_fmt(stream, "============ {} {} {} {}\n",
-                name_for_mode(mode), name_for_episode(episode), name_for_difficulty(difficulty), name_for_section_id(section_id));
+                name_for_mode(mode),
                name_for_episode(episode),
                name_for_difficulty(difficulty),
                name_for_section_id(section_id));
            table->print(stream);
          } catch (const runtime_error&) {
          }
@@ -564,13 +567,22 @@ void CommonItemSet::print_diff(FILE* stream, const CommonItemSet& other) const {
            continue;
          } else if (!this_table) {
            phosg::fwrite_fmt(stream, "> Table present in other but not this: {} {} {} {}\n",
-                name_for_mode(mode), name_for_episode(episode), name_for_difficulty(difficulty), name_for_section_id(section_id));
+                name_for_mode(mode),
                name_for_episode(episode),
                name_for_difficulty(difficulty),
                name_for_section_id(section_id));
          } else if (!other_table) {
            phosg::fwrite_fmt(stream, "> Table present in this but not other: {} {} {} {}\n",
-                name_for_mode(mode), name_for_episode(episode), name_for_difficulty(difficulty), name_for_section_id(section_id));
+                name_for_mode(mode),
                name_for_episode(episode),
                name_for_difficulty(difficulty),
                name_for_section_id(section_id));
          } else if (*this_table != *other_table) {
            phosg::fwrite_fmt(stream, "> Tables do not match: {} {} {} {}\n",
-                name_for_mode(mode), name_for_episode(episode), name_for_difficulty(difficulty), name_for_section_id(section_id));
+                name_for_mode(mode),
                name_for_episode(episode),
                name_for_difficulty(difficulty),
                name_for_section_id(section_id));
            this_table->print_diff(stream, *other_table);
          }
        }
@@ -665,8 +677,7 @@ shared_ptr<const CommonItemSet::Table> CommonItemSet::get_table(
 AFSV2CommonItemSet::AFSV2CommonItemSet(
    std::shared_ptr<const std::string> pt_afs_data, std::shared_ptr<const std::string> ct_afs_data) {
-  // Each AFS file has 40 entries (30 on v1); the first 10 are for Normal, then
+  // Each AFS file has 40 entries (30 on v1); the first 10 are for Normal, then Hard, etc.
  // Hard, etc.
  {
    AFSArchive pt_afs(pt_afs_data);
    bool include_ultimate;
@@ -692,8 +703,7 @@ AFSV2CommonItemSet::AFSV2CommonItemSet(
    }
  }
-  // ItemCT AFS files also have 40 entries, but only the 0th, 10th, 20th, and
+  // ItemCT AFS files also have 40 entries, but only the 0th, 10th, 20th, and 30th are used (section_id is ignored)
  // 30th are used (section_id is ignored)
  if (ct_afs_data) {
    AFSArchive ct_afs(ct_afs_data);
    bool include_ultimate;
@@ -64,54 +64,43 @@ public:
    template <bool BE>
    struct OffsetsT {
-      // This data structure uses index probability tables in multiple places. An
+      // This data structure uses index probability tables in multiple places. An index probability table is a table
-      // index probability table is a table where each entry holds the probability
+      // where each entry holds the probability that that entry's index is used. For example, if the armor slot count
-      // that that entry's index is used. For example, if the armor slot count
+      // probability table contains [77, 17, 5, 1, 0], this means there is a 77% chance of no slots, 17% chance of 1
-      // probability table contains [77, 17, 5, 1, 0], this means there is a 77%
+      // slot, 5% chance of 2 slots, 1% chance of 3 slots, and no chance of 4 slots. The values in index probability
-      // chance of no slots, 17% chance of 1 slot, 5% chance of 2 slots, 1% chance
+      // tables do not have to add up to 100; the game sums all of them and chooses a random number less than that
-      // of 3 slots, and no chance of 4 slots. The values in index probability
+      // maximum.
      // tables do not have to add up to 100; the game sums all of them and
      // chooses a random number less than that maximum.
-      // The area (floor) number is used in many places as well. Unlike the normal
+      // The area (floor) number is used in many places as well. Unlike the normal area numbers, which start with
-      // area numbers, which start with Pioneer 2, the area numbers in this
+      // Pioneer 2, the area numbers in this structure start with Forest 1, and boss areas are treated as the first
-      // structure start with Forest 1, and boss areas are treated as the first
+      // area of the next section (so De Rol Le has Mines 1 drops, for example). Final boss areas are treated as the
-      // area of the next section (so De Rol Le has Mines 1 drops, for example).
+      // last non-boss area (so Dark Falz boxes are like Ruins 3 boxes). We refer to these adjusted area numbers as
-      // Final boss areas are treated as the last non-boss area (so Dark Falz
+      // (area - 1), or area_norm.
      // boxes are like Ruins 3 boxes). We refer to these adjusted area numbers as
      // (area - 1).
-      // This index probability table determines the types of non-rare weapons.
+      // This index probability table determines the types of non-rare weapons. The indexes in this table correspond to
-      // The indexes in this table correspond to the non-rare weapon types 01
+      // the non-rare weapon types 01 through 0C (Saber through Wand).
      // through 0C (Saber through Wand).
      // V2/V3: -> parray<uint8_t, 0x0C>
      /* 00 */ U32T<BE> base_weapon_type_prob_table_offset;
-      // This table specifies the base subtype for each weapon type. Negative
+      // This table specifies the base subtype for each weapon type. Negative values here mean that the weapon cannot
-      // values here mean that the weapon cannot be found in the first N areas (so
+      // be found in the first N areas (so -2, for example, means that the weapon never appears in Forest 1 or 2 at
-      // -2, for example, means that the weapon never appears in Forest 1 or 2 at
+      // all). Nonnegative values here mean the subtype can be found in all areas, and specify the base subtype
-      // all). Nonnegative values here mean the subtype can be found in all areas,
+      // (usually in the range [0, 4]). The subtype of weapon that actually appears depends on this value and a value
-      // and specify the base subtype (usually in the range [0, 4]). The subtype
+      // from the following table.
      // of weapon that actually appears depends on this value and a value from
      // the following table.
      // V2/V3: -> parray<int8_t, 0x0C>
      /* 04 */ U32T<BE> subtype_base_table_offset;
-      // This table specifies how many areas each weapon subtype appears in. For
+      // This table specifies how many areas each weapon subtype appears in. For example, if Sword (subtype 02, which
-      // example, if Sword (subtype 02, which is index 1 in this table and the
+      // is index 1 in this table and the table above) has a subtype base of -2 and a subtype area length of 4, then
-      // table above) has a subtype base of -2 and a subtype area length of 4,
+      // Sword items can be found when area - 1 is 2, 3, 4, or 5 (Cave 1 through Mine 1), and Gigush (the next sword
-      // then Sword items can be found when area - 1 is 2, 3, 4, or 5 (Cave 1
+      // subtype) can be found in Mine 1 through Ruins 3.
      // through Mine 1), and Gigush (the next sword subtype) can be found in Mine
      // 1 through Ruins 3.
      // V2/V3: -> parray<uint8_t, 0x0C>
      /* 08 */ U32T<BE> subtype_area_length_table_offset;
-      // This index probability table specifies how likely each possible grind
+      // This index probability table specifies how likely each possible grind value is. The table is indexed as
-      // value is. The table is indexed as [grind][subtype_area_index], where the
+      // [grind][subtype_area_index], where the subtype area index is how many areas the player is beyond the first
-      // subtype area index is how many areas the player is beyond the first area
+      // area in which the subtype can first be found (clamped to [0, 3]). To continue the example above, in Cave 3,
-      // in which the subtype can first be found (clamped to [0, 3]). To continue
+      // subtype_area_index would be 2, since Swords can first be found two areas earlier in Cave 1.
      // the example above, in Cave 3, subtype_area_index would be 2, since Swords
      // can first be found two areas earlier in Cave 1.
      // For example, this table could look like this:
      //   [64 1E 19 14] // Chance of getting a grind +0
      //   [00 1E 17 0F] // Chance of getting a grind +1
@@ -121,29 +110,28 @@ public:
      // V2/V3: -> parray<parray<uint8_t, 4>, 9>
      /* 0C */ U32T<BE> grind_prob_table_offset;
-      // TODO: Figure out exactly how this table is used. Anchor: 80106D34
+      // This index probability table specifies how likely each type of armor or shield is. The general formula is:
      //   data1[2] = max((area_norm + (result from this table) + armor_or_shield_type_bias - 3), 0)
      // In this way, (armor_or_shield_type_bias + area_norm - 3) can be thought of as the "base" value for each area,
      // and this table specifies how likely the armor/shield is to be "upgraded" from that value.
      // V2/V3: -> parray<uint8_t, 0x05>
      /* 10 */ U32T<BE> armor_shield_type_index_prob_table_offset;
-      // This index probability table specifies how common each possible slot
+      // This index probability table specifies how common each possible slot count is for armor drops.
      // count is for armor drops.
      // V2/V3: -> parray<uint8_t, 0x05>
      /* 14 */ U32T<BE> armor_slot_count_prob_table_offset;
-      // This array (indexed by enemy_type) specifies the range of meseta values
+      // This array (indexed by enemy_type) specifies the range of meseta values that each enemy can drop.
      // that each enemy can drop.
      // V2/V3: -> parray<Range<U16T>, 0x64>
      /* 18 */ U32T<BE> enemy_meseta_ranges_offset;
-      // Each byte in this table (indexed by enemy_type) represents the percent
+      // Each byte in this table (indexed by enemy_type) represents the percent chance that the enemy drops anything at
-      // chance that the enemy drops anything at all. (This check is done before
+      // all. (This check is done before the rare drop check, so the chance of getting a rare item from an enemy is
-      // the rare drop check, so the chance of getting a rare item from an enemy
+      // essentially this probability multiplied by the rare drop rate.)
      // is essentially this probability multiplied by the rare drop rate.)
      // V2/V3: -> parray<uint8_t, 0x64>
      /* 1C */ U32T<BE> enemy_type_drop_probs_offset;
-      // Each byte in this table (indexed by enemy_type) represents the class of
+      // Each byte in this table (indexed by enemy_type) represents the class of item that can drop. The values are:
      // item that the enemy can drop. The values are:
      //   00 = weapon
      //   01 = armor
      //   02 = shield
@@ -154,41 +142,34 @@ public:
      // V2/V3: -> parray<uint8_t, 0x64>
      /* 20 */ U32T<BE> enemy_item_classes_offset;
-      // This table (indexed by area - 1) specifies the ranges of meseta values
+      // This table (indexed by area - 1) specifies the ranges of meseta values that can drop from boxes.
      // that can drop from boxes.
      // V2/V3: -> parray<Range<U16T>, 0x0A>
      /* 24 */ U32T<BE> box_meseta_ranges_offset;
-      // This array specifies the chance that a rare weapon will have each
+      // This array specifies the chance that a rare weapon will have each possible bonus value. This is indexed as
-      // possible bonus value. This is indexed as [(bonus_value - 10 / 5)][spec],
+      // [(bonus_value - 10 / 5)][spec], so the first row refers the probability of getting a -10% bonus, the next row
-      // so the first row refers the probability of getting a -10% bonus, the next
+      // is the chance of getting -5%, etc., all the way up to +100%. For non-rare items (or all items on v1/v2), spec
-      // row is the chance of getting -5%, etc., all the way up to +100%. For
+      // is determined randomly based on the following field; for rare items on v3+, spec is always 5.
      // non-rare items, spec is determined randomly based on the following field;
      // for rare items, spec is always 5.
      // V2: -> parray<parray<uint8_t, 5>, 0x17>
      // V3: -> parray<parray<U16T, 6>, 0x17>
      /* 28 */ U32T<BE> bonus_value_prob_table_offset;
-      // This array specifies the value of spec to be used in the above lookup for
+      // This array specifies the value of spec to be used in the above lookup for non-rare items. This is NOT an index
-      // non-rare items. This is NOT an index probability table; this is a direct
+      // probability table; this is a direct lookup with indexes [bonus_index][area - 1]. A value of 0xFF in any byte
-      // lookup with indexes [bonus_index][area - 1]. A value of 0xFF in any byte
+      // of this array prevents any weapon from having a bonus in that slot. An example table might look like this:
      // of this array prevents any weapon from having a bonus in that slot.
      // For example, the array might look like this:
      //   [00 00 00 01 01 01 01 02 02 02]
      //   [FF FF FF 00 00 00 01 01 01 01]
      //   [FF FF FF FF FF FF FF FF FF 00]
      //    F1 F2 C1 C2 C3 M1 M2 R1 R2 R3  // (Episode 1 areas, for reference)
-      // In this example, spec is 0, 1, or 2 in all cases where a weapon can have
+      // In this example, spec is 0, 1, or 2 in all cases where a weapon can have a bonus. In Forest 1 and 2 and Cave
-      // a bonus. In Forest 1 and 2 and Cave 1, weapons may have at most one
+      // 1, weapons may have at most one bonus; in all other areas except Ruins 3, they can have at most two bonuses,
-      // bonus; in all other areas except Ruins 3, they can have at most two
+      // and in Ruins 3, they can have up to three bonuses.
      // bonuses, and in Ruins 3, they can have up to three bonuses.
      // V2/V3: // -> parray<parray<uint8_t, 10>, 3>
      /* 2C */ U32T<BE> nonrare_bonus_prob_spec_offset;
-      // This array specifies the chance that a weapon will have each bonus type.
+      // This array specifies the chance that a weapon will have each bonus type. The table is indexed as
-      // The table is indexed as [bonus_type][area - 1] for non-rare items; for
+      // [bonus_type][area - 1] for non-rare items; for rare items, a random value in the range [0, 9] is used instead
-      // rare items, a random value in the range [0, 9] is used instead of
+      // of (area - 1).
      // (area - 1).
      // For example, the table might look like this:
      //   [46 46 3F 3E 3E 3D 3C 3C 3A 3A] // Chance of getting no bonus
      //   [14 14 0A 0A 09 02 02 04 05 05] // Chance of getting Native bonus
@@ -200,54 +181,50 @@ public:
      // V2/V3: -> parray<parray<uint8_t, 10>, 6>
      /* 30 */ U32T<BE> bonus_type_prob_table_offset;
-      // This array (indexed by area - 1) specifies a multiplier of used in
+      // This array (indexed by area - 1) specifies a parameter used in weapon special generation. If the sampled value
-      // special ability determination. It seems this uses the star values from
+      // from this table is 0, no special is generated. Otherwise, a random floating-point value W in the range [0,
-      // ItemPMT, but not yet clear exactly in what way.
+      // special_mult] is generated and truncated to an integer. If this value is greater than 3, no special is
-      // TODO: Figure out exactly what this does. Anchor: 80106FEC
+      // generated; otherwise, a random special worth (W + 1) stars is chosen. It seems Sega only intended special_mult
      // to be in the range [0, 4], but values greater than 4 will work, and will simply increase the probability of
      // getting no special.
      // V2/V3: -> parray<uint8_t, 0x0A>
      /* 34 */ U32T<BE> special_mult_offset;
-      // This array (indexed by area - 1) specifies the probability that any
+      // This array (indexed by area - 1) specifies the probability that a non-rare weapon will have a special ability.
      // non-rare weapon will have a special ability.
      // V2/V3: -> parray<uint8_t, 0x0A>
      /* 38 */ U32T<BE> special_percent_offset;
-      // This index probability table is indexed by [tool_class][area - 1]. The
+      // This index probability table is indexed by [tool_class][area - 1]. The tool class refers to an entry in
-      // tool class refers to an entry in ItemPMT, which links it to the actual
+      // ItemPMT, which links it to the actual item code.
      // item code.
      // V2/V3: -> parray<parray<U16T, 0x0A>, 0x1C>
      /* 3C */ U32T<BE> tool_class_prob_table_offset;
-      // This index probability table determines how likely each technique is to
+      // This index probability table determines how likely each technique is to appear. The table is indexed as
-      // appear. The table is indexed as [technique_num][area - 1].
+      // [technique_num][area - 1].
      // V2/V3: -> parray<parray<uint8_t, 0x0A>, 0x13>
      /* 40 */ U32T<BE> technique_index_prob_table_offset;
-      // This table specifies the ranges for technique disk levels. The table is
+      // This table specifies the ranges for technique disk levels. The table is indexed as [technique_num][area - 1].
-      // indexed as [technique_num][area - 1]. If either min or max in the range
+      // If either min or max in the range is 0xFF, or if max < min, technique disks are not dropped for that technique
-      // is 0xFF, or if max < min, technique disks are not dropped for that
+      // and area pair.
      // technique and area pair.
      // V2/V3: -> parray<parray<Range<uint8_t>, 0x0A>, 0x13>
      /* 44 */ U32T<BE> technique_level_ranges_offset;
      // See comments on armor_shield_type_index_prob_table_offset for how this is used.
      /* 48 */ uint8_t armor_or_shield_type_bias;
      /* 49 */ parray<uint8_t, 3> unused1;
-      // These values specify the maximum number of stars any generated unit can
+      // These values specify the maximum number of stars any generated unit can have in each area. The values here are
-      // have in each area. The values here are not inclusive; that is, a value
+      // not inclusive; that is, a value of 7 means that only units with 1-6 stars can drop in that area. The game
-      // of 7 means that only units with 1-6 stars can drop in that area. The
+      // uniformly chooses a random number of stars in the acceptable range, then uniformly chooses a random unit with
-      // game uniformly chooses a random number of stars in the acceptable
+      // that many stars.
      // range, then uniformly chooses a random unit with that many stars.
      // V2/V3: -> parray<uint8_t, 0x0A>
      /* 4C */ U32T<BE> unit_max_stars_offset;
-      // This index probability table determines which type of items drop from
+      // This index probability table determines which type of items drop from boxes. The table is indexed as
-      // boxes. The table is indexed as [item_class][area - 1], with item_class
+      // [item_class][area - 1], with item_class as the result value (that is, in the example below, the game looks at
-      // as the result value (that is, in the example below, the game looks at a
+      // a single column and sums the values going down, then the chosen item class is one of the row indexes based on
-      // single column and sums the values going down, then the chosen item
+      // the weight values in the column.) The resulting value has the same meaning as in enemy_item_classes above.
      // class is one of the row indexes based on the weight values in the
      // column.) The resulting item_class value has the same meaning as in
      // enemy_item_classes above.
      // For example, this array might look like the following:
      //   [07 07 08 08 06 07 08 09 09 0A] // Chances per area of a weapon drop
      //   [02 02 02 02 03 02 02 02 03 03] // Chances per area of an armor drop
@@ -299,8 +276,8 @@ public:
  explicit JSONCommonItemSet(const phosg::JSON& json);
 };
-// Note: There are clearly better ways of doing this, but this implementation
+// Note: There are clearly better ways of doing this, but this implementation closely follows what the original code in
-// closely follows what the original code in the client does.
+// the client does.
 template <typename ItemT, size_t MaxCount>
 struct ProbabilityTable {
  ItemT items[MaxCount];
@@ -368,11 +345,9 @@ protected:
  RELFileSet(std::shared_ptr<const std::string> data);
  template <typename T>
-  std::pair<const T*, size_t> get_table(
+  std::pair<const T*, size_t> get_table(const TableSpec& spec, size_t index) const {
      const TableSpec& spec, size_t index) const {
    const T* entries = &r.pget<T>(
-        spec.offset + index * spec.entries_per_table * sizeof(T),
+        spec.offset + index * spec.entries_per_table * sizeof(T), spec.entries_per_table * sizeof(T));
        spec.entries_per_table * sizeof(T));
    return std::make_pair(entries, spec.entries_per_table);
  }
 };
@@ -485,17 +460,14 @@ private:
  } __packed_ws__(LuckTableEntry, 2);
  struct Offsets {
-    // Each section ID's favored weapon class has different probabilities than
+    // Each section ID's favored weapon class has different probabilities than those used for all other weapons. The
-    // those used for all other weapons. The tables are labeled with (D) for the
+    // tables are labeled with (D) for the default values and (F) for the favored-class values.
    // default values and (F) for the favored-class values.
-    // Note that the favored bonuses for Redria are all zero; these values are
+    // Note that the favored bonuses for Redria are all zero; these values are unused because Redria does not have a
-    // unused because Redria does not have a favored weapon type. Curiously,
+    // favored weapon type. Curiously, Yellowboze also does not have a favored weapon type, but the values for
    // Yellowboze also does not have a favored weapon type, but the values for
    // Yellowboze are not all zero.
-    // This table specifies how likely a special is to be upgraded or
+    // This table specifies how likely a special is to be upgraded or downgraded by one level.
    // downgraded by one level.
    // In PSO V3, the special upgrade table is:
    //   Viridia    => (D) +1=10%, 0=60%, -1=30%
    //   Viridia    => (F) +1=25%, 0=50%, -1=25%
@@ -519,9 +491,8 @@ private:
    //   Whitill    => (F) +1=25%, 0=50%, -1=25%
    be_uint32_t special_upgrade_prob_table_offset; // [{c, o -> (DeltaProbabilityEntry)[10][c]})
-    // This table specifies how likely a weapon's grind is to be upgraded or
+    // This table specifies how likely a weapon's grind is to be upgraded or downgraded, and by how much. The final
-    // downgraded, and by how much. The final grind value is clamped to the
+    // grind value is clamped to the range between 0 and the weapon's maximum grind from ItemPMT, inclusive.
    // range between 0 and the weapon's maximum grind from ItemPMT, inclusive.
    // In PSO V3, the grind delta table is:
    //   Viridia    => (D) +3=3%,  +2=7%,  +1=13%, 0=60%, -1=10%, -2=7%,  -3=0%
    //   Viridia    => (F) +3=5%,  +2=13%, +1=25%, 0=50%, -1=7%,  -2=0%,  -3=0%
@@ -545,9 +516,8 @@ private:
    //   Whitill    => (F) +3=5%,  +2=13%, +1=25%, 0=50%, -1=7%,  -2=0%,  -3=0%
    be_uint32_t grind_delta_prob_table_offset; // [{c, o -> (DeltaProbabilityEntry)[10][c]})
-    // This table specifies how likely a weapon's bonuses are to be upgraded
+    // This table specifies how likely a weapon's bonuses are to be upgraded or downgraded, and by how much. The final
-    // or downgraded, and by how much. The final bonuses are capped above at
+    // bonuses are capped above at 100, but there is no lower limit (so negative results are possible).
    // 100, but there is no lower limit (so negative results are possible).
    // In PSO V3, the bonus delta table is:
    //   Viridia    => (D) +10=5%,  +5=15%, 0=60%, -5=15%, -10=5%
    //   Viridia    => (F) +10=8%,  +5=20%, 0=60%, -5=10%, -10=2%
@@ -571,11 +541,10 @@ private:
    //   Whitill    => (F) +10=8%,  +5=20%, 0=60%, -5=10%, -10=2%
    be_uint32_t bonus_delta_prob_table_offset; // [{c, o -> (DeltaProbabilityEntry)[10][c]})
-    // There is a secondary computation done during weapon adjustment that
+    // There is a secondary computation done during weapon adjustment that appears to determine how "good" the
-    // appears to determine how "good" the resulting weapon is compared to its
+    // resulting weapon is compared to its original state. If the result of this computation is positive, the game
-    // original state. If the result of this computation is positive, the game
+    // plays a jingle when the tekker result is accepted. These tables describe how much each delta affects this value,
-    // plays a jingle when the tekker result is accepted. These tables describe
+    // which we call luck.
    // how much each delta affects this value, which we call luck.
    // In PSO V3, the special upgrade luck table is:
    //   +1 => +20, 0 => 0, -1 => -20
@@ -63,14 +63,11 @@ struct WindowIndex {
    return match_iter - match_offset;
  };
-  // The data structure we want is a binary-searchable set of all strings
+  // The data structure we want is a binary-searchable set of all strings starting at all possible offsets within the
-  // starting at all possible offsets within the sliding window, and we need
+  // sliding window, and we need to be able to search lexicographically but insert and delete by offset. A
-  // to be able to search lexicographically but insert and delete by offset.
+  // std::map<std::string, size_t> would accomplish this, but would be horrendously inefficient: we'd have to copy
-  // A std::map<std::string, size_t> would accomplish this, but would be
+  // strings far too much. We can solve this by instead storing the offset of each string as keys in a set and using a
-  // horrendously inefficient: we'd have to copy strings far too much. We can
+  // custom comparator to treat them as references to binary strings within the data.
  // solve this by instead storing the offset of each string as keys in a set
  // and using a custom comparator to treat them as references to binary
  // strings within the data.
  bool set_comparator(size_t a, size_t b) const {
    size_t max_length = min<size_t>(MaxMatchLength, this->size - max<size_t>(a, b));
    size_t end_a = a + max_length;
@@ -87,11 +84,9 @@ struct WindowIndex {
  };
  pair<size_t, size_t> get_best_match() const {
-    // Find the best match from the index. It's unlikely that we'll get an
+    // Find the best match from the index. It's unlikely that we'll get an exact match, so check the entry before the
-    // exact match, so check the entry before the upper_bound result too.
+    // upper_bound result too. Note: We use upper_bound rather than lower_bound because in PRS, a backreference can be
-    // Note: We use upper_bound rather than lower_bound because in PRS, a
+    // encoded with fewer bits if it's close to the decompression offset, and this makes us pick the latest match by
    // backreference can be encoded with fewer bits if it's close to the
    // decompression offset, and this makes us pick the latest match by
    // default.
    size_t match_offset = 0;
    size_t match_size = 0;
@@ -123,9 +118,7 @@ struct LZSSInterleavedWriter {
  uint8_t next_control_bit;
  uint8_t buf[0x19];
-  LZSSInterleavedWriter()
+  LZSSInterleavedWriter() : buf_offset(1), next_control_bit(1) {
      : buf_offset(1),
        next_control_bit(1) {
    this->buf[0] = 0;
  }
@@ -166,9 +159,7 @@ struct LZSSInterleavedWriter {
 class ControlStreamReader {
 public:
-  ControlStreamReader(phosg::StringReader& r)
+  ControlStreamReader(phosg::StringReader& r) : r(r), bits(0x0000) {}
      : r(r),
        bits(0x0000) {}
  bool read() {
    if (!(this->bits & 0x0100)) {
@@ -285,8 +276,7 @@ string prs_compress_optimal(const void* in_data_v, size_t in_size, ProgressCallb
  long_window_thread.join();
  extended_window_thread.join();
-  // For each node, populate the literal value, and the best ways to get to the
+  // For each node, populate the literal value, and the best ways to get to the following nodes
  // following nodes
  for (size_t z = 0; z < in_size; z++) {
    if ((z & 0xFFF) == 0 && progress_fn) {
      progress_fn(CompressPhase::CONSTRUCT_PATHS, z, in_size, 0);
@@ -441,9 +431,8 @@ string prs_compress_optimal(const string& data, ProgressCallback progress_fn) {
 string prs_compress_pessimal(const void* vdata, size_t size) {
  const uint8_t* in_data = reinterpret_cast<const uint8_t*>(vdata);
-  // The worst possible encoding we can do is a literal byte when no byte with
+  // The worst possible encoding we can do is a literal byte when no byte with the same value is within the window, or
-  // the same value is within the window, or an extended copy if there is a byte
+  // an extended copy if there is a byte with the same value in the window.
  // with the same value in the window.
  WindowIndex<0x1FFF, 1> window(in_data, size);
  LZSSInterleavedWriter w;
  for (size_t z = 0; z < size; z++) {
@@ -539,9 +528,8 @@ void PRSCompressor::advance() {
        match_size++;
      }
-      // If there are multiple matches of the longest length, use the latest one,
+      // If there are multiple matches of the longest length, use the latest one, since it's more likely that it can be
-      // since it's more likely that it can be expressed as a short copy instead
+      // expressed as a short copy instead of a long copy.
      // of a long copy.
      if (match_size >= (best_match_size + best_match_literals)) {
        best_match_offset = match_offset;
        best_match_size = match_size;
@@ -558,15 +546,13 @@ void PRSCompressor::advance() {
    this->advance_literal();
  }
-  // If there is a suitable match, write a backreference; otherwise, write a
+  // If there is a match, write a backreference; otherwise, write a literal. The backreference should be encoded:
  // literal. The backreference should be encoded:
  // - As a short copy if offset in [-0x100, -1] and size in [2, 5]
  // - As a long copy if offset in [-0x1FFF, -1] and size in [3, 9]
  // - As an extended copy if offset in [-0x1FFF, -1] and size in [10, 0x100]
-  // Technically an extended copy can be used for sizes 1-9 as well, but if
+  // Technically an extended copy can be used for sizes 1-9 as well, but if size is 1 or 2, writing literals is better
-  // size is 1 or 2, writing literals is better (since it uses fewer data
+  // (since it uses fewer data bytes and control bits), and a long copy can cover sizes 3-9 (and also uses fewer data
-  // bytes and control bits), and a long copy can cover sizes 3-9 (and also
+  // bytes and control bits).
  // uses fewer data bytes and control bits).
  ssize_t backreference_offset = best_match_offset - this->reverse_log.end_offset();
  if (best_match_size < 2) {
    // The match is too small; a literal would use fewer bits
@@ -576,8 +562,8 @@ void PRSCompressor::advance() {
    this->advance_short_copy(backreference_offset, best_match_size);
  } else if (best_match_size < 3) {
-    // We can't use a long copy for size 2, and it's not worth it to use an
+    // We can't use a long copy for size 2, and it's not worth it to use an extended copy for this either (as noted
-    // extended copy for this either (as noted above), so write a literal
+    // above), so write a literal
    this->advance_literal();
  } else if ((backreference_offset >= -0x1FFF) && (best_match_size <= 9)) {
@@ -655,14 +641,12 @@ string& PRSCompressor::close() {
 void PRSCompressor::write_control(bool z) {
  if (this->pending_control_bits & 0x0100) {
-    this->output.pput_u8(
+    this->output.pput_u8(this->control_byte_offset, this->pending_control_bits & 0xFF);
        this->control_byte_offset, this->pending_control_bits & 0xFF);
    this->control_byte_offset = this->output.size();
    this->output.put_u8(0);
    this->pending_control_bits = z ? 0x8080 : 0x8000;
  } else {
-    this->pending_control_bits =
+    this->pending_control_bits = (this->pending_control_bits >> 1) | (z ? 0x8080 : 0x8000);
        (this->pending_control_bits >> 1) | (z ? 0x8080 : 0x8000);
  }
 }
@@ -671,8 +655,7 @@ void PRSCompressor::flush_control() {
    while (!(this->pending_control_bits & 0x0100)) {
      this->pending_control_bits >>= 1;
    }
-    this->output.pput_u8(
+    this->output.pput_u8(this->control_byte_offset, this->pending_control_bits & 0xFF);
        this->control_byte_offset, this->pending_control_bits & 0xFF);
  } else {
    if (this->control_byte_offset != this->output.size() - 1) {
      throw logic_error("data written without control bits");
@@ -681,25 +664,17 @@ void PRSCompressor::flush_control() {
  }
 }
-string prs_compress(
+string prs_compress(const void* vdata, size_t size, ssize_t compression_level, ProgressCallback progress_fn) {
    const void* vdata,
    size_t size,
    ssize_t compression_level,
    ProgressCallback progress_fn) {
  PRSCompressor prs(compression_level, progress_fn);
  prs.add(vdata, size);
  return std::move(prs.close());
 }
-string prs_compress(
+string prs_compress(const string& data, ssize_t compression_level, ProgressCallback progress_fn) {
    const string& data,
    ssize_t compression_level,
    ProgressCallback progress_fn) {
  return prs_compress(data.data(), data.size(), compression_level, progress_fn);
 }
-string prs_compress_indexed(
+string prs_compress_indexed(const void* in_data_v, size_t in_size, ProgressCallback progress_fn) {
    const void* in_data_v, size_t in_size, ProgressCallback progress_fn) {
  const uint8_t* in_data = reinterpret_cast<const uint8_t*>(in_data_v);
  LZSSInterleavedWriter w;
@@ -718,14 +693,11 @@ string prs_compress_indexed(
    auto m_long = w_long.get_best_match();
    auto m_extended = w_extended.get_best_match();
-    // Write the match that achieves the best ratio of output bytes to
+    // Write the match that achieves the best ratio of output bytes to compressed bits used. To do this without
-    // compressed bits used. To do this without floating-point math, we multiply
+    // floating-point math, we multiply the output byte count for each type of command by 468 / (command_bits), since
-    // the output byte count for each type of command by 468 / (command_bits),
+    // 468 is the least common multiple of the number of bits for each command type. The command type with the highest
-    // since 468 is the least common multiple of the number of bits for each
+    // score is the one we'll use, breaking ties by choosing the shorter command type. Note that the size of any copy
-    // command type. The command type with the highest score is the one we'll
+    // type can be zero if no match was found; if no matches were found at all, then we can always write a literal.
    // use, breaking ties by choosing the shorter command type. Note that the
    // size of any copy type can be zero if no match was found; if no matches
    // were found at all, then we can always write a literal.
    size_t score_literal = 52;
    size_t score_short = m_short.second * 39;
    size_t score_long = m_long.second * 26;
@@ -838,41 +810,30 @@ string prs_compress_indexed(const string& data, ProgressCallback progress_fn) {
 PRSDecompressResult prs_decompress_with_meta(
    const void* data, size_t size, size_t max_output_size, bool allow_unterminated) {
-  // PRS is an LZ77-based compression algorithm. Compressed data is split into
+  // PRS is an LZ77-based compression algorithm. Compressed data is split into two streams: a control stream and a data
-  // two streams: a control stream and a data stream. The control stream is read
+  // stream. The control stream is read one bit at a time, and the data stream is read one byte at a time. The streams
-  // one bit at a time, and the data stream is read one byte at a time. The
+  // are interleaved such that the decompressor never has to move backward in the input stream - when the decompressor
-  // streams are interleaved such that the decompressor never has to move
+  // needs a control bit and there are no unused bits from the previous byte of the control stream, it reads a byte
-  // backward in the input stream - when the decompressor needs a control bit
+  // from the input and treats it as the next 8 control bits.
  // and there are no unused bits from the previous byte of the control stream,
  // it reads a byte from the input and treats it as the next 8 control bits.
  // There are 3 distinct commands in PRS, labeled here with their control bits:
-  // 1 - Literal byte. The decompressor copies one byte from the input data
+  // 1  - Literal byte. The decompressor copies one byte from the input data stream to the output.
-  //     stream to the output.
+  // 00 - Short backreference. The decompressor reads two control bits and adds 2 to this value to determine the number
-  // 00 - Short backreference. The decompressor reads two control bits and adds
+  //      of bytes to copy, then reads one byte from the data stream to determine how far back in the output to copy
-  //      2 to this value to determine the number of bytes to copy, then reads
+  //      from. This byte is treated as an 8-bit negative number - so 0xF7, for example, means to start copying data
-  //      one byte from the data stream to determine how far back in the output
+  //      from 9 bytes before the end of the output. The range must start before the end of the output, but the end of
-  //      to copy from. This byte is treated as an 8-bit negative number - so
+  //      the range may be beyond the end of the output. In this case, the bytes between the beginning of the range and
-  //      0xF7, for example, means to start copying data from 9 bytes before the
+  //      original end of the output are simply repeated.
-  //      end of the output. The range must start before the end of the output,
+  // 01 - Long backreference. The decompressor reads two bytes from the data and byteswaps the resulting 16-bit value
-  //      but the end of the range may be beyond the end of the output. In this
+  //      (that is, the low byte is read first). The start offset (again, as a negative number) is the top 13 bits of
-  //      case, the bytes between the beginning of the range and original end of
+  //      this value; the size is the low 3 bits of this value, plus 2. If the size bits are all zero, an additional
-  //      the output are simply repeated.
+  //      byte is read from the data stream and 1 is added to it to determine the backreference size (we call this an
-  // 01 - Long backreference. The decompressor reads two bytes from the data and
+  //      extended backreference). Therefore, the maximum backreference size is 256 bytes.
-  //      byteswaps the resulting 16-bit value (that is, the low byte is read
+  // Decompression ends when either there are no more input bytes to read, or when a long backreference is read with
-  //      first). The start offset (again, as a negative number) is the top 13
+  // all zeroes in its offset field. The original implementation stops decompression successfully when any attempt to
-  //      bits of this value; the size is the low 3 bits of this value, plus 2.
+  // read from the input encounters the end of the stream, but newserv's implementation only allows this at the end of
-  //      If the size bits are all zero, an additional byte is read from the
+  // an opcode - if end-of-stream is encountered partway through an opcode, we throw instead, because it's likely the
-  //      data stream and 1 is added to it to determine the backreference size
+  // input has been truncated or is malformed in some way.
  //      (we call this an extended backreference). Therefore, the maximum
  //      backreference size is 256 bytes.
  // Decompression ends when either there are no more input bytes to read, or
  // when a long backreference is read with all zeroes in its offset field. The
  // original implementation stops decompression successfully when any attempt
  // to read from the input encounters the end of the stream, but newserv's
  // implementation only allows this at the end of an opcode - if end-of-stream
  // is encountered partway through an opcode, we throw instead, because it's
  // likely the input has been truncated or is malformed in some way.
  phosg::StringWriter w;
  phosg::StringReader r(data, size);
@@ -894,10 +855,9 @@ PRSDecompressResult prs_decompress_with_meta(
      ssize_t offset;
      size_t count;
      // Control 01 = long backreference
      if (cr.read()) {
-        // The bits stored in the data stream are AAAAABBBCCCCCCCC, which we
+        // Control 01 = long backreference
-        // rearrange into offset = CCCCCCCCAAAAA and size = BBB.
+        // The bits from the data stream are AAAAABBBCCCCCCCC, which we rearrange as offset=CCCCCCCCAAAAA and size=BBB.
        uint16_t a = r.get_u8();
        a |= (r.get_u8() << 8);
        offset = (a >> 3) | (~0x1FFF);
@@ -905,24 +865,21 @@ PRSDecompressResult prs_decompress_with_meta(
        if (offset == ~0x1FFF) {
          break;
        }
-        // If the size field is zero, it's an extended backreference (size comes
+        // If the size field is zero, it's an extended backreference (size comes from another byte in the data stream)
        // from another byte in the data stream)
        count = (a & 7) ? ((a & 7) + 2) : (r.get_u8() + 1);
        // Control 00 = short backreference
      } else {
-        // Count comes from 2 bits in the control stream instead of from the
+        // Control 00 = short backreference
-        // data stream (and 2 is added). Importantly, the control stream bits
+        // Count comes from 2 bits in the control stream instead of from the data stream (and 2 is added). Importantly,
-        // are read first - this may involve reading another control stream
+        // the control stream bits are read first - this may involve reading another control stream byte, which happens
-        // byte, which happens before the offset is read from the data stream.
+        // before the offset is read from the data stream.
        count = cr.read() << 1;
        count = (count | cr.read()) + 2;
        offset = r.get_u8() | (~0xFF);
      }
-      // Copy bytes from the referenced location in the output. Importantly,
+      // Copy bytes from the referenced location in the output. Importantly, copy only one byte at a time, in order to
-      // copy only one byte at a time, in order to support ranges that cover the
+      // support ranges that cover the current end of the output.
      // current end of the output.
      size_t read_offset = w.size() + offset;
      if (read_offset >= w.size()) {
        throw runtime_error("backreference offset beyond beginning of output");
@@ -1069,11 +1026,10 @@ void prs_disassemble(FILE* stream, const std::string& data) {
  return prs_disassemble(stream, data.data(), data.size());
 }
-// BC0 is a compression algorithm fairly similar to PRS, but with a simpler set
+// BC0 is a compression algorithm fairly similar to PRS, but with a simpler set of commands. Like PRS, there is a
-// of commands. Like PRS, there is a control stream, indicating when to copy a
+// control stream, indicating when to copy a literal byte from the input and when to copy from a backreference; unlike
-// literal byte from the input and when to copy from a backreference; unlike
+// PRS, there is only one type of backreference. Also, there is no stop opcode; the decompressor simply stops when
-// PRS, there is only one type of backreference. Also, there is no stop opcode;
+// there are no more input bytes to read.
 // the decompressor simply stops when there are no more input bytes to read.
 struct BC0PathNode {
  uint16_t memo_offset = 0;
@@ -1112,8 +1068,7 @@ string bc0_compress_optimal(
    }
  }
-  // For each node, populate the literal value, and the best ways to get to the
+  // For each node, populate the literal value, and the best ways to get to the following nodes
  // following nodes
  for (size_t z = 0; z < in_size; z++) {
    if ((z & 0xFFF) == 0 && progress_fn) {
      progress_fn(CompressPhase::CONSTRUCT_PATHS, z, in_size, 0);
@@ -1238,11 +1193,9 @@ string bc0_encode(const void* in_data_v, size_t in_size) {
  return std::move(w.close());
 }
-// The BC0 decompression implementation in PSO GC is vulnerable to overflow
+// The BC0 decompression implementation in PSO GC is vulnerable to overflow attacks - there is no bounds checking on
-// attacks - there is no bounds checking on the output buffer. It is unlikely
+// the output buffer. It is unlikely that this can be usefully exploited (e.g. for RCE) because the output pointer is
-// that this can be usefully exploited (e.g. for RCE) because the output pointer
+// loaded from memory before every byte is written, so we cannot change the output pointer to any arbitrary address.
 // is loaded from memory before every byte is written, so we cannot change the
 // output pointer to any arbitrary address.
 string bc0_decompress(const string& data) {
  return bc0_decompress(data.data(), data.size());
@@ -1252,22 +1205,18 @@ string bc0_decompress(const void* data, size_t size) {
  phosg::StringReader r(data, size);
  phosg::StringWriter w;
-  // Unlike PRS, BC0 uses a memo which "rolls over" every 0x1000 bytes. The
+  // Unlike PRS, BC0 uses a memo which "rolls over" every 0x1000 bytes. The boundaries of these "memo pages" are offset
-  // boundaries of these "memo pages" are offset by -0x12 bytes for some reason,
+  // by -0x12 bytes for some reason, so the first output byte corresponds to position 0xFEE on the first memo page.
-  // so the first output byte corresponds to position 0xFEE on the first memo
+  // Backreferences refer to offsets based on the start of memo pages; for example, if the current output offset is
-  // page. Backreferences refer to offsets based on the start of memo pages; for
+  // 0x1234, a backreference with offset 0x123 refers to the byte that was written at offset 0x1111 (because that byte
-  // example, if the current output offset is 0x1234, a backreference with
+  // is at offset 0x111 in the memo, because the memo rolls over every 0x1000 bytes and the first memo byte was 0x12
-  // offset 0x123 refers to the byte that was written at offset 0x1111 (because
+  // bytes before the beginning of the next page). The memo is initially zeroed from 0 to 0xFEE; it seems PSO GC
-  // that byte is at offset 0x111 in the memo, because the memo rolls over every
+  // doesn't initialize the last 0x12 bytes of the first memo page.
  // 0x1000 bytes and the first memo byte was 0x12 bytes before the beginning of
  // the next page). The memo is initially zeroed from 0 to 0xFEE; it seems PSO
  // GC doesn't initialize the last 0x12 bytes of the first memo page.
  parray<uint8_t, 0x1000> memo;
  uint16_t memo_offset = 0x0FEE;
-  // The low byte of this value contains the control stream data; the high bits
+  // The low byte of this value contains the control stream data; the high bits specify which low bits are valid. When
-  // specify which low bits are valid. When the last 1 is shifted out of the
+  // the last 1 is shifted out of the high byte, we need to read a new control stream byte to get the next set of
  // high byte, we need to read a new control stream byte to get the next set of
  // control bits.
  uint16_t control_stream_bits = 0x0000;
@@ -1282,14 +1231,13 @@ string bc0_decompress(const void* data, size_t size) {
    }
    if ((control_stream_bits & 1) == 0) {
-      // Control bit 0 means to perform a backreference copy. The offset and
+      // Control bit 0 means to perform a backreference copy. The offset and size are stored in two bytes in the input
-      // size are stored in two bytes in the input stream, laid out as follows:
+      // stream, laid out as follows:
      //   a1 = 0bBBBBBBBB
      //   a2 = 0bAAAACCCC
-      // The offset is the concatenation of bits AAAABBBBBBBB, which refers to
+      // The offset is the concatenation of bits AAAABBBBBBBB, which refers to a position in the memo; the number of
-      // a position in the memo; the number of bytes to copy is (CCCC + 3). The
+      // bytes to copy is (CCCC + 3). The decompressor copies that many bytes from that offset in the memo, and writes
-      // decompressor copies that many bytes from that offset in the memo, and
+      // them to the output and to the current position in the memo.
      // writes them to the output and to the current position in the memo.
      uint8_t a1 = r.get_u8();
      if (r.eof()) {
        break;
@@ -1305,8 +1253,8 @@ string bc0_decompress(const void* data, size_t size) {
      }
    } else {
-      // Control bit 1 means to write a byte directly from the input to the
+      // Control bit 1 means to write a byte directly from the input to the output. As above, the byte is also written
-      // output. As above, the byte is also written to the memo.
+      // to the memo.
      uint8_t v = r.get_u8();
      w.put_u8(v);
      memo[memo_offset] = v;
@@ -22,39 +22,32 @@ const char* phosg::name_for_enum<CompressPhase>(CompressPhase v);
 typedef std::function<void(CompressPhase phase, size_t input_progress, size_t input_size, size_t output_size)> ProgressCallback;
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // PRS compression
 ////////////////////////////////////////////////////////////////////////////////
-// Use this class if you need to compress from multiple input buffers, or need
+// Use this class if you need to compress from multiple input buffers, or need to compress multiple chunks and don't
-// to compress multiple chunks and don't want to copy their contents
+// want to copy their contents unnecessarily. (For most common use cases, use prs_compress, below, instead.) To use
-// unnecessarily. (For most common use cases, use prs_compress, below, instead.)
+// this class, instantiate it, then call .add() one or more times, then call .close() and use the returned string as
-// To use this class, instantiate it, then call .add() one or more times, then
+// the compressed result.
 // call .close() and use the returned string as the compressed result.
 class PRSCompressor {
 public:
  // compression_level specifies how aggressively to search for alternate paths:
-  //   -1: Don't perform any compression at all, but produce output that can be
+  //   -1: Don't perform any compression at all, but produce output that can be understood by prs_decompress. The
-  //       understood by prs_decompress. The output will be about 9/8 the size
+  //       output will be about 9/8 the size of the input.
-  //       of the input.
+  //   0:  Greedily search for the longest backreference at every point. Don't consider any alternate paths. Generally
-  //   0:  Greedily search for the longest backreference at every point. Don't
+  //       offers a good balance between speed and output size.
-  //       consider any alternate paths. Generally offers a good balance between
+  //   1:  Consider two paths at each point when a backreference is found: using the backreference or ignoring it.
-  //       speed and output size.
+  //   2+: Consider further chains of paths at each point. Using values 2 or greater for compression_level generally
-  //   1:  Consider two paths at each point when a backreference is found: using
+  //       yields diminishing returns.
  //       the backreference or ignoring it.
  //   2+: Consider further chains of paths at each point. Using values 2 or
  //       greater for compression_level generally yields diminishing returns.
  explicit PRSCompressor(ssize_t compression_level = 0, ProgressCallback progress_fn = nullptr);
  ~PRSCompressor() = default;
-  // Adds more input data to be compressed, which logically comes after all
+  // Adds more input data to be compressed, which logically comes after all previous data provided via add() calls.
-  // previous data provided via add() calls. Cannot be called after close() is
+  // Cannot be called after close() is called.
  // called.
  void add(const void* data, size_t size);
  void add(const std::string& data);
-  // Ends compression and returns the complete compressed result. It's OK to
+  // Ends compression and returns the complete compressed result. It's OK to std::move() from the returned reference.
  // std::move() from the returned string reference.
  std::string& close();
  // Returns the total number of bytes passed to add() calls so far.
@@ -149,36 +142,24 @@ private:
  phosg::StringWriter output;
 };
-// These functions use PRSCompressor to compress a buffer of data. This is
+// These functions use PRSCompressor to compress a buffer of data. This is essentially a shortcut for constructing a
-// essentially a shortcut for constructing a PRSCompressor, calling .add() on
+// PRSCompressor, calling .add() on it once, then calling .close().
 // it once, then calling .close().
 std::string prs_compress(
-    const void* vdata,
+    const void* vdata, size_t size, ssize_t compression_level = 0, ProgressCallback progress_fn = nullptr);
    size_t size,
    ssize_t compression_level = 0,
    ProgressCallback progress_fn = nullptr);
 std::string prs_compress(
-    const std::string& data,
+    const std::string& data, ssize_t compression_level = 0, ProgressCallback progress_fn = nullptr);
    ssize_t compression_level = 0,
    ProgressCallback progress_fn = nullptr);
 // A faster form of prs_compress that doesn't have a tunable compression level.
-std::string prs_compress_indexed(
+std::string prs_compress_indexed(const void* vdata, size_t size, ProgressCallback progress_fn = nullptr);
-    const void* vdata,
+std::string prs_compress_indexed(const std::string& data, ProgressCallback progress_fn = nullptr);
    size_t size,
    ProgressCallback progress_fn = nullptr);
 std::string prs_compress_indexed(
    const std::string& data,
    ProgressCallback progress_fn = nullptr);
-// Compresses data using PRS to the smallest possible output size. This function
+// Compresses data using PRS to the smallest possible output size. This function is slow, but produces results
-// is slow, but produces results significantly smaller than even Sega's original
+// significantly smaller than even Sega's original compressor.
 // compressor.
 std::string prs_compress_optimal(const void* vdata, size_t size, ProgressCallback progress_fn = nullptr);
 std::string prs_compress_optimal(const std::string& data, ProgressCallback progress_fn = nullptr);
-// Compresses data using PRS to the LARGEST possible output size. There is no
+// Compresses data using PRS to the LARGEST possible output size. There is no practical use for this function except
-// practical use for this function except for amusement.
+// for amusement.
 std::string prs_compress_pessimal(const void* vdata, size_t size);
 // Decompresses PRS-compressed data.
@@ -186,13 +167,14 @@ struct PRSDecompressResult {
  std::string data;
  size_t input_bytes_used;
 };
-PRSDecompressResult prs_decompress_with_meta(const void* data, size_t size, size_t max_output_size = 0, bool allow_unterminated = false);
+PRSDecompressResult prs_decompress_with_meta(
-PRSDecompressResult prs_decompress_with_meta(const std::string& data, size_t max_output_size = 0, bool allow_unterminated = false);
+    const void* data, size_t size, size_t max_output_size = 0, bool allow_unterminated = false);
 PRSDecompressResult prs_decompress_with_meta(
    const std::string& data, size_t max_output_size = 0, bool allow_unterminated = false);
 std::string prs_decompress(const void* data, size_t size, size_t max_output_size = 0, bool allow_unterminated = false);
 std::string prs_decompress(const std::string& data, size_t max_output_size = 0, bool allow_unterminated = false);
-// Returns the decompressed size of PRS-compressed data, without actually
+// Returns the decompressed size of PRS-compressed data, without actually decompressing it.
 // decompressing it.
 size_t prs_decompress_size(const void* data, size_t size, size_t max_output_size = 0, bool allow_unterminated = false);
 size_t prs_decompress_size(const std::string& data, size_t max_output_size = 0, bool allow_unterminated = false);
@@ -200,21 +182,16 @@ size_t prs_decompress_size(const std::string& data, size_t max_output_size = 0,
 void prs_disassemble(FILE* stream, const void* data, size_t size);
 void prs_disassemble(FILE* stream, const std::string& data);
-////////////////////////////////////////////////////////////////////////////////
+///////////////////////////////////////////////////////////////////////////////////////////////////////////////////////
 // BC0 compression
 ////////////////////////////////////////////////////////////////////////////////
-// Compresses data using the BC0 algorithm. Like with PRS, the optimal variant
+// Compresses data using the BC0 algorithm. Like with PRS, the optimal variant is slow, but produces the smallest
-// is slow, but produces the smallest possible output.
+// possible output.
 std::string bc0_compress_optimal(
    const void* in_data_v,
    size_t in_size,
    ProgressCallback progress_fn = nullptr);
 std::string bc0_compress(const std::string& data, ProgressCallback progress_fn = nullptr);
 std::string bc0_compress(const void* in_data_v, size_t in_size, ProgressCallback progress_fn = nullptr);
 std::string bc0_compress_optimal(const void* in_data_v, size_t in_size, ProgressCallback progress_fn = nullptr);
-// Encodes data in a BC0-compatible format without compression (similar to using
+// Encodes data in a BC0-compatible format without compression (similar to compression_level=-1 in prs_compress).
 // compression_level=-1 with prs_compress).
 std::string bc0_encode(const void* in_data_v, size_t in_size);
 // Decompresses BC0-compressed data.
@@ -458,7 +458,7 @@ public:
  // If the map file has no random sections, does nothing and returns a shared_ptr to this. If it has any random
  // sections, returns a new map with all non-random sections copied verbatim, and random sections replaced with
-  // non-random sections according to the challenge mode generation algorithm.
+  // non-random sections according to the challenge mode enemy generation algorithm.
  std::shared_ptr<MapFile> materialize_random_sections(uint32_t random_seed);
  std::shared_ptr<const MapFile> materialize_random_sections(uint32_t random_seed) const;