CueControl: also reject hotcue endposition writes when unset

Address review feedback on the hotcue-position guard: apply the same
confirm-required + reject-when-unset treatment to hotcue_endposition, so
writing it with no hotcue set no longer stores a bogus value. Route
endposition writes through slotHotcueEndPositionChangeRequest (rejects
when m_pCue is null), and use setAndConfirm() in setEndPosition() since
the control is now confirm-required.

Also trim the verbose comments on the position path per review.

Author: Gary

src/engine/controls/cuecontrol.cpp

@@ -2638,10 +2638,6 @@ HotcueControl::HotcueControl(const QString& group, int hotcueIndex)
             &HotcueControl::slotHotcuePositionChanged,
             Qt::DirectConnection);
     m_hotcuePosition->set(Cue::kNoPosition);
-    // The position may only be moved by dragging an existing hotcue, so route
-    // write requests through a handler that rejects them unless the hotcue is
-    // set. This also makes the control confirm-required, so engine-side updates
-    // must use setAndConfirm() (see HotcueControl::setPosition()).
     m_hotcuePosition->connectValueChangeRequest(
             this,
             &HotcueControl::slotHotcuePositionChangeRequest,
@@ -2655,6 +2651,10 @@ HotcueControl::HotcueControl(const QString& group, int hotcueIndex)
             &HotcueControl::slotHotcueEndPositionChanged,
             Qt::DirectConnection);
     m_hotcueEndPosition->set(Cue::kNoPosition);
+    m_hotcueEndPosition->connectValueChangeRequest(
+            this,
+            &HotcueControl::slotHotcueEndPositionChangeRequest,
+            Qt::DirectConnection);
 
     m_pHotcueStatus = std::make_unique<ControlObject>(keyForControl(QStringLiteral("status")));
     m_pHotcueStatus->setReadOnly();
@@ -2846,21 +2846,25 @@ void HotcueControl::slotHotcuePositionChanged(double newPosition) {
 }
 
 void HotcueControl::slotHotcuePositionChangeRequest(double newPosition) {
-    // Reject the change if no hotcue is set: the position can only be moved by
-    // dragging an existing hotcue, not to create one (see issue #10409).
+    // Reject the change if no hotcue is set
     if (!m_pCue) {
         return;
     }
-    // Delegate to CueControl::hotcuePositionChanged(), which validates the
-    // position and moves the cue. The resulting cue change syncs the position
-    // control back to the confirmed value via loadCuesFromTrack().
     emit hotcuePositionChanged(this, newPosition);
 }
 
 void HotcueControl::slotHotcueEndPositionChanged(double newEndPosition) {
     emit hotcueEndPositionChanged(this, newEndPosition);
 }
 
