Closed spacey-sooty closed 1 week ago
One case I'm assuming you're referring to is Defer
, which uses Set<Subsystem>
so that not passing any requirements is explicit.
addRequirements
has overloads for both, especially as getRequirements
returns a Set
.
What others are there?
The cases I could find:
CommandScheduler.cancel(Command...)
- Having a Collection<Command>
overload would probably be fine, but there isn't a clear use-case for canceling a Collection<Command>
, and in general we avoid adding stuff that nobody would use.CommandScheduler.isScheduled(Command...)
- I honestly don't know when this method is called with more than one argument, let alone with a collection.CommandScheduler.registerComposedCommands(Command...)
- A Collection<Command>
overload would be useful for SelectCommand
, though it wouldn't have any other uses. (I'd probably make a Set<Command>
overload that skips the check for duplicates, and have the Command...
and Collection<Command>
overloads just do the duplicate checks)CommandScheduler.registerSubsystem(Subsystem...)
- I honestly don't know when this method is called with more than one argument, let alone with a collection. (The name is singular, too)CommandScheduler.schedule(Command...)
- We discourage manually calling schedule()
in user code, and the only times I saw a single call being used to schedule multiple commands at a time, it was with varargs, not an already existing array object. (RobotDisabledCommandTest.java#L124, RobotDisabledCommandTest.java#L148, and CommandScheduleTest#L71)CommandScheduler.unregisterSubsystem(Subsystem...)
- I honestly don't know when this method is called with more than one argument, let alone with a collection. (The name is singular, too)Command.addRequirements(Subsystem...)
- This is covered in #6304Command.alongWith(Command...)
, Command.andThen(Command...)
, Command.deadlineFor(Command...)
, Command.deadlineWith(Command...)
(deprecated), Command.raceWith(Command...)
, Commands.deadline(Command, Command...)
, Commands.parallel(Command...)
, Commands.race(Command...)
, Commands.repeatingSequence(Command...)
, Commands.sequence(Command...)
, ParallelCommandGroup(Command...)
, ParallelDeadlineGroup(Command, Command...)
, ParallelRaceGroup(Command...)
, ScheduleCommand(Command...)
, SequentialCommandGroup(Command...)
, SequentialCommandGroup.addCommands(Command...)
)
I'm ambivalent with these- I don't know of any cases where having a Collection<Command>
overload would be useful, but someone might be able to provide an example.Command.andThen(Runnable, Subsystem...)
, Command.beforeStarting(Runnable, Subsystem...)
, Commands.idle(Subsystem...)
, Commands.run(Runnable, Subsystem...)
, Commands.runEnd(Runnable, Runnable, Subsystem...)
, Commands.runOnce(Runnable, Subsystem...)
, Commands.startEnd(Runnable, Runnable, Subsystem...)
, Commands.startRun(Runnable, Runnable, Subsystem...)
, FunctionalCommand(Runnable, Runnable, Consumer<Boolean>, BooleanSupplier, Subsystem...)
, InstantCommand(Runnable, Subsystem...)
, MecanumControlCommand(...)
(4 overloads), NotifierCommand(Runnable, double, Subsystem...)
, PIDCommand(...)
(2 overloads), ProfiledPIDCommand(...)
(4 overloads), RamseteCommand(...)
(2 overloads, deprecated), RunCommand(Runnable, Subsystem...)
, StartEndCommand(Runnable, Runnable, Subsystem...)
, SwerveControllerCommand(...)
(4 overloads), TrapezoidProfileCommand(...)
(1 overload))
I could see the case for making overloads that take a Set<Subsystem>
for the requirements.Given how trivial it is to convert between the two using Set.of
/Set.toArray
, I don't think we should duplicate everything for rare cases.
Can this be closed?
In many places (eg. Commands.java) things like
Set<Command>
andSet<Subsystem>
are used inconsistently withCommand... commands
orSubsystem... subsystems
. This should be fixed either by adding more overloads so as to support both or by removing some and adding others.