From 263fe49663ad8d3cc1b3fd8d88e41e3cf941e9ad Mon Sep 17 00:00:00 2001
From: Benjamin Klum <benjamin.klum@helgoboss.org>
Date: Wed, 6 Nov 2024 10:29:03 +0100
Subject: [PATCH] #1311 Make targets that don't return a value cause at least
 minimum-value feedback

---
 main/src/domain/mapping.rs                     | 17 ++++++++++++++++-
 main/src/infrastructure/ui/stream_deck_tool.rs |  4 ----
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/main/src/domain/mapping.rs b/main/src/domain/mapping.rs
index 1b2c73395..6f7b1685a 100644
--- a/main/src/domain/mapping.rs
+++ b/main/src/domain/mapping.rs
@@ -1132,12 +1132,27 @@ impl MainMapping {
         //   whether it wants numerical or textual feedback.
         let prop_provider = MappingPropProvider::new(self, control_context);
         let feedback_value = if self.core.mode.wants_advanced_feedback() {
+            // Advanced feedback (e.g. text expression)
             self.core
                 .mode
                 .build_feedback(&prop_provider, control_context.mode_context)
         } else {
+            // Normal numeric feedback
             let style = self.core.mode.feedback_style(&prop_provider);
-            FeedbackValue::Numeric(NumericFeedbackValue::new(style, combined_target_value?))
+            // https://github.com/helgoboss/helgobox/issues/1311:
+            // Before falling back to UnitValue::MIN, targets that didn't return a value (e.g. actions with invocation
+            // mode trigger) wouldn't cause any feedback. That means nothing happens to the connected feedback element.
+            // It's like feedback is disabled.
+            // This behavior turns out to be weird as soon as you have a display source that wants to display something
+            // (e.g. text), no matter whether the target supports feedback or not: The display will just be empty,
+            // nothing will happen to it. It will not even be switched off, so old content stays there ...
+            // which is misleading.
+            // I think the best way to deal with it is to emit the minimum feedback value = 0%. Emitting 0% is different
+            // from off. "Off" means, the mapping and/or target is inactive. So emitting 0% makes the source aware that
+            // an active target is connected.
+            let final_target_value =
+                combined_target_value.unwrap_or(AbsoluteValue::Continuous(UnitValue::MIN));
+            FeedbackValue::Numeric(NumericFeedbackValue::new(style, final_target_value))
         };
         let cause = if self.core.is_echo() {
             FeedbackCause::Echo
diff --git a/main/src/infrastructure/ui/stream_deck_tool.rs b/main/src/infrastructure/ui/stream_deck_tool.rs
index ff251f12b..24567f012 100644
--- a/main/src/infrastructure/ui/stream_deck_tool.rs
+++ b/main/src/infrastructure/ui/stream_deck_tool.rs
@@ -22,9 +22,6 @@ pub fn create_stream_deck_compartment_reflecting_toolbar(
         .flatten()
         .flatten();
     let reaper_resource_path = Reaper::get().resource_path();
-    // TODO-high CONTINUE How do we deal with targets that can't return a current value? Trigger-like targets?
-    //  Those don't ever send feedback because there's no current value, and as such there's also no feedback value
-    //  and the screen stays as it is.
     let button_mappings = items.enumerate().map(|(i, item)| {
         let action = Reaper::get()
             .main_section()
@@ -97,7 +94,6 @@ pub fn create_stream_deck_compartment_reflecting_toolbar(
                 };
                 Some(cmd)
             },
-            // TODO-high CONTINUE This should be Trigger for trigger-like actions, as soon as the feedback prob is solved
             invocation: {
                 let invocation = match action_character {
                     ActionCharacter::Toggle => ActionInvocationKind::Absolute7Bit,