From a5120bb9d696af3ef091df36f5fe30e5a549e7db Mon Sep 17 00:00:00 2001 From: thedax Date: Sun, 13 Jul 2025 20:11:54 -0400 Subject: [PATCH] Fix client-side crashes when discarding items. (#117) -Various things were wrong in InventoryTransaction, but they should now be fixed. --- src/bin/kawari-world.rs | 46 ++++++++++++------------------------ src/inventory/storage.rs | 2 ++ src/ipc/zone/mod.rs | 50 ++++++++++++++++++++++------------------ src/lib.rs | 3 +++ 4 files changed, 47 insertions(+), 54 deletions(-) diff --git a/src/bin/kawari-world.rs b/src/bin/kawari-world.rs index aa15ea8..244d24d 100644 --- a/src/bin/kawari-world.rs +++ b/src/bin/kawari-world.rs @@ -36,7 +36,7 @@ use tokio::sync::mpsc::{Receiver, UnboundedReceiver, UnboundedSender, channel, u use tokio::sync::oneshot; use tokio::task::JoinHandle; -use kawari::{INVENTORY_ACTION_ACK_GENERAL, INVENTORY_ACTION_ACK_SHOP}; +use kawari::{INVALID_ACTOR_ID, INVENTORY_ACTION_ACK_GENERAL, INVENTORY_ACTION_ACK_SHOP}; fn spawn_main_loop() -> (ServerHandle, JoinHandle<()>) { let (send, recv) = channel(64); @@ -814,25 +814,7 @@ async fn client_loop( } ClientZoneIpcData::ItemOperation(action) => { tracing::info!("Client is modifying inventory! {action:#?}"); - - let ipc = ServerZoneIpcSegment { - op_code: ServerZoneIpcType::InventoryActionAck, - timestamp: timestamp_secs(), - data: ServerZoneIpcData::InventoryActionAck { - sequence: action.context_id, - action_type: INVENTORY_ACTION_ACK_GENERAL as u16, - }, - ..Default::default() - }; - - connection - .send_segment(PacketSegment { - source_actor: connection.player_data.actor_id, - target_actor: connection.player_data.actor_id, - segment_type: SegmentType::Ipc, - data: SegmentData::Ipc { data: ipc }, - }) - .await; + connection.send_inventory_ack(action.context_id, INVENTORY_ACTION_ACK_GENERAL as u16).await; connection.player_data.inventory.process_action(action); @@ -842,23 +824,25 @@ async fn client_loop( } if action.operation_type == ItemOperationKind::Discard { - tracing::info!("Player is discarding from their inventory!"); + tracing::info!("Client is discarding from their inventory!"); let ipc = ServerZoneIpcSegment { op_code: ServerZoneIpcType::InventoryTransaction, timestamp: timestamp_secs(), data: ServerZoneIpcData::InventoryTransaction { - unk1: connection.player_data.item_sequence, + sequence: connection.player_data.item_sequence, operation_type: action.operation_type, src_actor_id: connection.player_data.actor_id, src_storage_id: action.src_storage_id, src_container_index: action.src_container_index, - unk2: action.src_stack, - unk3: action.src_catalog_id, // TODO: unk2 and unk3 are not being set accurately here, but the client doesn't seem to care what they are - dst_actor_id: 0, - dst_storage_id: 0xE000, + src_stack: action.src_stack, + src_catalog_id: action.src_catalog_id, + dst_actor_id: INVALID_ACTOR_ID, + dummy_container: ContainerType::DiscardingItemSentinel, + dst_storage_id: ContainerType::DiscardingItemSentinel, dst_container_index: u16::MAX, - dst_catalog_id: u32::MAX, + dst_stack: 0, + dst_catalog_id: 0, }, ..Default::default() }; @@ -876,10 +860,10 @@ async fn client_loop( op_code: ServerZoneIpcType::InventoryTransactionFinish, timestamp: timestamp_secs(), data: ServerZoneIpcData::InventoryTransactionFinish { - unk1: connection.player_data.item_sequence, - unk2: connection.player_data.item_sequence, // yes, this repeats, it's not a copy paste error - unk3: 0x90, - unk4: 0x200, + sequence: connection.player_data.item_sequence, + sequence_repeat: connection.player_data.item_sequence, + unk1: 0x90, + unk2: 0x200, }, ..Default::default() }; diff --git a/src/inventory/storage.rs b/src/inventory/storage.rs index 92c341d..2843a4c 100644 --- a/src/inventory/storage.rs +++ b/src/inventory/storage.rs @@ -96,6 +96,8 @@ pub enum ContainerType { HousingInteriorStoreroom6 = 27006, HousingInteriorStoreroom7 = 27007, HousingInteriorStoreroom8 = 27008, + + DiscardingItemSentinel = 65535, } /// Represents a generic item storage. diff --git a/src/ipc/zone/mod.rs b/src/ipc/zone/mod.rs index 141a317..e9c6880 100644 --- a/src/ipc/zone/mod.rs +++ b/src/ipc/zone/mod.rs @@ -405,40 +405,42 @@ pub enum ServerZoneIpcData { #[br(pre_assert(*magic == ServerZoneIpcType::InventoryTransaction))] InventoryTransaction { /// This is later reused in InventoryTransactionFinish, so it might be some sort of sequence or context id, but it's not the one sent by the client - unk1: u32, + sequence: u32, /// Same as the one sent by the client, not the one that the server responds with in InventoryActionAck! operation_type: ItemOperationKind, #[brw(pad_before = 3)] src_actor_id: u32, + #[brw(pad_size_to = 4)] src_storage_id: ContainerType, src_container_index: u16, #[brw(pad_before = 2)] - /// On retail, this contains very strange values that don't make sense (in regards to stack size). It's possible this field is a different size or isn't intended for stack size. - unk2: u32, - /// On retail, this contains very strange values that don't make sense (in regards to catalog id). It's possible this field is a different size or isn't intended for catalog id. - unk3: u32, + src_stack: u32, + src_catalog_id: u32, - /// This is all static as far as I can tell, across two captures and a bunch of discards these never changed - /// seems to always be 0 + /// This section was observed to be static, across two captures and a bunch of discards these never changed. + /// Always set to 0xE000_0000, also known as no/invalid actor. dst_actor_id: u32, - /// seems to always be 57344/0xE000 - dst_storage_id: u16, - /// seems to always be 65535/0xFFFF + /// Used in discard operations, both this dummy container and dst_storage_id are set to a container type of 0xFFFF. + /// While this struct is nearly identical to ItemOperation, it deviates here by not having 2 bytes of padding. + dummy_container: ContainerType, + dst_storage_id: ContainerType, dst_container_index: u16, - /// seems to always be 0x0000FFFF - #[brw(pad_after = 10)] + /// Always set to zero. + #[brw(pad_before = 2)] + dst_stack: u32, + /// Always set to zero. dst_catalog_id: u32, }, #[br(pre_assert(*magic == ServerZoneIpcType::InventoryTransactionFinish))] InventoryTransactionFinish { /// Same value as unk1 in InventoryTransaction. - unk1: u32, - /// Repeated unk1 value. - unk2: u32, + sequence: u32, + /// Repeated unk1 value. No, it's not a copy-paste error. + sequence_repeat: u32, /// Unknown, seems to always be 0x00000090. - unk3: u32, + unk1: u32, /// Unknown, seems to always be 0x00000200. - unk4: u32, + unk2: u32, }, #[br(pre_assert(*magic == ServerZoneIpcType::ContentFinderFound))] ContentFinderFound { @@ -944,26 +946,28 @@ mod tests { ( ServerZoneIpcType::InventoryTransaction, ServerZoneIpcData::InventoryTransaction { - unk1: 0, + sequence: 0, operation_type: ItemOperationKind::Move, src_actor_id: 0, src_storage_id: ContainerType::Inventory0, src_container_index: 0, - unk2: 0, - unk3: 0, + src_stack: 0, + src_catalog_id: 0, dst_actor_id: 0, - dst_storage_id: 0, + dummy_container: ContainerType::Inventory0, + dst_storage_id: ContainerType::Inventory0, dst_container_index: 0, + dst_stack: 0, dst_catalog_id: 0, }, ), ( ServerZoneIpcType::InventoryTransactionFinish, ServerZoneIpcData::InventoryTransactionFinish { + sequence: 0, + sequence_repeat: 0, unk1: 0, unk2: 0, - unk3: 0, - unk4: 0, }, ), ( diff --git a/src/lib.rs b/src/lib.rs index c40ea29..06d0a93 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -88,6 +88,9 @@ pub const CLASSJOB_ARRAY_SIZE: usize = 32; /// The maximum durability of an item. pub const ITEM_CONDITION_MAX: u16 = 30000; +/// The invalid/nothing/none actor ID. +pub const INVALID_ACTOR_ID: u32 = 0xE000_0000; + // These operation codes/types change regularly, so update them when needed! /// The operation opcode/type when discarding an item from the inventory.