+void HotcueControl::slotHotcueEndPositionChangeRequest(double newEndPosition) {
+    // Reject the change if no hotcue is set
+    if (!m_pCue) {
+        return;
+    }
+    emit hotcueEndPositionChanged(this, newEndPosition);
+}
+
 void HotcueControl::slotHotcueColorChangeRequest(double newColor) {
     if (newColor < 0 || newColor > 0xFFFFFF) {
         qWarning() << "slotHotcueColorChangeRequest got invalid value:" << newColor;
@@ -2925,14 +2929,13 @@ void HotcueControl::resetCue() {
 }
 
 void HotcueControl::setPosition(mixxx::audio::FramePos position) {
-    // Use setAndConfirm() because m_hotcuePosition is confirm-required: a plain
-    // set() would be intercepted as a change request instead of updating the
-    // engine-authoritative value.
+    // setAndConfirm() because m_hotcuePosition is confirm-required.
     m_hotcuePosition->setAndConfirm(position.toEngineSamplePosMaybeInvalid());
 }
 
 void HotcueControl::setEndPosition(mixxx::audio::FramePos endPosition) {
-    m_hotcueEndPosition->set(endPosition.toEngineSamplePosMaybeInvalid());
+    // setAndConfirm() because m_hotcueEndPosition is confirm-required.
+    m_hotcueEndPosition->setAndConfirm(endPosition.toEngineSamplePosMaybeInvalid());
 }
 
 mixxx::CueType HotcueControl::getType() const {

src/engine/controls/cuecontrol.h

@@ -139,6 +139,7 @@ class HotcueControl : public QObject {
     void slotHotcueEndPositionChanged(double newPosition);
     void slotHotcuePositionChanged(double newPosition);
     void slotHotcuePositionChangeRequest(double newPosition);
+    void slotHotcueEndPositionChangeRequest(double newEndPosition);
     void slotHotcueColorChangeRequest(double newColor);
 
   signals:

src/test/hotcuecontrol_test.cpp

@@ -281,6 +281,74 @@ TEST_F(HotcueControlTest, PositionChangeRequestRejectedWhenUnset) {
             m_pHotcue1Status->get());
 }
 
+TEST_F(HotcueControlTest, EndPositionChangeRequestRejectedWhenUnset) {
+    createAndLoadFakeTrack();
+
+    // With no hotcue set the end position control must reject write requests and
+    // stay unset instead of storing a bogus value (issue #10409).
+    EXPECT_FALSE(mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
+            m_pHotcue1EndPosition->get())
+                    .isValid());
+
+    m_pHotcue1EndPosition->set(mixxx::audio::FramePos(200).toEngineSamplePos());
+    ProcessBuffer();
+
+    EXPECT_FALSE(mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
+            m_pHotcue1EndPosition->get())
+                    .isValid());
+    EXPECT_DOUBLE_EQ(static_cast<double>(HotcueControl::Status::Empty),
+            m_pHotcue1Status->get());
+}
+
+TEST_F(HotcueControlTest, EndPositionChangeRequestMovesSetLoop) {
+    createAndLoadFakeTrack();
+
+    // Create a saved loop hotcue at [100, 200].
+    constexpr mixxx::audio::FramePos loopStartPosition(100);
+    constexpr mixxx::audio::FramePos loopEndPosition(200);
+    m_pChannel1->getEngineBuffer()->setLoop(
+            loopStartPosition, loopEndPosition, true);
+    m_pHotcue1Set->set(1);
+    m_pHotcue1Set->set(0);
+    EXPECT_FRAMEPOS_EQ_CONTROL(loopEndPosition, m_pHotcue1EndPosition);
+
+    // Writing the end position control of a set loop hotcue must still move the
+    // loop end (the confirm-required guard only rejects writes when unset).
+    constexpr mixxx::audio::FramePos newEndPosition(300);
+    m_pHotcue1EndPosition->set(newEndPosition.toEngineSamplePos());
+    ProcessBuffer();
+
+    EXPECT_FRAMEPOS_EQ_CONTROL(newEndPosition, m_pHotcue1EndPosition);
+}
+
+TEST_F(HotcueControlTest, EndPositionChangeRequestClearsSetLoop) {
+    createAndLoadFakeTrack();
+
+    // Create a saved loop hotcue at [100, 200].
+    constexpr mixxx::audio::FramePos loopStartPosition(100);
+    constexpr mixxx::audio::FramePos loopEndPosition(200);
+    m_pChannel1->getEngineBuffer()->setLoop(
+            loopStartPosition, loopEndPosition, true);
+    m_pHotcue1Set->set(1);
+    m_pHotcue1Set->set(0);
+    EXPECT_FRAMEPOS_EQ_CONTROL(loopEndPosition, m_pHotcue1EndPosition);
+
+    // Writing Cue::kNoPosition to the end position of a set loop hotcue clears
+    // the end and converts the loop back into a plain hotcue. This still routes
+    // through the confirm-required handler because the hotcue is set.
+    ControlProxy hotcueType(m_sGroup1, QStringLiteral("hotcue_1_type"));
+    m_pHotcue1EndPosition->set(Cue::kNoPosition);
+    ProcessBuffer();
+
+    EXPECT_FALSE(mixxx::audio::FramePos::fromEngineSamplePosMaybeInvalid(
+            m_pHotcue1EndPosition->get())
+                    .isValid());
+    EXPECT_DOUBLE_EQ(static_cast<double>(mixxx::CueType::HotCue),
+            hotcueType.get());
+    // The hotcue itself remains at its start position.
+    EXPECT_FRAMEPOS_EQ_CONTROL(loopStartPosition, m_pHotcue1Position);
+}
+
 TEST_F(HotcueControlTest, PositionChangeRequestMovesSetHotcue) {
     createAndLoadFakeTrack();