Implement actual inventory-manipulation part of trading server side.

client/src/lib.rs

@@ -32,7 +32,7 @@ use common::{
     recipe::RecipeBook,
     span,
     terrain::{block::Block, neighbors, BiomeKind, SitesKind, TerrainChunk, TerrainChunkSize},
-    trade::{PendingTrade, TradeActionMsg},
+    trade::{PendingTrade, TradeActionMsg, TradeResult},
     uid::{Uid, UidAllocator},
     vol::RectVolSize,
 };
@@ -75,6 +75,10 @@ pub enum Event {
         answer: InviteAnswer,
         kind: InviteKind,
     },
+    TradeComplete {
+        result: TradeResult,
+        trade: PendingTrade,
+    },
     Disconnect,
     DisconnectionNotification(u64),
     InventoryUpdated(InventoryUpdateEvent),
@@ -1578,8 +1582,10 @@ impl Client {
                 tracing::info!("UpdatePendingTrade {:?} {:?}", id, trade);
                 self.pending_trade = Some((id, trade));
             },
-            ServerGeneral::DeclinedTrade => {
+            ServerGeneral::FinishedTrade(result) => {
-                self.pending_trade = None;
+                if let Some((_, trade)) = self.pending_trade.take() {
+                    frontend_events.push(Event::TradeComplete { result, trade })
+                }
             },
             _ => unreachable!("Not a in_game message"),
         }

common/net/src/msg/server.rs

@@ -8,7 +8,7 @@ use common::{
     recipe::RecipeBook,
     resources::TimeOfDay,
     terrain::{Block, TerrainChunk},
-    trade::PendingTrade,
+    trade::{PendingTrade, TradeResult},
     uid::Uid,
 };
 use hashbrown::HashMap;
