[commands] Update requirements consistently

[spacey-sooty]

Resolves #6291

[spacey-sooty]

This doesn't update everywhere because in many places for example ParralelRaceGroup the class uses getRequirements() which returns a Set<Subsystem> which cannot be passed into addRequirements directly. We could loop over them but I'm not sure if that's something we want to do

[Starlight220]

Is it feasible to add an addRequirements(Collection) (or maybe take a Set) overload for those cases?

[spacey-sooty]

Is it feasible to add an addRequirements(Collection) (or maybe take a Set) overload for those cases?

I would say probably as it's already a for loop iterating through items so it'd just be the same pretty much

[KangarooKoala]

Would it be reasonable to change Command.m_requirements to private instead of protected after this? (Potentially in a separate PR)

[spacey-sooty]

If we add an overload this seems like a reasonable change. It'd be a long term fix for the inconsistency as well so might still be fine in this pr?

[spacey-sooty]

/format

[KangarooKoala]

PMD in the Java format CI is actually complaining that m_requirements could be made final. Was there a particular reason to make it non-final?

[spacey-sooty]

PMD in the Java format CI is actually complaining that m_requirements could be made final. Was there a particular reason to make it non-final?

There is no guarantee if addRequirements being called in the constructor only. Making it final would make that guarantee which seems like a reasonable change to me, but technically a breaking change.

[KangarooKoala]

PMD in the Java format CI is actually complaining that m_requirements could be made final. Was there a particular reason to make it non-final?

There is no guarantee if addRequirements being called in the constructor only. Making it final would make that guarantee which seems like a reasonable change to me, but technically a breaking change.

final just indicates that the variable can't be changed to refer to a different object, the object can still be mutated (such as adding new members).

final Set<Integer> x = new HashSet<>();
x.add(0); // This is fine
x = Set.of(1); // This is illegal

[spacey-sooty]

PMD in the Java format CI is actually complaining that m_requirements could be made final. Was there a particular reason to make it non-final?

There is no guarantee if addRequirements being called in the constructor only. Making it final would make that guarantee which seems like a reasonable change to me, but technically a breaking change.

final just indicates that the variable can't be changed to refer to a different object, the object can still be mutated (such as adding new members).

final Set<Integer> x = new HashSet<>();
x.add(0); // This is fine
x = Set.of(1); // This is illegal

Ahhhh my misunderstanding its the equivalent of the const keyword in C++ that makes sense

[spacey-sooty]

Flaky test :cry:

[spacey-sooty]

Not required for python as it never had this problem