From 421aa44d5f0b6961c2e3347ee2fca70e432fbd5a Mon Sep 17 00:00:00 2001
From: juliancoffee <lightdarkdaughter@gmail.com>
Date: Mon, 5 Sep 2022 00:10:30 +0300
Subject: [PATCH] Make kit generate all possible modulars

---
 common/src/comp/inventory/item/modular.rs | 11 +--
 server/src/cmd.rs                         | 87 ++++++++++++++---------
 2 files changed, 61 insertions(+), 37 deletions(-)

diff --git a/common/src/comp/inventory/item/modular.rs b/common/src/comp/inventory/item/modular.rs
index 37f4338989..a3ce9f6104 100644
--- a/common/src/comp/inventory/item/modular.rs
+++ b/common/src/comp/inventory/item/modular.rs
@@ -479,9 +479,7 @@ pub fn generate_weapons(
     let primaries = generate_weapon_primary_components(tool, material, hand_restriction)?;
     let mut weapons = Vec::new();
 
-    // TODO: should we always ignore handness?
-    // We seems to do so in `random_weapon`
-    for (comp, _hand) in primaries {
+    for (comp, comp_hand) in primaries {
         let secondaries = SECONDARY_COMPONENT_POOL
             .get(&tool)
             .into_iter()
@@ -490,7 +488,12 @@ pub fn generate_weapons(
                 hand_restriction == *hand || hand_restriction.is_none() || hand.is_none()
             });
 
-        for (def, _hand) in secondaries {
+        for (def, hand) in secondaries {
+            if comp_hand.is_some() && hand.is_some() && comp_hand != *hand {
+                // if handedness of components incompatible, skip
+                continue;
+            }
+
             let secondary = Item::new_from_item_base(
                 ItemBase::Simple(Arc::clone(def)),
                 Vec::new(),
diff --git a/server/src/cmd.rs b/server/src/cmd.rs
index 79be96586f..d868cfef3c 100644
--- a/server/src/cmd.rs
+++ b/server/src/cmd.rs
@@ -1944,51 +1944,72 @@ where
         if target_inventory.free_slots() < count {
             return Err("Inventory doesn't have enough slots".to_owned());
         }
-        let mut rng = thread_rng();
-        for (item_id, quantity) in kit {
-            let mut item = match &item_id {
-                KitSpec::Item(item_id) => Item::new_from_asset(item_id)
-                    .map_err(|_| format!("Unknown item: {:#?}", item_id))?,
-                KitSpec::ModularWeapon { tool, material } => {
-                    comp::item::modular::random_weapon(*tool, *material, None, &mut rng)
-                        .map_err(|err| format!("{:#?}", err))?
-                },
-            };
-            let mut res = Ok(());
 
-            // Either push stack or push one by one.
-            if item.is_stackable() {
-                // FIXME: in theory, this can fail,
-                // but we don't have stack sizes yet.
-                let _ = item.set_amount(quantity);
-                res = target_inventory.push(item);
+        for (item_id, quantity) in kit {
+            push_item(item_id, quantity, server, &mut |item| {
+                let res = target_inventory.push(item);
                 let _ = target_inv_update.insert(
                     target,
                     comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Debug),
                 );
-            } else {
-                let ability_map = server.state.ecs().read_resource::<AbilityMap>();
-                let msm = server.state.ecs().read_resource::<MaterialStatManifest>();
-                for _ in 0..quantity {
-                    res = target_inventory.push(item.duplicate(&ability_map, &msm));
-                    let _ = target_inv_update.insert(
-                        target,
-                        comp::InventoryUpdate::new(comp::InventoryUpdateEvent::Debug),
-                    );
-                }
-            }
-            // I think it's possible to pick-up item during this loop
-            // and fail into case where you had space but now you don't?
-            if res.is_err() {
-                return Err("Can't fit item to inventory".to_owned());
-            }
+
+                res
+            })?;
         }
+
         Ok(())
     } else {
         Err("Could not get inventory".to_string())
     }
 }
 
+fn push_item(
+    item_id: KitSpec,
+    quantity: u32,
+    server: &Server,
+    push: &mut dyn FnMut(Item) -> Result<(), Item>,
+) -> CmdResult<()> {
+    let items = match &item_id {
+        KitSpec::Item(item_id) => vec![
+            Item::new_from_asset(item_id).map_err(|_| format!("Unknown item: {:#?}", item_id))?,
+        ],
+        KitSpec::ModularWeapon { tool, material } => {
+            comp::item::modular::generate_weapons(*tool, *material, None)
+                .map_err(|err| format!("{:#?}", err))?
+        },
+    };
+
+    let mut res = Ok(());
+    for mut item in items {
+        // Either push stack or push one by one.
+        if item.is_stackable() {
+            // FIXME: in theory, this can fail,
+            // but we don't have stack sizes yet.
+            let _ = item.set_amount(quantity);
+            res = push(item);
+        } else {
+            let ability_map = server.state.ecs().read_resource::<AbilityMap>();
+            let msm = server.state.ecs().read_resource::<MaterialStatManifest>();
+
+            for _ in 0..quantity {
+                res = push(item.duplicate(&ability_map, &msm));
+
+                if res.is_err() {
+                    break;
+                }
+            }
+        }
+
+        // I think it's possible to pick-up item during this loop
+        // and fail into case where you had space but now you don't?
+        if res.is_err() {
+            return Err("Can't fit item to inventory".to_owned());
+        }
+    }
+
+    Ok(())
+}
+
 fn handle_object(
     server: &mut Server,
     client: EcsEntity,