@@ -124,7 +124,7 @@ pub enum ServerGeneral {
     /// Send a popup notification such as "Waypoint Saved"
     Notification(Notification),
     UpdatePendingTrade(usize, PendingTrade),
-    DeclinedTrade,
+    FinishedTrade(TradeResult),
 }
 
 impl ServerGeneral {
@@ -232,7 +232,7 @@ impl ServerMsg {
                         | ServerGeneral::Outcomes(_)
                         | ServerGeneral::Knockback(_)
                         | ServerGeneral::UpdatePendingTrade(_, _)
-                        | ServerGeneral::DeclinedTrade => {
+                        | ServerGeneral::FinishedTrade(_) => {
                             c_type == ClientType::Game && presence.is_some()
                         },
                         // Always possible

common/src/trade.rs

@@ -4,7 +4,7 @@ use crate::{
 };
 use hashbrown::HashMap;
 use serde::{Deserialize, Serialize};
-use tracing::warn;
+use tracing::{warn, trace};
 
 /// Clients submit `TradeActionMsg` to the server, which adds the Uid of the
 /// player out-of-band (i.e. without trusting the client to say who it's
@@ -18,6 +18,13 @@ pub enum TradeActionMsg {
     Decline,
 }
 
+#[derive(Clone, Debug, PartialEq, Serialize, Deserialize)]
+pub enum TradeResult {
+    Completed,
+    Declined,
+    NotEnoughSpace,
+}
+
 /// Items are not removed from the inventory during a PendingTrade: all the
 /// items are moved atomically (if there's space and both parties agree) upon
 /// completion
@@ -141,6 +148,7 @@ impl Trades {
         msg: TradeActionMsg,
         inventory: &Inventory,
     ) {
+        trace!("for trade id {}, message {:?}", id, msg);
         if let Some(trade) = self.trades.get_mut(&id) {
             if let Some(party) = trade.which_party(who) {
                 trade.process_msg(party, msg, inventory);

server/src/client.rs

@@ -90,7 +90,7 @@ impl Client {
                     | ServerGeneral::Outcomes(_)
                     | ServerGeneral::Knockback(_)
                     | ServerGeneral::UpdatePendingTrade(_, _)
-                    | ServerGeneral::DeclinedTrade => {
+                    | ServerGeneral::FinishedTrade(_) => {
                         self.in_game_stream.try_lock().unwrap().send(g)
                     },
                     // Always possible
@@ -170,7 +170,7 @@ impl Client {
                     | ServerGeneral::Outcomes(_)
                     | ServerGeneral::Knockback(_)
                     | ServerGeneral::UpdatePendingTrade(_, _)
-                    | ServerGeneral::DeclinedTrade => PreparedMsg::new(2, &g, &self.in_game_stream),
+                    | ServerGeneral::FinishedTrade(_) => PreparedMsg::new(2, &g, &self.in_game_stream),
                     // Always possible
                     ServerGeneral::PlayerListUpdate(_)
                     | ServerGeneral::ChatMsg(_)

server/src/events/trade.rs

@@ -2,15 +2,16 @@ use crate::{events::group_manip::handle_invite, Server};
 use common::{
     comp::{
         group::InviteKind,
-        inventory::{Inventory, slot::InvSlotId},
+        inventory::Inventory,
     },
-    trade::{PendingTrade, TradeActionMsg, Trades},
+    trade::{PendingTrade, TradeActionMsg, Trades, TradeResult},
-    uid::Uid,
 };
 use common_net::{msg::ServerGeneral, sync::WorldSyncExt};
 use specs::{world::WorldExt, Entity as EcsEntity};
-use tracing::warn;
+use std::cmp::Ordering;
+use tracing::{error, warn, trace};
 
+/// Invoked when pressing the trade button near an entity, triggering the invite UI flow
 pub fn handle_initiate_trade(server: &mut Server, interactor: EcsEntity, counterparty: EcsEntity) {
     if let Some(uid) = server.state_mut().ecs().uid_from_entity(counterparty) {
         handle_invite(server, interactor, uid, InviteKind::Trade);
@@ -19,6 +20,7 @@ pub fn handle_initiate_trade(server: &mut Server, interactor: EcsEntity, counter
     }
 }
 
+/// Invoked when the trade UI is up, handling item changes, accepts, etc
 pub fn handle_process_trade_action(
     server: &mut Server,
     entity: EcsEntity,
@@ -31,21 +33,23 @@ pub fn handle_process_trade_action(
             let to_notify = trades.decline_trade(trade_id, uid);
             to_notify
                 .and_then(|u| server.state.ecs().entity_from_uid(u.0))
-                .map(|e| server.notify_client(e, ServerGeneral::DeclinedTrade));
+                .map(|e| server.notify_client(e, ServerGeneral::FinishedTrade(TradeResult::Declined)));
         } else {
             if let Some(inv) = server.state.ecs().read_component::<Inventory>().get(entity) {
                 trades.process_trade_action(trade_id, uid, msg, inv);
             }
             if let Some(trade) = trades.trades.get(&trade_id) {
+                let mut msg = ServerGeneral::UpdatePendingTrade(trade_id, trade.clone());
                 if trade.should_commit() {
-                    // TODO: inventory manip
+                    let result = commit_trade(server.state.ecs(), trade);
+                    msg = ServerGeneral::FinishedTrade(result);
                 }
                 // send the updated state to both parties
                 for party in trade.parties.iter() {
                     server.state.ecs().entity_from_uid(party.0).map(|e| {
                         server.notify_client(
                             e,
-                            ServerGeneral::UpdatePendingTrade(trade_id, trade.clone()),
+                            msg.clone(),
                         )
                     });
                 }
@@ -53,3 +57,77 @@ pub fn handle_process_trade_action(
         }
     }
 }
+
+/// Commit a trade that both parties have agreed to, modifying their respective inventories
+fn commit_trade(ecs: &specs::World, trade: &PendingTrade) -> TradeResult {
+    let mut entities = vec![];
+    for who in [0, 1].iter().cloned() {
+        match ecs.entity_from_uid(trade.parties[who].0) {
+            Some(entity) => entities.push(entity),
+            None => return TradeResult::Declined,
+        }
+    }
+    let mut inventories = ecs.write_component::<Inventory>();
+    for who in [0, 1].iter().cloned() {
+        if inventories.get_mut(entities[who]).is_none() {
+            return TradeResult::Declined;
+        }
+    }
+    let invmsg = "inventories.get_mut(entities[who]).is_none() should have returned already";
+    trace!("committing trade: {:?}", trade);
+    // Compute the net change in slots of each player during the trade, to detect out-of-space-ness
+    // before transferring any items
+    let mut delta_slots: [isize; 2] = [0, 0];
+    for who in [0, 1].iter().cloned() {
+        for (slot, quantity) in trade.offers[who].iter() {
+            let inventory = inventories.get_mut(entities[who]).expect(invmsg);
+            let item = match inventory.get(*slot) {
+                Some(item) => item,
+                None => {
+                    error!("PendingTrade invariant violation in trade {:?}: slots offered in a trade should be non-empty", trade);
+                    return TradeResult::Declined;
+                },
+            };
+            match item.amount().cmp(&quantity) {
+                Ordering::Less => {
+                    error!("PendingTrade invariant violation in trade {:?}: party {} offered more of an item than they have", trade, who);
+                    return TradeResult::Declined;
+                },
+                Ordering::Equal => {
+                    delta_slots[who] -= 1; // exact, removes the whole stack
+                    delta_slots[1-who] += 1; // overapproximation, assumes the stack won't merge
+                },
+                Ordering::Greater => {
+                    // no change to delta_slots[who], since they have leftovers
+                    delta_slots[1-who] += 1; // overapproximation, assumes the stack won't merge
+                },
+            }
+        }
+    }
+    trace!("delta_slots: {:?}", delta_slots);
+    for who in [0, 1].iter().cloned() {
+        // Inventories should never exceed 2^{63} slots, so the usize -> isize conversions here
+        // should be safe
+        let inv = inventories.get_mut(entities[who]).expect(invmsg);
+        if inv.populated_slots() as isize + delta_slots[who] > inv.capacity() as isize {
+            return TradeResult::NotEnoughSpace;
+        }
+    }
+    let mut items = [vec![], vec![]];
+    for who in [0, 1].iter().cloned() {
+        for (slot, quantity) in trade.offers[who].iter() {
+            // take the items one by one, to benefit from Inventory's stack handling
+            for _ in 0..*quantity {
+                inventories.get_mut(entities[who]).expect(invmsg).take(*slot).map(|item| items[who].push(item));
+            }
+        }
+    }
+    for who in [0, 1].iter().cloned() {
+        if let Err(leftovers) = inventories.get_mut(entities[1-who]).expect(invmsg).push_all(items[who].drain(..)) {
+            // this should only happen if the arithmetic above for delta_slots says there's enough
+            // space and there isn't (i.e. underapproximates)
+            error!("Not enough space for all the items, destroying leftovers {:?}", leftovers);
+        }
+    }
+    TradeResult::Completed
+}

voxygen/src/session.rs

@@ -16,6 +16,7 @@ use common::{
     outcome::Outcome,
     span,
     terrain::{Block, BlockKind},
+    trade::TradeResult,
     util::{
         find_dist::{Cube, Cylinder, FindDist},
         Dir,
@@ -145,6 +146,15 @@ impl SessionState {
                     let msg = format!("{} invite to {} {}", kind_str, target_name, answer_str);
                     self.hud.new_message(ChatType::Meta.chat_msg(msg));
                 },
+                client::Event::TradeComplete { result, trade: _ } => {
+                    // TODO: i18n, entity names
+                    let msg = match result {
+                        TradeResult::Completed => "Trade completed successfully.",
+                        TradeResult::Declined => "Trade declined.",
+                        TradeResult::NotEnoughSpace => "Not enough space to complete the trade.",
+                    };
+                    self.hud.new_message(ChatType::Meta.chat_msg(msg));
+                },
                 client::Event::InventoryUpdated(inv_event) => {
                     let sfx_triggers = self.scene.sfx_mgr.triggers.read();