[design-discussion] Command interruptibility/runsWhenDisabled rework

[Starlight220]

Due to this being a relatively small change, I'm opening this issue for design discussion as opposed to a full design doc. I'm also bringing together the discussion of interruptible and runsWhenDisabled because I think the solution / improved implementation would be similar for both properties; the discussion should be split if that proves not to be the case.

Current Situation

Interruptibility is currently implemented as a boolean flag passed at scheduling (or trigger binding, when applicable) and queried when another command (with shared requirements) is scheduled, with the following interpretation: true (default) meaning that the current command is canceled (= end(true) is called), false meaning that the new command is not scheduled (without calling initialize or any other lifecycle method).

runsWhenDisabled is currently a boolean read-only property of the Command class/interface, and queried when the robot is disabled (if false) to cancel the command or abort scheduling it. The problem is that the only way to change this value from the default of false is through overriding the runsWhenDisabled method--requiring the verbosity of a full-fledged or anonymous class, counter-productive to the concise and expressive inline command syntax.

Suggested Solution

The current, scheduler-centric interruptible should be replaced by a read-only property of the Command class/interface, and be type-migrated from an unclear boolean to an InterruptionBehavior enum with descriptively-named constants (CANCEL_SELF, CANCEL_INCOMING--I'm open to naming improvements).

I think that runsWhenDisabled is descriptively-named enough that it can stay a boolean.

Both properties would get decorators to create a wrapping[^1] command with the desired property value. These decorators could be made to return an unmodified this if the property is already set to the given value. These decorators would need to be carefully named to avoid confusion between the getter and decorator. [^1]: An alternative would be for the decorators to mutate this. If we do go the wrapper route, the wrapped command would be added to the grouped list of commands that can't be scheduled.

As for an internal implementation improvement, I recommend adding protected field variables for these properties instead of overriding getters (which would be made final/non-virtual); setting a field is simpler and less verbose than overriding a method. Due to Java interfaces not being able to contain fields, they would be pushed down to CommandBase[^2] which would finally override the getter with a field query. [^2]: Would this not complicate the decorators to set these properties, possibly needing to be pushed down to CommandBase themselves?

Open Questions

[RobinsonZ]

This sounds great to me as a sort-of-random alum, and fixes an issue that's been around for a little while with inline commands (see #2217 from 2020).

That said, it seems to me that the InterruptionBehavior enum could be made unnecessary just by renaming the property to something like allowsInterruption. I'm not sure that the two-value enum is necessary, although I don't have a particularly strong opinion.

[Starlight220]

I want to make sure that it's clear that interruptible (or whatever we call it) is relevant only in the event of a new command being scheduled that shares requirements with the current command; cancel() can always be called and does not check interruptible. I think that this solution self-documents what it does a lot better.