Remove Command* constructors

wpilibNewCommands/src/main/native/cpp/frc2/command/Commands.cpp

@@ -83,11 +83,6 @@ CommandPtr cmd::Either(CommandPtr&& onTrue, CommandPtr&& onFalse,
       .ToPtr();
 }
 
-CommandPtr cmd::Defer(wpi::unique_function<Command*()> supplier,
-                      Requirements requirements) {
-  return DeferredCommand(std::move(supplier), requirements).ToPtr();
-}
-
 CommandPtr cmd::Defer(wpi::unique_function<CommandPtr()> supplier,
                       Requirements requirements) {
   return DeferredCommand(std::move(supplier), requirements).ToPtr();

wpilibNewCommands/src/main/native/cpp/frc2/command/DeferredCommand.cpp

@@ -6,33 +6,20 @@
 
 #include <wpi/sendable/SendableBuilder.h>
 
-#include "frc2/command/PrintCommand.h"
+#include "frc2/command/Commands.h"
 
 using namespace frc2;
 
-DeferredCommand::DeferredCommand(wpi::unique_function<Command*()> supplier,
+DeferredCommand::DeferredCommand(wpi::unique_function<CommandPtr()> supplier,
                                  Requirements requirements)
     : m_supplier{std::move(supplier)} {
   AddRequirements(requirements);
 }
 
-DeferredCommand::DeferredCommand(wpi::unique_function<CommandPtr()> supplier,
-                                 Requirements requirements)
-    : DeferredCommand(
-          [lambdaSupplier = std::move(supplier),
-           holder = std::optional<CommandPtr>{}]() mutable {
-            holder = lambdaSupplier();
-            return holder->get();
-          },
-          requirements) {}
-
 void DeferredCommand::Initialize() {
-  auto cmd = m_supplier();
-  if (cmd != nullptr) {
-    m_command = cmd;
-    CommandScheduler::GetInstance().RequireUngrouped(m_command);
-    m_command->SetComposed(true);
-  }
+  m_command = m_supplier().Unwrap();
+  CommandScheduler::GetInstance().RequireUngrouped(m_command.get());
+  m_command->SetComposed(true);
   m_command->Initialize();
 }
 
@@ -42,7 +29,10 @@ void DeferredCommand::Execute() {
 
 void DeferredCommand::End(bool interrupted) {
   m_command->End(interrupted);
-  m_command = &m_nullCommand;
+  m_command =
+      cmd::Print("[DeferredCommand] Lifecycle function called out-of-order!")
+          .WithName("none")
+          .Unwrap();
 }
 
 bool DeferredCommand::IsFinished() {
@@ -52,13 +42,5 @@ bool DeferredCommand::IsFinished() {
 void DeferredCommand::InitSendable(wpi::SendableBuilder& builder) {
   Command::InitSendable(builder);
   builder.AddStringProperty(
-      "deferred",
-      [this] {
-        if (m_command != &m_nullCommand) {
-          return m_command->GetName();
-        } else {
-          return std::string{"null"};
-        }
-      },
-      nullptr);
+      "deferred", [this] { return m_command->GetName(); }, nullptr);
 }

wpilibNewCommands/src/main/native/cpp/frc2/command/Subsystem.cpp

@@ -55,10 +55,6 @@ CommandPtr Subsystem::RunEnd(std::function<void()> run,
   return cmd::RunEnd(std::move(run), std::move(end), {this});
 }
 
-CommandPtr Subsystem::Defer(wpi::unique_function<Command*()> supplier) {
-  return cmd::Defer(std::move(supplier), {this});
-}
-
 CommandPtr Subsystem::Defer(wpi::unique_function<CommandPtr()> supplier) {
   return cmd::Defer(std::move(supplier), {this});
 }

wpilibNewCommands/src/main/native/include/frc2/command/Commands.h

@@ -141,16 +141,6 @@ CommandPtr Select(std::function<Key()> selector,
   return SelectCommand(std::move(selector), std::move(vec)).ToPtr();
 }
 
-/**
- * Runs the command supplied by the supplier.
- *
- * @param supplier the command supplier
- * @param requirements the set of requirements for this command
- */
-[[nodiscard]]
-CommandPtr Defer(wpi::unique_function<Command*()> supplier,
-                 Requirements requirements);
-
 /**
  * Runs the command supplied by the supplier.
  *

wpilibNewCommands/src/main/native/include/frc2/command/DeferredCommand.h

@@ -28,20 +28,6 @@ namespace frc2 {
  */
 class DeferredCommand : public CommandHelper<Command, DeferredCommand> {
  public:
-  /**
-   * Creates a new DeferredCommand that runs the supplied command when
-   * initialized, and ends when it ends. Useful for lazily
-   * creating commands at runtime. The supplier will be called each time this
-   * command is initialized. The supplier <i>must</i> create a new Command each
-   * call.
-   *
-   * @param supplier The command supplier
-   * @param requirements The command requirements.
-   *
-   */
-  DeferredCommand(wpi::unique_function<Command*()> supplier,
-                  Requirements requirements);
-
   /**
    * Creates a new DeferredCommand that runs the supplied command when
    * initialized, and ends when it ends. Useful for lazily
@@ -69,8 +55,7 @@ class DeferredCommand : public CommandHelper<Command, DeferredCommand> {
   void InitSendable(wpi::SendableBuilder& builder) override;
 
  private:
-  PrintCommand m_nullCommand{"[DeferredCommand] Supplied command was null!"};
-  wpi::unique_function<Command*()> m_supplier;
-  Command* m_command{&m_nullCommand};
+  wpi::unique_function<CommandPtr()> m_supplier;
+  std::unique_ptr<Command> m_command;
 };
 }  // namespace frc2

wpilibNewCommands/src/main/native/include/frc2/command/Subsystem.h

@@ -151,16 +151,6 @@ class Subsystem {
   [[nodiscard]]
   CommandPtr RunEnd(std::function<void()> run, std::function<void()> end);
 
-  /**
-   * Constructs a DeferredCommand with the provided supplier. This subsystem is
-   * added as a requirement.
-   *
-   * @param supplier the command supplier.
-   * @return the command.
-   */
-  [[nodiscard]]
-  CommandPtr Defer(wpi::unique_function<Command*()> supplier);
-
   /**
    * Constructs a DeferredCommand with the provided supplier. This subsystem is
    * added as a requirement.

wpilibNewCommands/src/test/native/cpp/frc2/command/DeferredCommandTest.cpp

@@ -5,26 +5,46 @@
 #include "CommandTestBase.h"
 #include "frc2/command/Commands.h"
 #include "frc2/command/DeferredCommand.h"
+#include "frc2/command/FunctionalCommand.h"
 
 using namespace frc2;
 
 class DeferredCommandTest : public CommandTestBase {
  public:
-  void DeferredFunctionsTest(bool interrupted) {
-    std::unique_ptr<MockCommand> innerHolder = std::make_unique<MockCommand>();
-    MockCommand* innerCommand = innerHolder.get();
-    DeferredCommand deferred{[innerCommand] { return innerCommand; }, {}};
+  void DeferredFunctionsTest(bool testInterrupted) {
+    int initializeCount = 0;
+    int executeCount = 0;
+    int isFinishedCount = 0;
+    int endCount = 0;
+    bool finished = false;
 
-    EXPECT_CALL(*innerCommand, Initialize());
-    EXPECT_CALL(*innerCommand, Execute());
-    EXPECT_CALL(*innerCommand, End(interrupted));
+    DeferredCommand deferred{[&] {
+                               return FunctionalCommand{
+                                   [&] { initializeCount++; },
+                                   [&] { executeCount++; },
+                                   [&](bool interrupted) {
+                                     EXPECT_EQ(interrupted, testInterrupted);
+                                     endCount++;
+                                   },
+                                   [&] {
+                                     isFinishedCount++;
+                                     return finished;
+                                   }}
+                                   .ToPtr();
+                             },
+                             {}};
 
     deferred.Initialize();
+    EXPECT_EQ(1, initializeCount);
     deferred.Execute();
+    EXPECT_EQ(1, executeCount);
     EXPECT_FALSE(deferred.IsFinished());
-    innerCommand->SetFinished(true);
+    EXPECT_EQ(1, isFinishedCount);
+    finished = true;
     EXPECT_TRUE(deferred.IsFinished());
-    deferred.End(interrupted);
+    EXPECT_EQ(2, isFinishedCount);
+    deferred.End(testInterrupted);
+    EXPECT_EQ(1, endCount);
   }
 };
 
@@ -56,14 +76,3 @@ TEST_F(DeferredCommandTest, DeferredRequirements) {
 
   EXPECT_TRUE(command.GetRequirements().contains(&subsystem));
 }
-
-TEST_F(DeferredCommandTest, DeferredNullCommand) {
-  DeferredCommand command{[] { return nullptr; }, {}};
-
-  EXPECT_NO_THROW({
-    command.Initialize();
-    command.Execute();
-    command.IsFinished();
-    command.End(false);
-  });
-}