improve error messages

src/ship/items/manager.rs

@@ -1072,10 +1072,9 @@ impl ItemManager {
                 }
 
             });
-        Ok(it.commit(self, entity_gateway)
+        it.commit(self, entity_gateway)
             .await
-            //.map_err(|err| err.into())
-            .unwrap())
+            .map_err(|err| err.into())
     }
 }
 

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 inventory = item_manager.get_character_inventory(&client.character).ok()?;
-                        let item = inventory.get_item_by_id(ClientItemId(item_id))?;
+                        let client = clients.get(&this.client()).ok_or(ShipError::ClientNotFound(this.client()))?;
+                        let inventory = item_manager.get_character_inventory(&client.character)?;
+                        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 {