handle V2 mag encoding properly

src/ItemData.cc

@@ -81,12 +81,6 @@ void ItemData::clear() {
   this->data2d = 0;
 }
 
-void ItemData::bswap_data2_if_mag() {
-  if (this->data1[0] == 0x02) {
-    this->data2d = bswap32(this->data2d);
-  }
-}
-
 bool ItemData::empty() const {
   return (this->data1d[0] == 0) &&
       (this->data1d[1] == 0) &&
@@ -323,6 +317,61 @@ void ItemData::add_mag_photon_blast(uint8_t pb_num) {
   }
 }
 
+void ItemData::decode_if_mag(GameVersion from_version) {
+  if (this->data1[0] != 2) {
+    return;
+  }
+
+  if (from_version == GameVersion::GC) {
+    // PSO GC erroneously byteswaps the data2d field, even though it's actually
+    // just four individual bytes, so we correct for that here.
+    this->data2d = bswap32(this->data2d);
+
+  } else if (from_version == GameVersion::DC || from_version == GameVersion::PC) {
+    // PSO PC encodes mags in a tediously annoying manner. The first four bytes are the same, but then...
+    // V2: pHHHHHHHHHHHHHHc pIIIIIIIIIIIIIIc JJJJJJJJJJJJJJJc KKKKKKKKKKKKKKKc QQQQQQQQ QQQQQQQQ YYYYYYYY pYYYYYYY
+    // V3: HHHHHHHHHHHHHHHH IIIIIIIIIIIIIIII JJJJJJJJJJJJJJJJ KKKKKKKKKKKKKKKK YYYYYYYY QQQQQQQQ PPPPPPPP CCCCCCCC
+    // c = color in V2 (4 bits; low bit first)
+    // C = color in V3
+    // p = PB flag bits in V2 (3 bits; ordered 1, 2, 0)
+    // P = PB flag bits in V3
+    // H, I, J, K = DEF, POW, DEX, MIND
+    // Q = IQ (little-endian in V2)
+    // Y = synchro (little-endian in V2)
+
+    // Order is important; data2[0] must not be written before data2w[0] is read
+    this->data2[1] = this->data2w[0]; // IQ
+    this->data2[0] = this->data2w[1] & 0x7FFF; // Synchro
+    this->data2[2] = ((this->data2[3] >> 7) & 1) | ((this->data1w[2] >> 14) & 2) | ((this->data1w[3] >> 13) & 4); // PB flags
+    this->data2[3] = (this->data1w[2] & 1) | ((this->data1w[3] & 1) << 1) | ((this->data1w[4] & 1) << 2) | ((this->data1w[5] & 1) << 3); // Color
+    this->data1w[2] &= 0x7FFE;
+    this->data1w[3] &= 0x7FFE;
+    this->data1w[4] &= 0xFFFE;
+    this->data1w[5] &= 0xFFFE;
+  }
+}
+
+void ItemData::encode_if_mag(GameVersion to_version) {
+  if (this->data1[0] != 2) {
+    return;
+  }
+
+  // This function is the inverse of decode_v2_mag; see that function for a
+  // description of what's going on here.
+  if (to_version == GameVersion::GC) {
+    this->data2d = bswap32(this->data2d);
+
+  } else if (to_version == GameVersion::DC || to_version == GameVersion::PC) {
+    this->data1w[2] = (this->data1w[2] & 0x7FFE) | ((this->data2[2] << 14) & 0x8000) | (this->data2[3] & 1);
+    this->data1w[3] = (this->data1w[3] & 0x7FFE) | ((this->data2[2] << 13) & 0x8000) | ((this->data2[3] >> 1) & 1);
+    this->data1w[4] = (this->data1w[4] & 0xFFFE) | ((this->data2[3] >> 2) & 1);
+    this->data1w[5] = (this->data1w[5] & 0xFFFE) | ((this->data2[3] >> 3) & 1);
+    // Order is important; data2w[0] must not be written before data2[0] is read
+    this->data2w[1] = this->data2[0] | ((this->data2[2] << 15) & 0x8000);
+    this->data2w[0] = this->data2[1];
+  }
+}
+
 uint16_t ItemData::get_sealed_item_kill_count() const {
   return ((this->data1[10] << 8) | this->data1[11]) & 0x7FFF;
 }

