improve error messages

src/ship/items/inventory.rs

@@ -93,10 +93,16 @@ pub enum YesThereIsSpace {
     ExistingStack,
 }
 
+#[derive(Debug, Clone)]
+pub enum NoThereIsNotSpace {
+    FullStack,
+    FullInventory,
+}
+
 #[derive(Debug, Clone)]
 pub enum SpaceForStack {
     Yes(YesThereIsSpace),
-    No,
+    No(NoThereIsNotSpace),
 }
 
 impl InventoryItem {
@@ -470,7 +476,7 @@ impl CharacterInventory {
                     SpaceForStack::Yes(YesThereIsSpace::ExistingStack)
                 }
                 else {
-                    SpaceForStack::No
+                    SpaceForStack::No(NoThereIsNotSpace::FullStack)
                 }
             }
             None => {
@@ -478,7 +484,7 @@ impl CharacterInventory {
                     SpaceForStack::Yes(YesThereIsSpace::NewStack)
                 }
                 else {
-                    SpaceForStack::No
+                    SpaceForStack::No(NoThereIsNotSpace::FullInventory)
                 }
             }
         }

src/ship/items/manager.rs

@@ -15,6 +15,7 @@ use crate::ship::trade::TradeItem;
 use crate::ship::drops::{ItemDrop, ItemDropType};
 use crate::ship::location::{AreaClient, RoomId};
 use crate::ship::shops::ShopItem;
+use crate::ship::packet::handler::trade::TradeError;
 
 use crate::ship::items::bank::*;
 use crate::ship::items::floor::*;
@@ -329,7 +330,7 @@ impl ItemManager {
                                 }));
                                 TriggerCreateItem::No
                             },
-                            SpaceForStack::No => {
+                            SpaceForStack::No(_) => {
                                 return Err(ItemManagerError::CouldNotAddToInventory(*item_id).into());
                             },
                         }
@@ -945,9 +946,48 @@ impl ItemManager {
                 let p1_inventory = it.manager.get_character_inventory(p1.1)?;
                 let p2_inventory = it.manager.get_character_inventory(p2.1)?;
 
-                //TODO: inv-selftrade+othertrade <= 30
+                [(p2_inventory, p1_inventory, p2.2), (p1_inventory, p2_inventory, p1.2)].iter()
-                //if p1_inventory
+                    .map(|(src_inventory, dest_inventory, to_trade)| {
-
+                        to_trade
+                            .iter()
+                            .try_fold(dest_inventory.count(), |acc, item| {
+                                match item {
+                                    TradeItem::Individual(..) => {
+                                        if acc >= 30 {
+                                            Err(TradeError::NoInventorySpace)
+                                        }
+                                        else {
+                                            Ok(acc + 1)
+                                        }
+                                    },
+                                    TradeItem::Stacked(item_id, amount) => {
+                                        let stacked_inventory_item = src_inventory
+                                            .get_item_by_id(*item_id)
+                                            .ok_or_else(|| TradeError::InvalidItemId(*item_id))?
+                                            .stacked()
+                                            .ok_or_else(|| TradeError::InvalidItemId(*item_id))?;
+                                        match dest_inventory.space_for_stacked_item(&stacked_inventory_item.tool, *amount) {
+                                            SpaceForStack::Yes(YesThereIsSpace::ExistingStack) => {
+                                                Ok(acc)
+                                            },
+                                            SpaceForStack::Yes(YesThereIsSpace::NewStack) => {
+                                                Ok(acc + 1)
+                                            },
+                                            SpaceForStack::No(NoThereIsNotSpace::FullStack) => {
+                                                Err(TradeError::NoStackSpace)
+                                            },
+                                            SpaceForStack::No(NoThereIsNotSpace::FullInventory) => {
+                                                Err(TradeError::NoInventorySpace)
+                                            },
+                                        }
+                                    },
+                                    TradeItem::Meseta(..) => {
+                                        Ok(acc)
+                                    }
+                                }
+                            })
+                    })
+                    .collect::<Result<Vec<_>, _>>()?;
 
                 let trade_items = [(p1, p2, p1_inventory, p2_inventory), (p2, p1, p2_inventory, p1_inventory)]
                     .map(|(src_client, dest_client, src_inventory, dest_inventory)| {
@@ -1032,10 +1072,9 @@ impl ItemManager {
                 }
 
             });
-        Ok(it.commit(self, entity_gateway)
+        it.commit(self, entity_gateway)
             .await
-            //.map_err(|err| err.into())
+            .map_err(|err| err.into())
-            .unwrap())
     }
 }
 

src/ship/items/mod.rs

@@ -2,7 +2,7 @@ mod bank;
 mod floor;
 pub mod inventory;
 mod manager;
-mod transaction;
+pub mod transaction;
 pub mod use_tool;
 use serde::{Serialize, Deserialize};
 

src/ship/items/transaction.rs

@@ -13,9 +13,10 @@ use crate::entity::gateway::GatewayError;
 use crate::ship::location::{AreaClient, RoomId};
 
 #[derive(Error, Debug)]
