fix: stop shift-click opening the stack splitter on vendor bundles (#751)

- Shift-clicking water or arrows at a vendor takes one bundle, as 1.12.1 does
- Drop the merchant comments that restate the code

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

crates/benilla-app/src/ui_merchant/mod.rs

@@ -204,7 +204,7 @@ fn resolve_item(
         item_id: item.entry,
         stats,
         link,
-        max_stack: template.map(|t| t.stackable.max(1)),
+        max_stack: template.map(|t| t.stackable),
     }
 }
 

crates/benilla-app/src/ui_script/merchant_tests.rs

@@ -1,8 +1,8 @@
 //! The stock merchant window (`MerchantFrame.xml`), engine-only, fed a synthetic stock and purse.
 
 use benilla_ui::script::{
-    ContainerState, DressUpIntent, ExtractedQuad, ItemStatsHead, MerchantItem, MerchantState,
-    QuadContent, ScriptValue, SoundRequest, UiScript,
+    ContainerState, CursorPayload, DressUpIntent, ExtractedQuad, ItemStatsHead, MerchantItem,
+    MerchantState, QuadContent, ScriptValue, SoundRequest, UiScript,
 };
 
 use super::test_ui::{bag_open, load_ui as load_xml, BAG_UI};
@@ -1192,3 +1192,88 @@ fn ctrl_and_shift_on_a_vendor_row_preview_and_post_without_buying() {
     );
     assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
 }
+
+#[test]
+fn shift_click_takes_one_bundle_and_splits_a_single() {
+    benilla_formats::wow_data_or_skip!();
+    let mut s = UiScript::new().unwrap();
+    s.set_screen_size(1024.0, 768.0);
+    for f in crate::ui_script::test_ui::production_order(&[
+        super::test_ui::MERCHANT_UI,
+        &[
+            "Interface\\FrameXML\\UIMenu.xml",
+            "Interface\\FrameXML\\UIDropDownMenu.xml",
+            "Interface\\FrameXML\\ChatFrame.xml",
+            "Interface\\FrameXML\\FloatingChatFrame.xml",
+            "Interface\\FrameXML\\StackSplitFrame.xml",
+            "Interface\\FrameXML\\MerchantFrame.xml",
+        ],
+    ]) {
+        load_xml(&s, f);
+    }
+    s.set_money(10_000);
+    s.set_merchant(Some(MerchantState {
+        items: vec![
+            MerchantItem {
+                name: Some("Refreshing Spring Water".into()),
+                price: 25,
+                quantity: 5,
+                num_available: -1,
+                item_id: 159,
+                max_stack: Some(20),
+                ..Default::default()
+            },
+            MerchantItem {
+                name: Some("Flask of Oil".into()),
+                price: 100,
+                quantity: 1,
+                num_available: -1,
+                item_id: 814,
+                max_stack: Some(20),
+                ..Default::default()
+            },
+        ],
+        ..Default::default()
+    }));
+    s.fire_event("MERCHANT_SHOW", vec![]);
+    assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+
+    let shift_click = |s: &mut UiScript, row: u32, button: &str| {
+        let (x, y) = super::test_ui::centre_of(s, &format!("MerchantItem{row}ItemButton"));
+        s.set_modifiers(true, false, false);
+        s.mouse_button(x, y, button, true);
+        s.mouse_button(x, y, button, false);
+        s.set_modifiers(false, false, false);
+    };
+    let split_shown = |s: &UiScript| s.eval::<bool>("return StackSplitFrame:IsShown()").unwrap();
+
+    shift_click(&mut s, 1, "LeftButton");
+    assert!(!split_shown(&s), "a bundle row opens no split");
+    assert!(
+        matches!(s.cursor_payload(), Some(CursorPayload::Merchant(m)) if m.item_id == 159),
+        "the left arm picks one bundle up: {:?}",
+        s.cursor_payload()
+    );
+    s.run("ClearCursor()").unwrap();
+
+    shift_click(&mut s, 1, "RightButton");
+    assert!(!split_shown(&s), "a bundle row opens no split");
+    assert_eq!(
+        s.take_merchant_buys(),
+        vec![(1, 1)],
+        "the right arm buys one bundle"
+    );
+
+    shift_click(&mut s, 2, "RightButton");
+    assert!(split_shown(&s), "a single row opens the split");
+    assert_eq!(
+        s.eval::<i64>("return StackSplitFrame.maxStack").unwrap(),
+        20,
+        "the split spans the stack size, under what the purse affords"
+    );
+    assert!(
+        s.take_merchant_buys().is_empty(),
+        "the split buys nothing until it is chosen"
+    );
+    assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}

crates/benilla-ui/src/script/merchant.rs

@@ -9,6 +9,7 @@
 
 use mlua::{Lua, MultiValue, Value};
 
+use super::binding_abi::number_arg;
 use super::container::UiCursorMode;
 use super::cursor::CursorPayload;
 use super::Model;
@@ -33,8 +34,8 @@ pub struct MerchantItem {
     /// `GetMerchantItemLink`'s answer; `None` while in flight and on a buyback row, as 1.12 has no
     /// `GetBuybackItemLink` and the buyback click takes no modifier (`MerchantFrame.lua:358-361`).
     pub link: Option<String>,
-    /// `GetMerchantItemMaxStack`: the template's `stackable`, 1 if it does not stack; `None` while
-    /// in flight.
+    /// The template's `stackable`, `GetMerchantItemMaxStack`'s answer for a row sold singly;
+    /// `None` while in flight and on a buyback row.
     pub max_stack: Option<u32>,
 }
 