src/ItemData.hh

@@ -4,6 +4,7 @@
 #include <string>
 
 #include "Text.hh"
+#include "Version.hh"
 
 constexpr uint32_t MESETA_IDENTIFIER = 0x00040000;
 
@@ -82,6 +83,10 @@ struct ItemData { // 0x14 bytes
   // makes it incompatible with little-endian versions of PSO (i.e. all other
   // versions). We manually byteswap data2 upon receipt and immediately before
   // sending where needed.
+  // Related note: PSO V2 has an annoyingly complicated format for mags that
+  // doesn't match the above table. We decode this upon receipt and encode it
+  // imemdiately before sending when interacting with V2 clients; see the
+  // implementation of decode_if_mag() for details.
 
   union {
     parray<uint8_t, 12> data1;
@@ -107,8 +112,6 @@ struct ItemData { // 0x14 bytes
 
   void clear();
 
-  void bswap_data2_if_mag();
-
   std::string hex() const;
   std::string name(bool include_color_codes) const;
   uint32_t primary_identifier() const;
@@ -131,6 +134,8 @@ struct ItemData { // 0x14 bytes
   uint8_t mag_photon_blast_for_slot(uint8_t slot) const;
   bool mag_has_photon_blast_in_any_slot(uint8_t pb_num) const;
   void add_mag_photon_blast(uint8_t pb_num);
+  void decode_if_mag(GameVersion version);
+  void encode_if_mag(GameVersion version);
 
   uint16_t get_sealed_item_kill_count() const;
   void set_sealed_item_kill_count(uint16_t v);

src/PlayerSubordinates.cc

@@ -471,6 +471,18 @@ size_t PlayerInventory::remove_all_items_of_type(uint8_t data1_0, int16_t data1_
   return ret;
 }
 
+void PlayerInventory::decode_mags(GameVersion version) {
+  for (size_t z = 0; z < this->items.size(); z++) {
+    this->items[z].data.decode_if_mag(version);
+  }
+}
+
+void PlayerInventory::encode_mags(GameVersion version) {
+  for (size_t z = 0; z < this->items.size(); z++) {
+    this->items[z].data.encode_if_mag(version);
+  }
+}
+
 size_t PlayerBank::find_item(uint32_t item_id) {
   for (size_t x = 0; x < this->num_items; x++) {
     if (this->items[x].data.id == item_id) {

src/PlayerSubordinates.hh

@@ -77,6 +77,9 @@ struct PlayerInventory {
   size_t find_equipped_mag() const;
 
   size_t remove_all_items_of_type(uint8_t data0, int16_t data1 = -1);
+
+  void decode_mags(GameVersion version);
+  void encode_mags(GameVersion version);
 } __attribute__((packed));
 
 struct PlayerBank {

src/ReceiveCommands.cc

@@ -2656,11 +2656,6 @@ static void on_61_98(shared_ptr<Client> c, uint16_t command, uint32_t flag, cons
       }
 
       player->inventory = cmd->inventory;
-      if (c->version() == GameVersion::GC) {
-        for (size_t z = 0; z < 30; z++) {
-          player->inventory.items[z].data.bswap_data2_if_mag();
-        }
-      }
       player->disp = cmd->disp.to_bb();
       player->battle_records = cmd->records.battle;
       player->challenge_records = cmd->records.challenge;
@@ -2693,6 +2688,7 @@ static void on_61_98(shared_ptr<Client> c, uint16_t command, uint32_t flag, cons
     default:
       throw logic_error("player data command not implemented for version");
   }
+  player->inventory.decode_mags(c->version());
 
   string name_str = remove_language_marker(encode_sjis(player->disp.name));
   c->channel.name = string_printf("C-%" PRIX64 " (%s)", c->id, name_str.c_str());
@@ -3742,9 +3738,7 @@ static void on_D0_V3_BB(shared_ptr<Client> c, uint16_t, uint32_t, const string&
   c->game_data.pending_item_trade->other_client_id = cmd.target_client_id;
   for (size_t x = 0; x < cmd.item_count; x++) {
     auto& item = c->game_data.pending_item_trade->items.emplace_back(cmd.item_datas[x]);
-    if (c->version() == GameVersion::GC) {
+    item.decode_if_mag(c->version());
-      item.bswap_data2_if_mag();
-    }
   }
 
   // If the other player has a pending trade as well, assume this is the second

src/ReceiveSubcommands.cc

@@ -218,7 +218,10 @@ static void on_sync_joining_player_item_state(shared_ptr<Client> c, uint8_t comm
       }
 
       for (size_t z = 0; z < num_floor_items; z++) {
-        decompressed_cmd->items[z].item_data.bswap_data2_if_mag();
+        // NOTE: If we use this codepath for non-V3 in the future, we'll need to
+        // change this hardcoded version. This only works because GC's mag
+        // encoding/decoding is symmetric (encode and decode do the same thing).
+        decompressed_cmd->items[z].item_data.decode_if_mag(GameVersion::GC);
       }
 
       string out_compressed_data = bc0_compress(decompressed);
@@ -282,7 +285,10 @@ static void on_sync_joining_player_disp_and_inventory(
   } else {
     auto out_cmd = check_size_t<G_SyncPlayerDispAndInventory_DC_PC_V3_6x70>(data, size);
     for (size_t z = 0; z < 30; z++) {
-      out_cmd.inventory.items[z].data.bswap_data2_if_mag();
+      // NOTE: If we use this codepath for non-V3 in the future, we'll need to
+      // change this hardcoded version. This only works because GC's mag
+      // encoding/decoding is symmetric (encode and decode do the same thing).
+      out_cmd.inventory.items[z].data.decode_if_mag(GameVersion::GC);
     }
     send_command_t(target, command, flag, out_cmd);
   }
@@ -696,11 +702,11 @@ static void on_player_drop_item(shared_ptr<Client> c, uint8_t command, uint8_t f
 }
 
 template <typename CmdT>
-void forward_subcommand_with_mag_bswap_t(shared_ptr<Client> c, uint8_t command, uint8_t flag, const CmdT& cmd) {
+void forward_subcommand_with_mag_transcode_t(shared_ptr<Client> c, uint8_t command, uint8_t flag, const CmdT& cmd) {
   // I'm lazy and this should never happen for item commands (since all players
   // need to stay in sync)
   if (command_is_private(command)) {
-    throw runtime_error("6x2B sent via private command");
+    throw runtime_error("item subcommand sent via private command");
   }
 
   auto l = c->require_lobby();
@@ -709,8 +715,9 @@ void forward_subcommand_with_mag_bswap_t(shared_ptr<Client> c, uint8_t command,
       continue;
     }
     CmdT out_cmd = cmd;
-    if ((c->version() == GameVersion::GC) != (other_c->version() == GameVersion::GC)) {
+    if (c->version() != other_c->version()) {
-      out_cmd.item_data.bswap_data2_if_mag();
+      out_cmd.item_data.decode_if_mag(c->version());
+      out_cmd.item_data.encode_if_mag(other_c->version());
     }
     send_command_t(other_c, command, flag, out_cmd);
   }
@@ -734,9 +741,7 @@ static void on_create_inventory_item_t(shared_ptr<Client> c, uint8_t command, ui
     auto p = c->game_data.player();
     {
       ItemData item = cmd.item_data;
-      if (c->version() == GameVersion::GC) {
+      item.decode_if_mag(c->version());
-        item.bswap_data2_if_mag();
-      }
       p->add_item(item);
     }
 
@@ -750,7 +755,7 @@ static void on_create_inventory_item_t(shared_ptr<Client> c, uint8_t command, ui
     p->print_inventory(stderr);
   }
 
-  forward_subcommand_with_mag_bswap_t(c, command, flag, cmd);
+  forward_subcommand_with_mag_transcode_t(c, command, flag, cmd);
 }
 
 static void on_create_inventory_item(shared_ptr<Client> c, uint8_t command, uint8_t flag, const void* data, size_t size) {
@@ -781,9 +786,7 @@ static void on_drop_partial_stack_t(shared_ptr<Client> c, uint8_t command, uint8
     // send an appropriate 6x29 alongside this?
     {
       ItemData item = cmd.item_data;
-      if (c->version() == GameVersion::GC) {
+      item.decode_if_mag(c->version());
-        item.bswap_data2_if_mag();
-      }
       l->add_item(item, cmd.area, cmd.x, cmd.z);
     }
 
@@ -798,7 +801,7 @@ static void on_drop_partial_stack_t(shared_ptr<Client> c, uint8_t command, uint8
     c->game_data.player()->print_inventory(stderr);
   }
 
-  forward_subcommand_with_mag_bswap_t(c, command, flag, cmd);
+  forward_subcommand_with_mag_transcode_t(c, command, flag, cmd);
 }
 
 static void on_drop_partial_stack(shared_ptr<Client> c, uint8_t command, uint8_t flag, const void* data, size_t size) {
@@ -874,9 +877,7 @@ static void on_buy_shop_item(shared_ptr<Client> c, uint8_t command, uint8_t flag
     auto p = c->game_data.player();
     {
       ItemData item = cmd.item_data;
-      if (c->version() == GameVersion::GC) {
+      item.decode_if_mag(c->version());
-        item.bswap_data2_if_mag();
-      }
       p->add_item(item);
     }
 
@@ -892,7 +893,7 @@ static void on_buy_shop_item(shared_ptr<Client> c, uint8_t command, uint8_t flag
     p->print_inventory(stderr);
   }
 
-  forward_subcommand_with_mag_bswap_t(c, command, flag, cmd);
+  forward_subcommand_with_mag_transcode_t(c, command, flag, cmd);
 }
 
 template <typename CmdT>
@@ -910,9 +911,7 @@ static void on_box_or_enemy_item_drop_t(shared_ptr<Client> c, uint8_t command, u
   if (l->flags & Lobby::Flag::ITEM_TRACKING_ENABLED) {
     {
       ItemData item = cmd.item_data;
-      if (c->version() == GameVersion::GC) {
+      item.decode_if_mag(c->version());
-        item.bswap_data2_if_mag();
-      }
       l->add_item(item, cmd.area, cmd.x, cmd.z);
     }
 
@@ -925,7 +924,7 @@ static void on_box_or_enemy_item_drop_t(shared_ptr<Client> c, uint8_t command, u
     }
   }
 
-  forward_subcommand_with_mag_bswap_t(c, command, flag, cmd);
+  forward_subcommand_with_mag_transcode_t(c, command, flag, cmd);
 }
 
 static void on_box_or_enemy_item_drop(shared_ptr<Client> c, uint8_t command, uint8_t flag, const void* data, size_t size) {

src/SendCommands.cc

@@ -1448,9 +1448,7 @@ static void send_join_spectator_team(shared_ptr<Client> c, shared_ptr<Lobby> l)
       p.lobby_data.name = wc_p->disp.name;
       remove_language_marker_inplace(p.lobby_data.name);
       p.inventory = wc_p->inventory;
-      for (size_t y = 0; y < 30; y++) {
+      p.inventory.encode_mags(c->version());
-        p.inventory.items[y].data.bswap_data2_if_mag();
-      }
       p.disp = wc_p->disp.to_dcpcv3();
       remove_language_marker_inplace(p.disp.visual.name);
 
@@ -1490,9 +1488,7 @@ static void send_join_spectator_team(shared_ptr<Client> c, shared_ptr<Lobby> l)
       p.lobby_data = entry.lobby_data;
       remove_language_marker_inplace(p.lobby_data.name);
       p.inventory = entry.inventory;
-      for (size_t z = 0; z < 30; z++) {
+      p.inventory.encode_mags(c->version());
-        p.inventory.items[z].data.bswap_data2_if_mag();
-      }
       p.disp = entry.disp;
       remove_language_marker_inplace(p.disp.visual.name);
 
@@ -1630,9 +1626,7 @@ void send_join_game(shared_ptr<Client> c, shared_ptr<Lobby> l) {
           if (l->clients[x]) {
             auto other_p = l->clients[x]->game_data.player();
             cmd.players_ep3[x].inventory = other_p->inventory;
-            for (size_t z = 0; z < 30; z++) {
+            cmd.players_ep3[x].inventory.encode_mags(c->version());
-              cmd.players_ep3[x].inventory.items[z].data.bswap_data2_if_mag();
-            }
             cmd.players_ep3[x].disp = convert_player_disp_data<PlayerDispDataDCPCV3>(other_p->disp);
           }
         }
@@ -1746,11 +1740,7 @@ void send_join_lobby_t(shared_ptr<Client> c, shared_ptr<Lobby> l,
       add_language_marker_inplace(e.lobby_data.name, 'J');
     }
     e.inventory = lp->inventory;
-    if (c->version() == GameVersion::GC) {
+    e.inventory.encode_mags(c->version());
-      for (size_t z = 0; z < 30; z++) {
-        e.inventory.items[z].data.bswap_data2_if_mag();
-      }
-    }
     e.disp = convert_player_disp_data<DispDataT>(lp->disp);
     e.disp.enforce_lobby_join_limits(c->version());
   }
@@ -1796,6 +1786,7 @@ void send_join_lobby_dc_nte(shared_ptr<Client> c, shared_ptr<Lobby> l,
     e.lobby_data.client_id = lc->lobby_client_id;
     e.lobby_data.name = lp->disp.name;
     e.inventory = lp->inventory;
+    e.inventory.encode_mags(c->version());
     e.disp = convert_player_disp_data<PlayerDispDataDCPCV3>(lp->disp);
     e.disp.enforce_lobby_join_limits(c->version());
   }
@@ -1898,8 +1889,7 @@ void send_get_player_info(shared_ptr<Client> c) {
 ////////////////////////////////////////////////////////////////////////////////
 // Trade window
 
-void send_execute_item_trade(shared_ptr<Client> c,
+void send_execute_item_trade(shared_ptr<Client> c, const vector<ItemData>& items) {
-    const vector<ItemData>& items) {
   SC_TradeItems_D0_D3 cmd;
   if (items.size() > cmd.item_datas.size()) {
     throw logic_error("too many items in execute trade command");
@@ -1908,9 +1898,7 @@ void send_execute_item_trade(shared_ptr<Client> c,
   cmd.item_count = items.size();
   for (size_t x = 0; x < items.size(); x++) {
     cmd.item_datas[x] = items[x];
-    if (c->version() == GameVersion::GC) {
+    cmd.item_datas[x].encode_if_mag(c->version());
-      cmd.item_datas[x].bswap_data2_if_mag();
-    }
   }
   send_command_t(c, 0xD3, 0x00, cmd);
 }
@@ -2039,9 +2027,7 @@ void send_drop_item(Channel& ch, const ItemData& item,
     bool from_enemy, uint8_t area, float x, float z, uint16_t entity_id) {
   G_DropItem_PC_V3_BB_6x5F cmd = {
       {{0x5F, 0x0B, 0x0000}, area, from_enemy, entity_id, x, z, 0, 0, item}, 0};
-  if (ch.version == GameVersion::GC) {
+  cmd.item_data.encode_if_mag(ch.version);
-    cmd.item_data.bswap_data2_if_mag();
-  }
   ch.send(0x60, 0x00, &cmd, sizeof(cmd));
 }
 
@@ -2059,9 +2045,7 @@ void send_drop_stacked_item(Channel& ch, const ItemData& item,
     uint8_t area, float x, float z) {
   G_DropStackedItem_PC_V3_BB_6x5D cmd = {
       {{0x5D, 0x0A, 0x0000}, area, 0, x, z, item}, 0};
-  if (ch.version == GameVersion::GC) {
+  cmd.item_data.encode_if_mag(ch.version);
-    cmd.item_data.bswap_data2_if_mag();
-  }
   ch.send(0x60, 0x00, &cmd, sizeof(cmd));
 }