-#[error("")]
 pub enum TransactionCommitError {
+    #[error("transaction commit gateway error {0}")]
     Gateway(#[from] GatewayError),
+    #[error("transaction commit itemmanager error {0}")]
     ItemManager(#[from] ItemManagerError),
 }
 
@@ -77,9 +78,10 @@ impl<'a, T, EG: EntityGateway> ItemTransaction<'a, T, EG> {
 
 
 #[derive(Error, Debug)]
-#[error("")]
 pub enum TransactionError<E: std::fmt::Debug> {
+    #[error("transaction action error {0:?}")]
     Action(E),
+    #[error("transaction commit error {0}")]
     Commit(#[from] TransactionCommitError),
     
 }

src/ship/packet/handler/trade.rs

@@ -18,10 +18,12 @@ use libpso::utf8_to_utf16_array;
 use crate::ship::packet::builder;
 use crate::ship::shops::{ShopItem, ToolShopItem, ArmorShopItem};
 
-#[derive(thiserror::Error, Debug)]
+#[derive(thiserror::Error, Debug, PartialEq, Eq)]
 pub enum TradeError {
     #[error("no partner")]
     CouldNotFindTradePartner,
+    #[error("invalid item id")]
+    InvalidItemId(ClientItemId),
     #[error("item does not match id")]
     ClientItemIdDidNotMatchItem(ClientItemId, [u8; 16]),
     #[error("invalid stack {1}")]
@@ -30,6 +32,10 @@ pub enum TradeError {
     NotInTradeMenu,
     #[error("trade menu at an invalid point")]
     MismatchedStatus,
+    #[error("no space in inventory")]
+    NoInventorySpace,
+    #[error("no space in stack")]
+    NoStackSpace,
 }
 
 
@@ -97,11 +103,11 @@ where
         },
         TradeRequestCommand::AddItem(item_id, amount) => {
             Ok(trades
-                .with(&id, |this, other| -> Option<Box<dyn Iterator<Item = (ClientId, SendShipPacket)> + Send>> {
+                .with(&id, |this, other| -> Result<Box<dyn Iterator<Item = (ClientId, SendShipPacket)> + Send>, anyhow::Error> {
                     if this.status == TradeStatus::Trading && other.status == TradeStatus::Trading {
-                        let client = clients.get(&this.client())?;//.ok_or(ShipError::ClientNotFound(id)).ok()?;
+                        let client = clients.get(&this.client()).ok_or(ShipError::ClientNotFound(this.client()))?;
-                        let inventory = item_manager.get_character_inventory(&client.character).ok()?;
+                        let inventory = item_manager.get_character_inventory(&client.character)?;
-                        let item = inventory.get_item_by_id(ClientItemId(item_id))?;
+                        let item = inventory.get_item_by_id(ClientItemId(item_id)).ok_or(ItemManagerError::NoSuchItemId(ClientItemId(item_id)))?;
 
                         match item {
                             InventoryItem::Individual(_) => {
@@ -109,24 +115,24 @@ where
                             },
                             InventoryItem::Stacked(stacked_item) => {
                                 if stacked_item.count() < amount as usize {
-                                    return None;
+                                    return Err(TradeError::InvalidStackAmount(ClientItemId(item_id), amount as usize).into());
                                 }
                                 this.items.push(TradeItem::Stacked(ClientItemId(item_id), amount as usize));
                             },
                         }
 
                         let trade_request = trade_request.clone();
-                        Some(Box::new(client_location.get_all_clients_by_client(id).unwrap().into_iter()
+                        Ok(Box::new(client_location.get_all_clients_by_client(id)?.into_iter()
                                     .filter(move |client| client.local_client.id() == target as u8)
                                     .map(move |client| {
                                         (client.client, SendShipPacket::DirectMessage(DirectMessage::new(target, GameMessage::TradeRequest(trade_request.clone()))))
                                     })))
                     }
                     else {
-                        None
+                        Err(TradeError::MismatchedStatus.into())
                     }
                 })?
-                .unwrap_or_else(|| {
+                .unwrap_or_else(|err| {
                     trades.remove_trade(&id);
                     Box::new(client_location.get_all_clients_by_client(id).unwrap().into_iter()
                              .filter(move |client| client.local_client.id() == target as u8)
@@ -194,7 +200,7 @@ where
         TradeRequestCommand::Confirm => {
             Ok(trades
                 .with(&id, |this, other| -> Option<Box<dyn Iterator<Item = (ClientId, SendShipPacket)> + Send>> {
-                    if this.status == TradeStatus::Trading && (other.status == TradeStatus::Trading || other.status == TradeStatus::Confirmed) {
+                    if status_is(&this.status, &[TradeStatus::Trading]) && status_is(&other.status, &[TradeStatus::Trading, TradeStatus::Confirmed]) {
                         this.status = TradeStatus::Confirmed;
 
                         let trade_request = trade_request.clone();
@@ -380,8 +386,6 @@ where
            if status_is_not(&this.status, &[TradeStatus::ItemsChecked]) || status_is_not(&other.status, &[TradeStatus::ItemsChecked, TradeStatus::TradeComplete]) {
                return Err(TradeError::MismatchedStatus.into())
            }
-
-           // TODO: check for space in inventory!
            this.status = TradeStatus::TradeComplete;
 
            if this.status == TradeStatus::TradeComplete && other.status == TradeStatus::TradeComplete {