@@ -241,19 +242,24 @@ pub(super) fn install(lua: &Lua) -> mlua::Result<()> {
         })?,
     )?;
 
-    // GetMerchantItemMaxStack(index): nil while in flight or out of range. A right shift-click, or
-    // a left one with the chat box closed, asks it and opens the stack-split spinner only above 1
-    // (`MerchantFrame.lua:313`, `:340`).
+    // GetMerchantItemMaxStack(index) (`0x4fb670`): the template's stack size, or 1 for a row sold
+    // in bundles (`[row+0x18] > 1`) and for every miss: out of range, no vendor, a template in
+    // flight (`0x4fb739`). A right shift-click, or a left one with the chat box closed, asks it and
+    // opens the stack-split spinner only above 1 (`MerchantFrame.lua:313`, `:340`).
     g.set(
         "GetMerchantItemMaxStack",
-        lua.create_function(|lua, index: usize| {
+        lua.create_function(|lua, index: Value| {
+            let index = number_arg(lua, index, "Usage: GetMerchantItemMaxStack(index)")?;
             let model = lua.app_data_ref::<Model>().expect("model app_data");
-            Ok(model
-                .merchant
-                .as_ref()
-                .and_then(|m| index.checked_sub(1).and_then(|n| m.items.get(n)))
+            // Signed, as 1.12.1 reads both.
+            let max_stack = usize::try_from(index)
+                .ok()
+                .and_then(|i| i.checked_sub(1))
+                .and_then(|n| model.merchant.as_ref()?.items.get(n))
+                .filter(|it| it.quantity as i32 <= 1)
                 .and_then(|it| it.max_stack)
-                .map_or(Value::Nil, |n| Value::Integer(i64::from(n))))
+                .map_or(1, |n| n as i32);
+            Ok(i64::from(max_stack))
         })?,
     )?;
 
@@ -533,7 +539,7 @@ mod tests {
                     name: Some("Refreshing Spring Water".into()),
                     texture: Some("Interface\\Icons\\INV_Drink_18".into()),
                     price: 25,
-                    quantity: 1,
+                    quantity: 5,
                     num_available: -1, // unlimited
                     item_id: 159,
                     stats: Some(ItemStatsHead {
@@ -575,7 +581,7 @@ mod tests {
             .unwrap();
         assert_eq!(name, "Refreshing Spring Water");
         assert_eq!(texture, "Interface\\Icons\\INV_Drink_18");
-        assert_eq!((price, quantity, num), (25, 1, -1));
+        assert_eq!((price, quantity, num), (25, 5, -1));
         assert_eq!(usable, 1);
 
         // In flight: name and texture nil, and usable (the null-record skip).
@@ -602,17 +608,52 @@ mod tests {
         assert!(s
             .eval::<bool>("return GetMerchantItemLink(9) == nil")
             .unwrap());
+    }
 
-        assert_eq!(
-            s.eval::<i64>("return GetMerchantItemMaxStack(1)").unwrap(),
-            20
-        );
-        assert!(s
-            .eval::<bool>("return GetMerchantItemMaxStack(2) == nil")
-            .unwrap());
-        assert!(s
-            .eval::<bool>("return GetMerchantItemMaxStack(9) == nil")
-            .unwrap());
+    #[test]
+    fn merchant_max_stack_is_one_for_bundles_and_misses() {
+        let mut s = UiScript::new().unwrap();
+        let max = |s: &mut UiScript, arg: &str| {
+            s.eval::<i64>(&format!("return GetMerchantItemMaxStack({arg})"))
+                .unwrap()
+        };
+        assert_eq!(max(&mut s, "1"), 1, "no vendor open");
+
+        let mut state = stock();
+        let single = |quantity: u32, max_stack: u32| MerchantItem {
+            name: Some("Flask of Oil".into()),
+            price: 100,
+            quantity,
+            num_available: -1,
+            item_id: 814,
+            max_stack: Some(max_stack),
+            ..Default::default()
+        };
+        state.items.push(single(1, 20));
+        state.items.push(single(1, 0));
+        state.items.push(single(0x8000_0000, u32::MAX));
+        s.set_merchant(Some(state));
+
+        assert_eq!(max(&mut s, "1"), 1, "a bundle row takes no split");
+        assert_eq!(max(&mut s, "2"), 1, "a template in flight");
+        assert_eq!(max(&mut s, "3"), 20, "a single row answers its stack size");
+        assert_eq!(max(&mut s, "4"), 0, "the stack size is not floored");
+        assert_eq!(max(&mut s, "5"), -1, "both fields read signed");
+        for out_of_range in ["0", "-1", "6", "9"] {
+            assert_eq!(max(&mut s, out_of_range), 1, "index {out_of_range}");
+        }
+        assert_eq!(max(&mut s, "\"3\""), 20);
+        assert_eq!(max(&mut s, "3.9"), 20);
+        for bad in ["", "nil", "\"x\"", "{}"] {
+            let err = s
+                .eval::<i64>(&format!("return GetMerchantItemMaxStack({bad})"))
+                .unwrap_err()
+                .to_string();
+            assert!(
+                err.contains("Usage: GetMerchantItemMaxStack(index)"),
+                "{bad:?}: {err}"
+            );
+        }
     }
 
     #[test]