Merge pS-w1-t5-drop: fix drop-to-FX hotspot classifier, narrowed to tcp.fx/mcp.fx (S-VIEW-BUG-1)

src/instrument_drop.cpp

@@ -39,6 +39,18 @@ std::string buildInstrumentDropChunk(const std::string& sampleId) {
     return encodeBase64(instrumentDropStateBytes(sampleId));
 }
 
+bool infoNamesFxHotspot(const std::string& info) {
+    // See the header contract. Two documented FX-bearing surfaces (SDK §GetThingFromPoint):
+    //   "fx_chain" / "fx_N"  — the FX-chain and floating-FX windows.
+    //   "tcp.fx" / "mcp.fx"  — the TCP / MCP FX button specifically.
+    // Bare "tcp" / "mcp" and any other "tcp.*" / "mcp.*" sub-element (e.g. "tcp.mute",
+    // "tcp.vol") are track-panel hits that are NOT on the FX button — those must not trigger
+    // an instrument drop. Exact-string match for the two .fx tokens; prefix-match for fx_.
+    if (info == "tcp.fx" || info == "mcp.fx") return true;
+    auto startsWith = [&info](const char* p) { return info.rfind(p, 0) == 0; };
+    return startsWith("fx_");
+}
+
 std::string encodeBase64(const std::vector<std::uint8_t>& bytes) {
     std::string out;
     out.reserve(((bytes.size() + 2) / 3) * 4);

src/instrument_drop.h

@@ -49,6 +49,27 @@ namespace reasampler {
 // Deterministic: the same sampleId always yields the same blob (base64 of the same bytes).
 std::string buildInstrumentDropChunk(const std::string& sampleId);
 
+// -- FX-drop-target classification (S-VIEW-BUG-1) ------------------------------
+//
+// Pure classifier for GetThingFromPoint's info string: is the point over a surface where an
+// instrument drop should instantiate ReaSampler 9000 on the resolved track? This is string
+// logic (no REAPER types), so it lives here and is unit-tested outside the DAW — the shell
+// (instrument_drop_win) only supplies the info bytes GetThingFromPoint filled.
+//
+// THE BUG (S-VIEW-BUG-1): the original shell predicate matched ONLY "fx_" — but per the SDK
+// (reaper_plugin_functions.h §GetThingFromPoint) "fx_chain"/"fx_N" are the FX-CHAIN and
+// FLOATING-FX windows; a hit on the TCP/MCP FX button (where the intuitive "drop onto the
+// track's FX chain" gesture lands) reports a string in the "tcp.*"/"mcp.*" family —
+// specifically "tcp.fx" / "mcp.fx". So dropping on the TCP FX button never armed the drop;
+// the gesture fell through and the file dropped to arrange as audio.
+//
+// The fix: two documented FX-bearing surfaces are hotspots (SDK §GetThingFromPoint):
+//   * "fx_chain" / "fx_N"  — the FX-chain and floating-FX windows (prefix "fx_")
+//   * "tcp.fx" / "mcp.fx"  — the TCP / MCP FX button (exact token)
+// Bare "tcp"/"mcp" and any other "tcp.*"/"mcp.*" sub-element (e.g. "tcp.mute") are track-panel
+// hits on non-FX surfaces — they must NOT trigger an instrument drop. Only the FX button does.
+bool infoNamesFxHotspot(const std::string& info);
+
 // The raw (pre-base64) component-state bytes — exposed so the round-trip test can decode them
 // back through the instrument's OWN reader (sample_map::deserializeComponentState) and assert
 // the capture is selected, proving buildInstrumentDropChunk feeds the instrument exactly what

src/instrument_drop_win.cpp

@@ -5,10 +5,10 @@
 
 #include "instrument_drop_win.h"
 
-#include <cstring>
 #include <string>
 
 #include "app_version.h"      // vstPluginName() — the CHANNEL-correct FX name (stable/beta pairing)
+#include "instrument_drop.h"  // infoNamesFxHotspot — the PURE, unit-tested hotspot classifier
 
 #include "reaper_plugin.h"
 
@@ -23,19 +23,6 @@
 
 namespace reasampler {
 
-namespace {
-
-// GetThingFromPoint's info string prefixes (verified against reaper_plugin_functions.h:
-// "Updates infoOut with information such as 'arrange', 'fx_chain', 'fx_0' ... If a track
-// panel is hit, string will begin with 'tcp' or 'mcp' or 'tcp.mute' etc"). The FX region
-// reports "fx_chain" (the FX list area) or "fx_N" (a specific FX button). We treat either
-// as the FX hotspot — the S17 drop target.
-bool infoNamesFxHotspot(const char* info) {
-    return std::strncmp(info, "fx_", 3) == 0;
-}
-
-}  // namespace
-
 FxDropTarget resolveFxDropTarget(int screenX, int screenY) {
     FxDropTarget out;
     char info[256] = {0};
@@ -46,6 +33,8 @@ FxDropTarget resolveFxDropTarget(int screenX, int screenY) {
     MediaTrack* track = GetThingFromPoint(screenX, screenY, info, sizeof(info));
     out.track = track;
     out.overReaperUi = (track != nullptr) || (info[0] != '\0');
+    // S-VIEW-BUG-1: the hotspot is either the FX chain/floating window ("fx_*") OR the track/
+    // mixer panel that hosts the FX button ("tcp*"/"mcp*"). The pure classifier owns the rule.
     out.overFxHotspot = (track != nullptr) && infoNamesFxHotspot(info);
     return out;
 }

src/instrument_drop_win.h

@@ -33,14 +33,14 @@ struct FxDropTarget {
 };
 
 // Hit-test a screen point (REAPER screen coords) to an FX drop target. Wraps
-// GetThingFromPoint, whose info string tells us what was hit ("tcp"/"mcp" for a track panel,
-// "fx_chain"/"fx_N" for the FX area/button). `overReaperUi` is the shell-supplied predicate
-// the pure drag_out::decideGesture consumes (true when the point is over REAPER's own UI —
-// i.e. GetThingFromPoint returned a track OR a recognizable non-track thing, false when the
-// pointer has left REAPER entirely). `overFxHotspot` is true when the info string names the
-// FX region specifically — the S17 "FX-button hotspot vs. whole TCP" question is resolved to
-// the FX hotspot (the discoverable, unambiguous target), decided here from the SDK's own
-// hit-test string rather than a home-grown geometry guess.
+// GetThingFromPoint, whose info string tells us what was hit ("tcp.fx"/"mcp.fx" for the TCP/MCP
+// FX button; "fx_chain"/"fx_N" for the FX-chain and floating-FX windows; bare "tcp"/"mcp" or
+// other "tcp.*"/"mcp.*" tokens for non-FX track-panel regions). `overReaperUi` is the
+// shell-supplied predicate the pure drag_out::decideGesture consumes (true when the point is
+// over REAPER's own UI — i.e. GetThingFromPoint returned a track OR a recognizable non-track
+// thing, false when the pointer has left REAPER entirely). `overFxHotspot` is true only when
+// the info string names a genuine FX-bearing surface ("tcp.fx", "mcp.fx", or "fx_*") —
+// decided by instrument_drop::infoNamesFxHotspot from the SDK's own hit-test string.
 FxDropTarget resolveFxDropTarget(int screenX, int screenY);
 
 // Perform the drop on `track`: add a fresh ReaSampler 9000 instance and inject `chunkBase64`

tests/test_instrument_drop.cpp

@@ -105,6 +105,50 @@ static void testBase64DecodeRejectsMalformed() {
     CHECK(decodeBase64("Zg==Zg==").empty()); // interior padding (pad before the final quad)
 }
 
+// --- FX-hotspot classification (S-VIEW-BUG-1) ---------------------------------
+//
+// THE BUG: dropping a capture onto a track's FX button (in the TCP) never armed the
+// instrument drop, because the old predicate matched only "fx_" — which the SDK reserves for
+// the FX CHAIN / FLOATING-FX windows. A track-panel FX-button hit reports "tcp.fx"/"mcp.fx"
+// (SDK §GetThingFromPoint: "string will begin with 'tcp' or 'mcp' or 'tcp.mute' etc").
+//
+// THE FIX: narrow to two documented FX-bearing surfaces:
+//   * "tcp.fx" / "mcp.fx"  — the TCP/MCP FX button (exact token, NOT bare tcp/mcp)
+//   * "fx_*"               — the FX-chain / floating-FX windows (prefix, as before)
+// Bare "tcp"/"mcp" and any other "tcp.*"/"mcp.*" sub-element (e.g. "tcp.mute", "tcp.vol")
+// are non-FX track-panel regions — an instrument drop must NOT fire there.
+
+// The TCP/MCP FX button specifically — the surface that arms an instrument drop (S-VIEW-BUG-1
+// fix). Under the old "fx_"-only predicate these returned false — the exact miss that produced
+// the "drops as audio to arrange" symptom on the FX button.
+static void testTcpMcpFxButtonIsHotspot() {
+    CHECK(infoNamesFxHotspot("tcp.fx"));    // TCP FX button (SDK token)
+    CHECK(infoNamesFxHotspot("mcp.fx"));    // MCP FX area (SDK token)
+}
+
+// The FX chain / floating-FX windows (the surfaces the ORIGINAL predicate matched) still
+// classify as hotspots — the fix does not regress the "fx_" surface.
+static void testFxWindowStillHotspot() {
+    CHECK(infoNamesFxHotspot("fx_chain"));  // FX chain window
+    CHECK(infoNamesFxHotspot("fx_0"));      // first FX, floating
+    CHECK(infoNamesFxHotspot("fx_12"));     // arbitrary floating-FX index
+}
+
+// Non-FX surfaces are NOT hotspots — a drop here is not an instrument drop (it would fall
+// through to the OS drag / no-op). This includes the bare TCP/MCP tokens and all non-FX
+// "tcp.*"/"mcp.*" sub-elements (e.g. mute button, volume fader, track name, meter).
+static void testNonFxSurfacesAreNotHotspot() {
+    CHECK(!infoNamesFxHotspot("tcp"));       // bare track control panel — NOT an FX hotspot
+    CHECK(!infoNamesFxHotspot("mcp"));       // bare mixer control panel — NOT an FX hotspot
+    CHECK(!infoNamesFxHotspot("tcp.mute"));  // mute button — track panel, not FX
+    CHECK(!infoNamesFxHotspot("tcp.vol"));   // volume fader — track panel, not FX
+    CHECK(!infoNamesFxHotspot("arrange"));
+    CHECK(!infoNamesFxHotspot("spacer_0"));
+    CHECK(!infoNamesFxHotspot(""));          // pointer over nothing REAPER classifies
+    CHECK(!infoNamesFxHotspot("trans"));     // transport
+    CHECK(!infoNamesFxHotspot("envcp"));     // envelope control panel — a track thing, not FX
+}
+
 int main() {
     testBlobRoundTripsThroughInstrumentReader();
     testGuidLikeIdRoundTrips();
@@ -114,6 +158,10 @@ int main() {
     testBase64RoundTripAllBytes();
     testBase64DecodeRejectsMalformed();
 
+    testTcpMcpFxButtonIsHotspot();
+    testFxWindowStillHotspot();
+    testNonFxSurfacesAreNotHotspot();
+
     if (g_fail == 0) std::printf("All tests passed.\n");
     return g_fail ? 1 : 0;
 }