Automatically overflows items to 'overflow slots' that are inaccessible if items cannot be inserted in persistence on character load.

common/src/comp/inventory/mod.rs

@@ -45,6 +45,11 @@ pub struct Inventory {
     /// The "built-in" slots belonging to the inventory itself, all other slots
     /// are provided by equipped items
     slots: Vec<InvSlot>,
+    /// For when slot amounts are rebalanced or the inventory otherwise does not
+    /// have enough space to hold all the items after loading from database.
+    /// These slots are "remove-only" meaning that during normal gameplay items
+    /// can only be removed from these slots and never entered.
+    overflow_items: Vec<Item>,
 }
 
 /// Errors which the methods on `Inventory` produce
@@ -55,6 +60,14 @@ pub enum Error {
     Full(Vec<Item>),
 }
 
+impl Error {
+    pub fn returned_items(self) -> impl Iterator<Item = Item> {
+        match self {
+            Error::Full(items) => items.into_iter(),
+        }
+    }
+}
+
 #[derive(Clone, Copy, Debug, Serialize, Deserialize)]
 pub enum InventorySortOrder {
     Name,
@@ -115,6 +128,7 @@ impl Inventory {
             next_sort_order: InventorySortOrder::Name,
             loadout,
             slots: vec![None; DEFAULT_INVENTORY_SLOTS],
+            overflow_items: Vec::new(),
         }
     }
 
@@ -123,10 +137,11 @@ impl Inventory {
             next_sort_order: InventorySortOrder::Name,
             loadout,
             slots: vec![None; 1],
+            overflow_items: Vec::new(),
         }
     }
 
-    /// Total number of slots in in the inventory.
+    /// Total number of slots in the inventory.
     pub fn capacity(&self) -> usize { self.slots().count() }
 
     /// An iterator of all inventory slots
@@ -136,6 +151,9 @@ impl Inventory {
             .chain(self.loadout.inv_slots_with_id().map(|(_, slot)| slot))
     }
 
+    /// An iterator of all overflow slots in the inventory
+    pub fn overflow_items(&self) -> impl Iterator<Item = &Item> { self.overflow_items.iter() }
+
     /// A mutable iterator of all inventory slots
     fn slots_mut(&mut self) -> impl Iterator<Item = &mut InvSlot> {
         self.slots.iter_mut().chain(self.loadout.inv_slots_mut())
@@ -910,6 +928,9 @@ impl Inventory {
                 item.update_item_state(ability_map, msm);
             }
         });
+        self.overflow_items
+            .iter_mut()
+            .for_each(|item| item.update_item_state(ability_map, msm));
     }
 
     /// Increments durability lost for all valid items equipped in loadout and
@@ -957,6 +978,13 @@ impl Inventory {
             },
         }
     }
+
+    /// When loading a character from the persistence system, pushes any items
+    /// to overflow_items that were not able to be loaded into or pushed to the
+    /// inventory
+    pub fn persistence_push_overflow_items<I: Iterator<Item = Item>>(&mut self, overflow_items: I) {
+        self.overflow_items.extend(overflow_items);
+    }
 }
 
 impl Component for Inventory {

server/src/persistence/character/conversions.rs

@@ -63,13 +63,22 @@ pub fn convert_items_to_database_items(
         .map(|(slot, item)| (slot.to_string(), item, loadout_container_id));
 
     // Inventory slots.
-    let inventory = inventory.slots_with_id().map(|(pos, item)| {
+    let inventory = inventory
+        .slots_with_id()
+        .map(|(pos, item)| {
             (
                 serde_json::to_string(&pos).expect("failed to serialize InventorySlotPos"),
                 item.as_ref(),
                 inventory_container_id,
             )
-    });
+        })
+        .chain(inventory.overflow_items().enumerate().map(|(index, item)| {
+            (
+                format!("overflow_item {index}"),
+                Some(item),
+                inventory_container_id,
+            )
+        }));
 
     // Use Breadth-first search to recurse into containers/modular weapons to store
     // their parts
@@ -363,6 +372,24 @@ pub fn convert_inventory_from_database_items(
     let mut inventory = Inventory::with_loadout_humanoid(loadout);
     let mut item_indices = HashMap::new();
 
+    struct FailedInserts {
+        items: Vec<VelorenItem>,
+        map: HashMap<String, usize>,
+    }
+
+    impl FailedInserts {
+        fn insert(&mut self, db_pos: String, item: VelorenItem) {
+            let i = self.items.len();
+            self.items.push(item);
+            self.map.insert(db_pos, i);
+        }
+    }
+
+    let mut failed_inserts = FailedInserts {
+        items: Vec::new(),
+        map: HashMap::new(),
+    };
+
     // In order to items with components to properly load, it is important that this
     // item iteration occurs in order so that any modular items are loaded before
     // its components.
@@ -412,36 +439,55 @@ pub fn convert_inventory_from_database_items(
         };
 
         if db_item.parent_container_item_id == inventory_container_id {
-            let slot = slot(&db_item.position)?;
-            let insert_res = inventory.insert_at(slot, item).map_err(|_| {
-                // If this happens there were too many items in the database for the current
-                // inventory size
-                //
-                // FIXME: On failure, collect the set of items that don't fit and return them
-                // (to be dropped next to the player) as this could be the
-                // result of a change in the slot capacity for an equipped bag
-                // (or a change in the inventory size).
-                PersistenceError::ConversionError(format!(
-                    "Error inserting item into inventory, position: {:?}",
-                    slot
-                ))
-            })?;
+            if let Ok(slot) = slot(&db_item.position) {
+                let insert_res = inventory.insert_at(slot, item);
 
-            if insert_res.is_some() {
-                // If inventory.insert returns an item, it means it was swapped for an item that
-                // already occupied the slot. Multiple items being stored in the database for
-                // the same slot is an error.
+                match insert_res {
+                    Ok(None) => {
+                        // Insert successful
+                    },
+                    Ok(Some(_item)) => {
+                        // If inventory.insert returns an item, it means it was swapped for an item
+                        // that already occupied the slot. Multiple items
+                        // being stored in the database for the same slot is
+                        // an error.
                         return Err(PersistenceError::ConversionError(
                             "Inserted an item into the same slot twice".to_string(),
                         ));
+                    },
+                    Err(item) => {
+                        // If this happens there were too many items in the database for the current
+                        // inventory size
+                        failed_inserts.insert(db_item.position.clone(), item);
+                    },
+                }
+            } else {
+                failed_inserts.insert(db_item.position.clone(), item);
             }
         } else if let Some(&j) = item_indices.get(&db_item.parent_container_item_id) {
             get_mutable_item(
                 j,
                 inventory_items,
                 &item_indices,
-                &mut inventory,
-                &|inv, s| inv.slot_mut(slot(s).ok()?).and_then(|a| a.as_mut()),
+                &mut (&mut inventory, &mut failed_inserts),
+                &|(inv, f_i): &mut (&mut Inventory, &mut FailedInserts), s| {
+                    // Attempts first to access inventory if that slot exists there. If it does not
+                    // it instead attempts to access failed inserts list.
+                    // Question for Sharp/XVar: Should this attempt to look in failed inserts list
+                    // first?
+                    slot(s)
+                        .ok()
+                        .and_then(|slot| inv.slot_mut(slot))
+                        .and_then(|a| a.as_mut())
+                        .or(f_i.map.get(s).and_then(|i| f_i.items.get_mut(*i)))
+                    // if let Ok(slot) = slot(s) {
+                    //     dbg!(0);
+                    //     inv.slot_mut(slot).and_then(|a| a.as_mut())
+                    // } else {
+                    //     dbg!(1);
+                    //     f_i.map.get(s).and_then(|i| f_i.items.get_mut(*i))
+                    // }
+                },
             )?
             .persistence_access_add_component(item);
         } else {
@@ -452,6 +498,12 @@ pub fn convert_inventory_from_database_items(
         }
     }
 
+    // For failed inserts, attempt to push to inventory. If push fails, move to
+    // overflow slots.
+    if let Err(inv_error) = inventory.push_all(failed_inserts.items.into_iter()) {
+        inventory.persistence_push_overflow_items(inv_error.returned_items());
+    }
+
     // Some items may have had components added, so update the item config of each
     // item to ensure that it correctly accounts for components that were added
     inventory.persistence_update_all_item_states(&ABILITY_MAP, &MATERIAL_STATS_MANIFEST);