From cba21a768f81fdc7484f8691baec8d5780b17c79 Mon Sep 17 00:00:00 2001 From: Oblarg Date: Sun, 12 Jan 2020 17:57:28 -0500 Subject: [PATCH] Fix C++ JoystickButton and POVButton (#2259) C++ JoystickButton and POVButton were both nonfunctional due to slicing when trigger passes itself by value to the button scheduler it creates. Fix is to remove the virtual Get() method entirely and use only the m_isActive functor; since the subclass now passes the button condition back as a functor to the base class, in which it's stored as a member, it will now still work after being sliced. --- .../cpp/frc2/command/button/Trigger.cpp | 24 +++++++------- .../frc2/command/button/JoystickButton.h | 12 +++---- .../include/frc2/command/button/POVButton.h | 15 +++------ .../include/frc2/command/button/Trigger.h | 33 ++++++++----------- 4 files changed, 33 insertions(+), 51 deletions(-) diff --git a/wpilibNewCommands/src/main/native/cpp/frc2/command/button/Trigger.cpp b/wpilibNewCommands/src/main/native/cpp/frc2/command/button/Trigger.cpp index 5c1a8f707b..875b54efef 100644 --- a/wpilibNewCommands/src/main/native/cpp/frc2/command/button/Trigger.cpp +++ b/wpilibNewCommands/src/main/native/cpp/frc2/command/button/Trigger.cpp @@ -15,8 +15,8 @@ Trigger::Trigger(const Trigger& other) : m_isActive(other.m_isActive) {} Trigger Trigger::WhenActive(Command* command, bool interruptible) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, command, interruptible]() mutable { - bool pressed = Get(); + [pressedLast = m_isActive(), *this, command, interruptible]() mutable { + bool pressed = m_isActive(); if (!pressedLast && pressed) { command->Schedule(interruptible); @@ -41,8 +41,8 @@ Trigger Trigger::WhenActive(std::function toRun, Trigger Trigger::WhileActiveContinous(Command* command, bool interruptible) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, command, interruptible]() mutable { - bool pressed = Get(); + [pressedLast = m_isActive(), *this, command, interruptible]() mutable { + bool pressed = m_isActive(); if (pressed) { command->Schedule(interruptible); @@ -70,8 +70,8 @@ Trigger Trigger::WhileActiveContinous(std::function toRun, Trigger Trigger::WhileActiveOnce(Command* command, bool interruptible) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, command, interruptible]() mutable { - bool pressed = Get(); + [pressedLast = m_isActive(), *this, command, interruptible]() mutable { + bool pressed = m_isActive(); if (!pressedLast && pressed) { command->Schedule(interruptible); @@ -86,8 +86,8 @@ Trigger Trigger::WhileActiveOnce(Command* command, bool interruptible) { Trigger Trigger::WhenInactive(Command* command, bool interruptible) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, command, interruptible]() mutable { - bool pressed = Get(); + [pressedLast = m_isActive(), *this, command, interruptible]() mutable { + bool pressed = m_isActive(); if (pressedLast && !pressed) { command->Schedule(interruptible); @@ -111,8 +111,8 @@ Trigger Trigger::WhenInactive(std::function toRun, Trigger Trigger::ToggleWhenActive(Command* command, bool interruptible) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, command, interruptible]() mutable { - bool pressed = Get(); + [pressedLast = m_isActive(), *this, command, interruptible]() mutable { + bool pressed = m_isActive(); if (!pressedLast && pressed) { if (command->IsScheduled()) { @@ -129,8 +129,8 @@ Trigger Trigger::ToggleWhenActive(Command* command, bool interruptible) { Trigger Trigger::CancelWhenActive(Command* command) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, command]() mutable { - bool pressed = Get(); + [pressedLast = m_isActive(), *this, command]() mutable { + bool pressed = m_isActive(); if (!pressedLast && pressed) { command->Cancel(); diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/button/JoystickButton.h b/wpilibNewCommands/src/main/native/include/frc2/command/button/JoystickButton.h index a23738bfa8..421562ed0f 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/button/JoystickButton.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/button/JoystickButton.h @@ -1,5 +1,5 @@ /*----------------------------------------------------------------------------*/ -/* Copyright (c) 2019 FIRST. All Rights Reserved. */ +/* Copyright (c) 2019-2020 FIRST. All Rights Reserved. */ /* Open Source Software - may be modified and shared by FRC teams. The code */ /* must be accompanied by the FIRST BSD license file in the root directory of */ /* the project. */ @@ -26,12 +26,8 @@ class JoystickButton : public Button { * @param buttonNumber The number of the button on the joystic. */ explicit JoystickButton(frc::GenericHID* joystick, int buttonNumber) - : m_joystick{joystick}, m_buttonNumber{buttonNumber} {} - - bool Get() const override { return m_joystick->GetRawButton(m_buttonNumber); } - - private: - frc::GenericHID* m_joystick; - int m_buttonNumber; + : Button([joystick, buttonNumber] { + return joystick->GetRawButton(buttonNumber); + }) {} }; } // namespace frc2 diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/button/POVButton.h b/wpilibNewCommands/src/main/native/include/frc2/command/button/POVButton.h index 758cab2056..bc81beaac2 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/button/POVButton.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/button/POVButton.h @@ -1,5 +1,5 @@ /*----------------------------------------------------------------------------*/ -/* Copyright (c) 2019 FIRST. All Rights Reserved. */ +/* Copyright (c) 2019-2020 FIRST. All Rights Reserved. */ /* Open Source Software - may be modified and shared by FRC teams. The code */ /* must be accompanied by the FIRST BSD license file in the root directory of */ /* the project. */ @@ -27,15 +27,8 @@ class POVButton : public Button { * @param povNumber The number of the POV on the joystick. */ POVButton(frc::GenericHID* joystick, int angle, int povNumber = 0) - : m_joystick{joystick}, m_angle{angle}, m_povNumber{povNumber} {} - - bool Get() const override { - return m_joystick->GetPOV(m_povNumber) == m_angle; - } - - private: - frc::GenericHID* m_joystick; - int m_angle; - int m_povNumber; + : Button([joystick, angle, povNumber] { + joystick->GetPOV(povNumber) == angle; + }) {} }; } // namespace frc2 diff --git a/wpilibNewCommands/src/main/native/include/frc2/command/button/Trigger.h b/wpilibNewCommands/src/main/native/include/frc2/command/button/Trigger.h index 7df6b4ecbb..a40779f5a1 100644 --- a/wpilibNewCommands/src/main/native/include/frc2/command/button/Trigger.h +++ b/wpilibNewCommands/src/main/native/include/frc2/command/button/Trigger.h @@ -47,13 +47,6 @@ class Trigger { Trigger(const Trigger& other); - /** - * Returns whether the trigger is active. Can be overridden by a subclass. - * - * @return Whether the trigger is active. - */ - virtual bool Get() const { return m_isActive(); } - /** * Binds a command to start when the trigger becomes active. Takes a * raw pointer, and so is non-owning; users are responsible for the lifespan @@ -79,11 +72,11 @@ class Trigger { Command, std::remove_reference_t>>> Trigger WhenActive(T&& command, bool interruptible = true) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, + [pressedLast = m_isActive(), *this, command = std::make_unique>( std::forward(command)), interruptible]() mutable { - bool pressed = Get(); + bool pressed = m_isActive(); if (!pressedLast && pressed) { command->Schedule(interruptible); @@ -138,11 +131,11 @@ class Trigger { Command, std::remove_reference_t>>> Trigger WhileActiveContinous(T&& command, bool interruptible = true) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, + [pressedLast = m_isActive(), *this, command = std::make_unique>( std::forward(command)), interruptible]() mutable { - bool pressed = Get(); + bool pressed = m_isActive(); if (pressed) { command->Schedule(interruptible); @@ -198,11 +191,11 @@ class Trigger { Command, std::remove_reference_t>>> Trigger WhileActiveOnce(T&& command, bool interruptible = true) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, + [pressedLast = m_isActive(), *this, command = std::make_unique>( std::forward(command)), interruptible]() mutable { - bool pressed = Get(); + bool pressed = m_isActive(); if (!pressedLast && pressed) { command->Schedule(interruptible); @@ -240,11 +233,11 @@ class Trigger { Command, std::remove_reference_t>>> Trigger WhenInactive(T&& command, bool interruptible = true) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, + [pressedLast = m_isActive(), *this, command = std::make_unique>( std::forward(command)), interruptible]() mutable { - bool pressed = Get(); + bool pressed = m_isActive(); if (pressedLast && !pressed) { command->Schedule(interruptible); @@ -298,11 +291,11 @@ class Trigger { Command, std::remove_reference_t>>> Trigger ToggleWhenActive(T&& command, bool interruptible = true) { CommandScheduler::GetInstance().AddButton( - [pressedLast = Get(), *this, + [pressedLast = m_isActive(), *this, command = std::make_unique>( std::forward(command)), interruptible]() mutable { - bool pressed = Get(); + bool pressed = m_isActive(); if (!pressedLast && pressed) { if (command->IsScheduled()) { @@ -333,7 +326,7 @@ class Trigger { * @return A trigger which is active when both component triggers are active. */ Trigger operator&&(Trigger rhs) { - return Trigger([*this, rhs] { return Get() && rhs.Get(); }); + return Trigger([*this, rhs] { return m_isActive() && rhs.m_isActive(); }); } /** @@ -342,7 +335,7 @@ class Trigger { * @return A trigger which is active when either component trigger is active. */ Trigger operator||(Trigger rhs) { - return Trigger([*this, rhs] { return Get() || rhs.Get(); }); + return Trigger([*this, rhs] { return m_isActive() || rhs.m_isActive(); }); } /** @@ -352,7 +345,7 @@ class Trigger { * and vice-versa. */ Trigger operator!() { - return Trigger([*this] { return !Get(); }); + return Trigger([*this] { return !m_isActive(); }); } private: