Fix Insert-as-FX review findings: id rename, selection-count drift, refusal tooltip, success msg, master-only, reserve test

Renames the permanent action id out of the placement family, makes the button's
painted state and pressed outcome share one selection count, surfaces
panel-known refusals in the tooltip, reports the target track on success,
distinguishes a master-only selection, and pins the overflow-reserve test to the
real bar spec.

src/app/main.cpp

@@ -130,7 +130,10 @@ static std::vector<reasampler::ActionTableRow> buildMainActionTable() {
     rows.push_back({"INSERT_SELECTED_CONFORM",
                     "insert selected sample at edit cursor (conform to tempo)",
                     &RunInsertSelected, 1});
-    rows.push_back({"INSERT_AS_FX",
+    // A LOAD_*, not an INSERT_*: INSERT_SELECTED/_CONFORM place a timeline item;
+    // this loads an instrument onto a track's FX chain. The id is permanent, so
+    // keeping the two apart here is the most durable statement of which verb this is.
+    rows.push_back({"LOAD_INSTRUMENT_ON_TRACK",
                     "insert selected sample as ReaSampler 9000 on the selected track",
                     &RunInsertAsFx});
     // One action fires N captures (per selected item / per razor area); the original

src/core/ui/insert_fx_enable.cpp

@@ -4,10 +4,12 @@
 
 namespace reasampler::ui {
 
-InsertFxRefusal insertFxRefusal(int focusedCaptureCount, bool trackSelected) {
+InsertFxRefusal insertFxRefusal(int focusedCaptureCount, bool trackSelected,
+                                bool masterOnlySelected) {
     if (focusedCaptureCount <= 0) return InsertFxRefusal::NoCapture;
     if (focusedCaptureCount > 1) return InsertFxRefusal::MultiCapture;
-    if (!trackSelected) return InsertFxRefusal::NoTrack;
+    if (!trackSelected)
+        return masterOnlySelected ? InsertFxRefusal::MasterOnlySelected : InsertFxRefusal::NoTrack;
     return InsertFxRefusal::None;
 }
 
@@ -29,6 +31,9 @@ std::string insertFxRefusalMessage(InsertFxRefusal refusal) {
         case InsertFxRefusal::NoTrack:
             return "select a track first -- the instrument is added to the selected "
                    "track's FX chain";
+        case InsertFxRefusal::MasterOnlySelected:
+            return "select a track other than the master -- the master track cannot "
+                   "host the instrument";
     }
     return {};
 }

src/core/ui/insert_fx_enable.h

@@ -17,9 +17,11 @@ enum class InsertFxRefusal {
     NoCapture,
     MultiCapture,
     NoTrack,
+    MasterOnlySelected,  // only the master track is selected — GetSelectedTrack ignores it
 };
 
-InsertFxRefusal insertFxRefusal(int focusedCaptureCount, bool trackSelected);
+InsertFxRefusal insertFxRefusal(int focusedCaptureCount, bool trackSelected,
+                                bool masterOnlySelected = false);
 
 // The button's live/dead bit: the payload axis alone, derived from the same fold so the
 // drawn state and the pressed outcome cannot disagree about the capture rules.

src/shell/actions/insert_fx_action.cpp

@@ -16,6 +16,9 @@
 
 #define REAPERAPI_MINIMAL
 #define REAPERAPI_WANT_GetSelectedTrack
+#define REAPERAPI_WANT_GetMasterTrack
+#define REAPERAPI_WANT_GetMediaTrackInfo_Value
+#define REAPERAPI_WANT_GetTrackName
 #define REAPERAPI_WANT_ShowConsoleMsg
 #include "reaper_plugin_functions.h"
 
@@ -27,6 +30,23 @@ void report(const std::string& sentence) {
     if (ShowConsoleMsg) ShowConsoleMsg(("ReaSampler: " + sentence + "\n").c_str());
 }
 
+// GetSelectedTrack ignores the master, so a master-only selection reads back as "no
+// track" -- distinguish it here for a clearer refusal (core/ui/insert_fx_enable.h).
+bool isMasterOnlySelected() {
+    if (!GetMasterTrack || !GetMediaTrackInfo_Value) return false;
+    MediaTrack* master = GetMasterTrack(nullptr);
+    return master && GetMediaTrackInfo_Value(master, "I_SELECTED") != 0.0;
+}
+
+// "Track N" for an unnamed track, matching REAPER's own convention (GetTrackName,
+// not P_NAME, for the same reason scope_resolve::trackName picks it).
+std::string trackDisplayName(MediaTrack* tr) {
+    std::vector<char> buf(256, '\0');
+    if (GetTrackName && GetTrackName(tr, buf.data(), static_cast<int>(buf.size())))
+        return std::string(buf.data());
+    return "the selected track";
+}
+
 }  // namespace
 
 void doInsertAsFx() {
@@ -37,7 +57,8 @@ void doInsertAsFx() {
     MediaTrack* track = GetSelectedTrack ? GetSelectedTrack(nullptr, 0) : nullptr;
 
     const ui::InsertFxRefusal refusal =
-        ui::insertFxRefusal(static_cast<int>(selected.size()), track != nullptr);
+        ui::insertFxRefusal(static_cast<int>(selected.size()), track != nullptr,
+                            !track && isMasterOnlySelected());
     if (refusal != ui::InsertFxRefusal::None) {
         report(ui::insertFxRefusalMessage(refusal));
         return;
@@ -49,6 +70,9 @@ void doInsertAsFx() {
     if (!performInstrumentDrop(track, wire::buildInstrumentDropPreset(selected.front())))
         report("could not add ReaSampler 9000 to the selected track -- check the plug-in "
                "is installed and scanned");
+    else
+        report("added ReaSampler 9000 to \"" + trackDisplayName(track) +
+               "\", loaded with the selected capture");
 }
 
 }  // namespace reasampler

src/shell/panel/panel_layout.cpp

@@ -131,11 +131,14 @@ std::vector<ActionBarRow> topBarRows() {
     // The one top-bar row with a live/dead gate: it places the PLAYER, and one instance
     // holds one capture. Only the payload half of the fold is knowable at draw time —
     // core/ui/insert_fx_enable.h states why the track half stays a press-time message.
-    rows.push_back({"INSERT_AS_FX", "Insert as FX",
+    // Counts through focusedSelectionIds() (not the raw selection) so the drawn state
+    // can't diverge from doInsertAsFx's own count — the raw selection can go stale
+    // against live membership (e.g. an undo/redo book restore) without being cleared.
+    rows.push_back({"LOAD_INSTRUMENT_ON_TRACK", "Insert as FX",
                     "insert selected sample as ReaSampler 9000 on the selected track",
                     ActionCluster::Placement,
                     insertFxButtonEnabled(
-                        static_cast<int>(g_panel.selection.indices.size()))});
+                        static_cast<int>(focusedSelectionIds().size()))});
     return rows;
 }
 
@@ -276,12 +279,27 @@ bool currentTooltip(int w, int h, std::string& textOut, int& ax, int& ay, int& a
         if (s.index == hv.index) { slot = &s; break; }
     if (!slot) return false;
 
+    const ActionBarRow& row = rows[static_cast<std::size_t>(hv.index)];
+
+    // Insert as FX paints dead only for the payload axis (NoCapture/MultiCapture);
+    // speak that refusal here rather than the generic phrase, so a dead button says
+    // WHY. NoTrack stays a press-time message (insert_fx_enable.h: track selection
+    // has no change callback, so it is only ever knowable at press time).
+    if (row.suffix == "LOAD_INSTRUMENT_ON_TRACK" && !row.enabled) {
+        const InsertFxRefusal refusal =
+            insertFxRefusal(static_cast<int>(focusedSelectionIds().size()), /*trackSelected=*/true);
+        if (refusal != InsertFxRefusal::None) {
+            textOut = insertFxRefusalMessage(refusal);
+            ax = slot->x; ay = slot->y; aw = slot->width; ah = slot->height;
+            return true;
+        }
+    }
+
     // fullName is stored prefix-free; strip defensively in case a source ever
     // carries the display prefix. Tooltip carries name + binding when bound
     // (e.g. "capture selected item — F5"), name only when unbound.
-    const std::string phrase = stripActionPrefix(rows[static_cast<std::size_t>(hv.index)].fullName,
+    const std::string phrase = stripActionPrefix(row.fullName, actionDisplayPrefix());
-                                                 actionDisplayPrefix());
+    const int cmd = resolveBarCommandId(row);
-    const int cmd = resolveBarCommandId(rows[static_cast<std::size_t>(hv.index)]);
     const std::string binding = barBindingText(cmd);
     textOut = binding.empty() ? phrase : phrase + " \xe2\x80\x94 " + binding;  // " — " (em dash, UTF-8)
     ax = slot->x; ay = slot->y; aw = slot->width; ah = slot->height;

src/shell/panel/panel_state.h

@@ -135,6 +135,9 @@ using ui::hitTestPruneButton;
 using ui::hitTestSlot;
 using ui::hitTestTabStrip;
 using ui::insertFxButtonEnabled;
+using ui::insertFxRefusal;
+using ui::insertFxRefusalMessage;
+using ui::InsertFxRefusal;
 using ui::kCardNameScrimAlpha;
 using ui::kCardStripHeight;
 using ui::kCardStripPad;

tests/test_action_bar.cpp

@@ -229,6 +229,48 @@ static void testOverflowReserveHonouredAcrossWidths() {
     CHECK(sawReserveCostAButton);
 }
 
+// --- Reserve against the REAL panel spec: names the button and the exact width -----
+
+// roundSpec() above uses clusterGap=16 for hand-checkable pixel math; the panel's actual
+// bar (shell/panel/panel_state.h's kBarSpec) uses clusterGap=24. Reproduced literally here
+// because this test target is REAPER-free and cannot include that shell header -- keep
+// the two in sync by hand if either spec ever changes.
+static ActionBarSpec realBarSpec() {
+    ActionBarSpec s;
+    s.buttonWidth = 108;
+    s.buttonGap = 4;
+    s.clusterGap = 24;
+    s.sidePad = 8;
+    s.verticalInset = 3;
+    return s;
+}
+
+// At the real spec plus the real 40 px reserve (28 + 2*6, panel_state.h's kMenuBtnSpec --
+// all default-constructed here since its defaults already match), 764 is the narrowest
+// band that still shows all six buttons; one pixel narrower drops Insert as FX (flat index
+// 5, the trailing Placement button) alone into overflow, leaving its five neighbours intact.
+static void testReserveDropsInsertAsFxAtRealWidth() {
+    const auto clusters = inventory();
+    const ActionBarSpec spec = realBarSpec();
+    const MenuButtonSpec menuSpec;
+    const int reserve = menuButtonReserve(MenuBarRect{0, 0, 900, 34}, menuSpec);
+    CHECK(reserve == 40);
+
+    const int allSixWidth = 764;
+    {
+        ActionBarRect action{0, 0, allSixWidth - reserve, 34};
+        const auto slots = computeBarSlots(action, clusters, spec);
+        CHECK(slots.size() == 6);
+        CHECK(slots.back().index == 5);  // Insert as FX still visible
+    }
+    {
+        ActionBarRect action{0, 0, allSixWidth - 1 - reserve, 34};
+        const auto slots = computeBarSlots(action, clusters, spec);
+        CHECK(slots.size() == 5);        // Insert as FX alone dropped
+        CHECK(slots.back().index == 4);  // Insert Conform is now the trailing visible button
+    }
+}
+
 // --- Degenerate --------------------------------------------------------------
 
 static void testDegenerate() {
@@ -356,6 +398,7 @@ int main() {
     testOverflowDropsTrailingWhole();
     testTooNarrowForAny();
     testOverflowReserveHonouredAcrossWidths();
+    testReserveDropsInsertAsFxAtRealWidth();
     testDegenerate();
     testHitTestHitsButtons();
     testHitTestGapsAreMisses();

tests/test_insert_fx_enable.cpp

@@ -27,6 +27,18 @@ static void testEveryCombination() {
     CHECK(insertFxRefusal(3, true) == InsertFxRefusal::MultiCapture);
 }
 
+// GetSelectedTrack ignores the master track, so "only the master is selected" is a
+// distinct refusal from a plain empty selection -- both are !trackSelected, but the
+// message differs so a master-only user isn't told "select a track" when they did.
+static void testMasterOnlySelectedIsDistinctFromNoTrack() {
+    CHECK(insertFxRefusal(1, false, /*masterOnlySelected=*/true) ==
+          InsertFxRefusal::MasterOnlySelected);
+    CHECK(insertFxRefusal(1, false, /*masterOnlySelected=*/false) == InsertFxRefusal::NoTrack);
+    // The payload axis still runs first -- a bad capture count refuses on ITS reason
+    // even with the master selected.
+    CHECK(insertFxRefusal(0, false, /*masterOnlySelected=*/true) == InsertFxRefusal::NoCapture);
+}
+
 // Exactly one capture is the only count that can run; two is already too many.
 static void testOnlyASingleCaptureRuns() {
     CHECK(insertFxRefusal(2, true) == InsertFxRefusal::MultiCapture);
@@ -68,18 +80,24 @@ static void testEveryRefusalHasItsOwnSentence() {
     const std::string noCap = insertFxRefusalMessage(InsertFxRefusal::NoCapture);
     const std::string multi = insertFxRefusalMessage(InsertFxRefusal::MultiCapture);
     const std::string noTrk = insertFxRefusalMessage(InsertFxRefusal::NoTrack);
+    const std::string masterOnly = insertFxRefusalMessage(InsertFxRefusal::MasterOnlySelected);
 
     CHECK(none.empty());
     CHECK(!noCap.empty());
     CHECK(!multi.empty());
     CHECK(!noTrk.empty());
+    CHECK(!masterOnly.empty());
     CHECK(noCap != multi);
     CHECK(noCap != noTrk);
     CHECK(multi != noTrk);
+    CHECK(masterOnly != noTrk);
+    CHECK(masterOnly != noCap);
+    CHECK(masterOnly != multi);
 }
 
 int main() {
     testEveryCombination();
+    testMasterOnlySelectedIsDistinctFromNoTrack();
     testOnlyASingleCaptureRuns();
     testNegativeCountRefuses();
     testButtonBitIsThePayloadAxisOfTheFold();