quest sharing is in, and declining at an NPC reopens its list

- quest sharing lands whole: four opcodes, the Share button live with the reference's own predicate (registering the one party event it does for the one reason it does — joining a party changes the answer without changing the log), nine verdict lines for the sharer, and the escort confirm as its own dialog whose No sends nothing at all, because there is no decline packet for it; the push result's u64 means two things by direction — the sharer being answered going up, the member the verdict is ABOUT coming down — and the verdict stays a raw byte so an unmapped value shows the reference's own nothing rather than killing the packet - the receiving half needs almost no code, and that is the design: the server sends an eligible member an ORDINARY quest-details packet whose giver guid is the sharing PLAYER, and Accept answers with the ordinary accept — a shared quest lands on the questgiver panel we already have, and the guid's own type bits drive the two real differences: declining a share owes the sharer a DECLINE verdict, and the panel is not held to the 5.56-yard NPC service range (the server's bound for a share is 14 yards, tested at accept) - the fold-back corrected three things days-old code had just shipped: DeclineQuest and CloseQuest are ONE routine in the reference — what varies is the giver, not which button you pressed; declining an NPC's quest RE-OPENS its list — the session end sends the giver a fresh hello, refuting our five-month-old claim that "vanilla's client-side decline sends no packet" (and the details packet's trailing u32 we had parsed and ignored is exactly what suppresses that re-open) — so declining at an NPC now returns you to its menu instead of closing the window; and BUSY is the client's OWN verdict — a share arriving while any panel is open is refused client-side, covering the mid-turn-in case the server cannot see Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-31 20:14:06 +07:00
parent 1052393487
commit ea3ff5d050
34 changed files with 1994 additions and 91 deletions
@@ -957,6 +957,9 @@ mod petition_tests;
 #[cfg(test)]
 mod raid_tests;

+#[cfg(test)]
+mod quest_share_tests;
+
 #[cfg(test)]
 mod quest_tests;

@@ -0,0 +1,118 @@
+//! The escort-quest confirm (decision 1733, `QuestShareFrame.xml`): the Lua wiring between the
+//! `QUEST_ACCEPT_CONFIRM` event `ui_quest_share`'s feed fires and the shared StaticPopup engine,
+//! driven exactly as that feed drives it.
+//!
+//! The asymmetry these tests pin is the reference's: **Yes sends, No does not**. There is no
+//! decline packet for an escort confirm, so a test that only checked "the popup closes" would pass
+//! against a version that silently answered No on the wire.
+
+use benilla_ui::script::{ScriptValue, UiScript};
+
+/// Load one shipped `assets/ui/<file>` into `s`, panicking on any loader error (the duel tests'
+/// loader, duplicated so this file is self-contained).
+fn load_xml(s: &UiScript, file: &str) -> usize {
+    let text = std::fs::read_to_string(
+        std::path::Path::new(env!("CARGO_MANIFEST_DIR"))
+            .join("assets/ui")
+            .join(file),
+    )
+    .unwrap();
+    let doc = benilla_ui::framexml::parse(&text).unwrap();
+    let report = benilla_ui::loader::load(s, &doc, &|_| None);
+    assert!(
+        report.errors.is_empty(),
+        "{file}: loader errors: {:?}",
+        report.errors
+    );
+    report.frames
+}
+
+fn setup() -> UiScript {
+    let mut s = UiScript::new().unwrap();
+    s.set_screen_size(1024.0, 768.0);
+    load_xml(&s, "Fonts.xml");
+    load_xml(&s, "MoneyFrame.xml");
+    load_xml(&s, "UiPanels.xml");
+    load_xml(&s, "QuestShareFrame.xml");
+    s
+}
+
+fn confirm(s: &mut UiScript, who: &str, quest: &str) {
+    s.fire_event(
+        "QUEST_ACCEPT_CONFIRM",
+        vec![
+            ScriptValue::Str(who.to_string()),
+            ScriptValue::Str(quest.to_string()),
+        ],
+    );
+}
+
+/// The event raises the popup with BOTH args filled, in the reference's order: the player first,
+/// the quest title second (`QUEST_ACCEPT = "%s is starting %s\nWould you like to as well?"`). A
+/// swap would read "Escort Duty is starting Thrall", which is exactly the kind of wrong that looks
+/// right at a glance.
+#[test]
+fn the_confirm_names_the_player_first_and_the_quest_second() {
+    let mut s = setup();
+    confirm(&mut s, "Thrall", "Escort Duty");
+    assert!(
+        s.eval::<bool>("return StaticPopup1:IsVisible()").unwrap(),
+        "QUEST_ACCEPT_CONFIRM shows the confirm popup"
+    );
+    assert_eq!(
+        s.eval::<String>("return StaticPopup1Text:GetText()")
+            .unwrap(),
+        "Thrall is starting Escort Duty\nWould you like to as well?"
+    );
+    assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}
+
+/// Yes queues exactly one `ConfirmAcceptQuest()` and closes the popup.
+#[test]
+fn yes_answers_once() {
+    let mut s = setup();
+    confirm(&mut s, "Thrall", "Escort Duty");
+    s.run("StaticPopup_OnClick(StaticPopup1, 1)").unwrap();
+    assert_eq!(s.take_quest_confirms(), 1);
+    assert_eq!(s.take_quest_confirms(), 0, "drained");
+    assert!(
+        !s.eval::<bool>("return StaticPopup1:IsVisible()").unwrap(),
+        "answering closes it"
+    );
+}
+
+/// **No sends nothing.** The reference's `QUEST_ACCEPT` entry has an `OnAccept` and no `OnCancel`
+/// at all (`StaticPopup.lua:727-737`), because there is no decline packet — the server's pending
+/// latch is cleared by the next thing that touches it. Same for ESC.
+#[test]
+fn no_and_escape_send_nothing() {
+    let mut s = setup();
+    confirm(&mut s, "Thrall", "Escort Duty");
+    s.run("StaticPopup_OnClick(StaticPopup1, 2)").unwrap();
+    assert_eq!(s.take_quest_confirms(), 0, "No must not answer the server");
+    assert!(!s.eval::<bool>("return StaticPopup1:IsVisible()").unwrap());
+
+    confirm(&mut s, "Thrall", "Escort Duty");
+    s.run("StaticPopup_EscapePressed()").unwrap();
+    assert_eq!(s.take_quest_confirms(), 0, "ESC must not answer the server");
+    assert!(!s.eval::<bool>("return StaticPopup1:IsVisible()").unwrap());
+}
+
+/// A second confirm replaces the first rather than stacking — the server keeps ONE share latch per
+/// player, so an older question is already dead by the time a newer one arrives, and answering the
+/// stale popup would send the wrong quest id.
+#[test]
+fn a_second_confirm_replaces_the_first() {
+    let mut s = setup();
+    confirm(&mut s, "Thrall", "Escort Duty");
+    confirm(&mut s, "Jaina", "Deeper Still");
+    assert_eq!(
+        s.eval::<String>("return StaticPopup1Text:GetText()")
+            .unwrap(),
+        "Jaina is starting Deeper Still\nWould you like to as well?"
+    );
+    assert!(
+        !s.eval::<bool>("return StaticPopup2:IsVisible()").unwrap(),
+        "the confirm is exclusive — it reuses the one slot"
+    );
+}
@@ -4,8 +4,8 @@
 //! `quest_tests.rs`/`bag_tests.rs`'s engine-only harness for the quest-log slice (decision 0088 arc).

 use benilla_ui::script::{
-    ExtractedQuad, QuadContent, QuestItemView, QuestLogDetail, QuestLogEntryView,
-    QuestLogObjectiveView, QuestLogState, SoundRequest, UiScript,
+    ExtractedQuad, PartyMemberInfo, PartyState, QuadContent, QuestItemView, QuestLogDetail,
+    QuestLogEntryView, QuestLogObjectiveView, QuestLogState, SoundRequest, UiScript,
 };

 /// Load one shipped `assets/ui/<file>` into `s`, panicking on any loader error (the panel/quest
@@ -1172,3 +1172,144 @@ fn shift_click_on_a_title_posts_the_quest_name_with_chat_open_and_watches_with_i
    assert_eq!(s.eval::<i64>("return GetQuestLogSelection()").unwrap(), 1);
    assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
 }
+
+// ── The Share Quest button (decision 1733) ───────────────────────────────────────────────────────
+
+/// A party of `n` others; `GetNumPartyMembers` is that list's length.
+fn party(n: usize) -> PartyState {
+    PartyState {
+        members: (0..n)
+            .map(|i| PartyMemberInfo {
+                name: format!("Mate{i}"),
+                guid: 0x300 + i as u64,
+            })
+            .collect(),
+        ..PartyState::default()
+    }
+}
+
+/// The 8-entry fixture with `pushable` set on the entries named by `ids` — the app sets it from
+/// the cached template's `QUEST_FLAGS_SHARABLE` bit, so a fixture is how the button's predicate is
+/// exercised without a wire.
+fn entries_sharable(ids: &[u32]) -> QuestLogState {
+    let mut state = eight_entries();
+    for e in &mut state.entries {
+        e.pushable = ids.contains(&e.quest_id);
+    }
+    state
+}
+
+/// The button's predicate is a CONJUNCTION, and each half is tested by moving only itself: a
+/// sharable quest with no party is dark, a party with an unsharable selection is dark, and only
+/// both together light it (ref `QuestLogFrame.lua:299-305`).
+#[test]
+fn share_quest_needs_both_a_sharable_selection_and_a_party() {
+    let mut s = UiScript::new().unwrap();
+    s.set_screen_size(1024.0, 768.0);
+    load_xml(&s, "Fonts.xml");
+    load_xml(&s, "MoneyFrame.xml");
+    load_xml(&s, "UiPanels.xml");
+    load_xml(&s, "MerchantFrame.xml");
+    load_xml(&s, "QuestLogFrame.xml");
+
+    // Solo, quest 1 sharable and selected by the auto-selection.
+    s.set_quest_log(entries_sharable(&[1]));
+    s.set_party(PartyState::default());
+    s.run("ToggleQuestLog()").unwrap();
+    assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+    assert_eq!(s.eval::<i64>("return GetQuestLogSelection()").unwrap(), 1);
+    // `1`/nil, never true/false (decision 1738) — compared the way the reference's own predicate
+    // does, which is why the button below reads the same either way.
+    assert!(
+        s.eval::<bool>("return GetQuestLogPushable() ~= nil")
+            .unwrap(),
+        "quest 1 carries the sharable bit"
+    );
+    assert!(
+        !s.eval::<bool>("return QuestLogPushQuestButton:IsEnabled()")
+            .unwrap(),
+        "solo: sharable is not enough"
+    );
+
+    // Join a party — the event alone must light it, with no quest-log change at all.
+    s.set_party(party(2));
+    s.fire_event("PARTY_MEMBERS_CHANGED", vec![]);
+    assert!(
+        s.eval::<bool>("return QuestLogPushQuestButton:IsEnabled()")
+            .unwrap(),
+        "PARTY_MEMBERS_CHANGED alone re-tests the button"
+    );
+
+    // Select an unsharable quest: party unchanged, button goes dark.
+    s.run("SelectQuestLogEntry(2)").unwrap();
+    s.run("BenillaQuestLogFrame_Update()").unwrap();
+    assert!(
+        s.eval::<bool>("return GetQuestLogPushable() == nil")
+            .unwrap(),
+        "quest 2 has no sharable bit"
+    );
+    assert!(
+        !s.eval::<bool>("return QuestLogPushQuestButton:IsEnabled()")
+            .unwrap(),
+        "in a party: an unsharable selection is still dark"
+    );
+
+    // Leaving the party darkens it again from the other side.
+    s.run("SelectQuestLogEntry(1)").unwrap();
+    s.run("BenillaQuestLogFrame_Update()").unwrap();
+    assert!(s
+        .eval::<bool>("return QuestLogPushQuestButton:IsEnabled()")
+        .unwrap());
+    s.set_party(PartyState::default());
+    s.fire_event("PARTY_MEMBERS_CHANGED", vec![]);
+    assert!(!s
+        .eval::<bool>("return QuestLogPushQuestButton:IsEnabled()")
+        .unwrap());
+}
+
+/// An EMPTY log disables the button outright, ahead of both other conjuncts — the ref's own first
+/// guard, and the one that matters because `GetQuestLogPushable()` on no selection is false anyway
+/// but `GetNumPartyMembers() > 0` is not.
+#[test]
+fn share_quest_is_dark_on_an_empty_log_even_in_a_party() {
+    let mut s = UiScript::new().unwrap();
+    s.set_screen_size(1024.0, 768.0);
+    load_xml(&s, "Fonts.xml");
+    load_xml(&s, "MoneyFrame.xml");
+    load_xml(&s, "UiPanels.xml");
+    load_xml(&s, "MerchantFrame.xml");
+    load_xml(&s, "QuestLogFrame.xml");
+
+    s.set_quest_log(QuestLogState::default());
+    s.set_party(party(4));
+    s.run("ToggleQuestLog()").unwrap();
+    assert!(!s
+        .eval::<bool>("return QuestLogPushQuestButton:IsEnabled()")
+        .unwrap());
+    assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}
+
+/// The click queues the SELECTION's quest id — not its list index, and not the row under the
+/// mouse. The fixture's ids are deliberately not equal to their 1-based positions after a
+/// selection move, so an index/id confusion shows up here.
+#[test]
+fn share_quest_click_queues_the_selected_quests_id() {
+    let mut s = UiScript::new().unwrap();
+    s.set_screen_size(1024.0, 768.0);
+    load_xml(&s, "Fonts.xml");
+    load_xml(&s, "MoneyFrame.xml");
+    load_xml(&s, "UiPanels.xml");
+    load_xml(&s, "MerchantFrame.xml");
+    load_xml(&s, "QuestLogFrame.xml");
+
+    s.set_quest_log(entries_sharable(&[1, 5]));
+    s.set_party(party(1));
+    s.run("ToggleQuestLog()").unwrap();
+
+    s.run("SelectQuestLogEntry(5)").unwrap();
+    s.run("BenillaQuestLogFrame_Update()").unwrap();
+    s.run("QuestLogPushQuestButton:Click()").unwrap();
+    assert_eq!(s.take_quest_log_pushes(), vec![5]);
+    assert!(s.take_quest_log_pushes().is_empty(), "drained");
+    assert!(s.errors().is_empty(), "script errors: {:?}", s.errors());
+}