wpilibsuite / allwpilib

Official Repository of WPILibJ and WPILibC
https://wpilib.org/
Other
1.05k stars 612 forks source link

Refactor existing WPILib commands to support suppliers #5302

Closed AngleSideAngle closed 1 year ago

AngleSideAngle commented 1 year ago

Is your feature request related to a problem? Please describe. It seems the best way to handle state at runtime with command factories is with suppliers being fed into commands. However, TrapezoidProfileCommand, for example, doesn't support this. This means it has to be wrapped in a DeferredCommand, leading to a lot of annoying complexity.

Describe the solution you'd like I think the simplest solution to this issue is to replace state with suppliers in the various classes that need it. To continue supporting existing functionality, they could have alternate constructors that set the supplier to supply the arguments.

Also see #5150 for deferred commands, and the pitfall surrounding using ProxyCommand for this.

Gold856 commented 1 year ago

What other commands need to be refactored to use suppliers? #5457 already did that for TrapezoidProfileCommand.

AngleSideAngle commented 1 year ago

I forget exactly what my thought process was when I created this issue, but trapezoid profile command seems like the only command that needs it.