allwpilib
allwpilib copied to clipboard
wpilibsuite
[docs] [commands] Disambiguate ProxyCommand and DeferredCommand and deprecate ProxyCommand supplier constructor
There have been many instances of confusion around the differences between ProxyCommand and DeferredCommand, and while reading the source answers most of these questions it would be better for the documentation to clearly explain the use cases and differences of each. ProxyCommand docs now explicitly state its use case as an escape hatch from automatic command composition requirements. DeferredCommand docs state that it is for scheduling a command at scheduling time instead of binding time.
C++ docs and wpiformat will be done once the java wording and coverage is approved, since it's just a copy over.
Fixes #5244
For what it's worth, the proxy command supplier constructor is very useful in scheduling a command from a SendableChooser, since you can have trigger.onTrue(new ProxyCommand(chooser::get))
Yeah, that's a use case that would get cut by this.
We should have a factory that sugars combining a SelectCommand and SendableChooser.
I don't see that as a valid use-case for ProxyCommand - that should be a DeferredCommand, which is the whole point of this change.
DeferredCommand wouldn't work here because command requirements from a SendableChooser need to be evaluated at runtime
I don't follow why that needs to be the case, but if you need to escape from the requirement algebra you can use proxy for that, but not for the deferring. It is not really any harder to type proxy(defer(...)).
I think that the point here is that Deferred is for creating a command at runtime, whereas what is needed for choosing autos is something that can choose between multiple existing commands (such as Select).
I think that the point here is that Deferred is for creating a command at runtime, whereas what is needed for choosing autos is something that can choose between multiple existing commands (such as Select).
I agree with that, but it seems a bit out of scope for this PR. Maybe split the deprecation into another PR which either is blocked on one to add that sugar or adds it itself?
My question is more three PRs (deprecation blocked on the sugar) or two (deprecation and sugar combined).
Please split this into two PRs. We can't merge deprecations mid-season, because too many teams see them as errors that must be fixed, rather than as warnings.
