Remove unit from groups too on SetUnitNoSelect.

[saurtron]

Work done

• Remove unit from selection groups when set to "noselect".

Remarks

• Could use a third parameter to allow control from outside but probably not needed. Let me know if you prefer this to be toggleable through a method argument.
• Atm when units are killed, they are automatically removed from groups, but not from selection. It should probably be removed from both or none, this will allow fixing that while still allowing games to override the behavior with one function call, or if they already calling SetUnitNoSelect it'll work transparently when removing the Unit::OnDestroyed::SetGroup(nullptr).

[sprunk]

I'm not sure, there's plenty of use cases where a unit can become unselectable just temporarily in which case you may want to keep its group. For example:

• berserk debuff (think HoMM)
• unit is docked/garrisoned within another (think supcom airplanes within a carrier, or marines in a bunker)
• unit is temporarily hidden from play but comes back, think a variant of RA2 chrono legionnaire that doesn't immediately leave a ghost, or maybe a hero disappears to bring reinforcements and comes back later.

AFAIK skirmish AI can use groups for general internal organisation too, see #1334.

Having a separate Spring.SetUnitNoGroup in addition to unselectable sounds best.

[saurtron]

I'm not sure, there's plenty of use cases where a unit can become unselectable just temporarily in which case you may want to keep its group.

Well, atm it's optional through a parameter, wouldn't that cover the use cases you mention?

Otherwise api users can use Spring.SetUnitGroup(unitID, -1), but thought this change makes it more straightforward to just do it in one call, no biggie otherwise.

Also tbh not sure whether the unit can still be added to groups after noSelect is set, not sure what to do about that but maybe we need a SetUnitNoGroup... but then a new flag would be needed maybe.

[GoogleFrog]

What is optional through a parameter? I don't see one.

Sprung has some good examples of cases where you'd want noselect units to retain their group. Leaving units in groups seems like the non-harmful default behaviour because you're not going to be selecting those units anyway.

Calls should be atomic. Don't combine two things into one call. It makes the code confusing to read and adds unexpected side effects when writing it. If someone sets noselect and comes across a bug caused by groups, then they can remove the unit from the group too. If a game dev finds that they are applying the same operations all the time, then they can make a utility for it in lua.

I think it's pretty clear that this PR doesn't prevent units from being readded to groups. If it did, then that would be an engine bug.

Add Spring.SetUnitNoGroup if you have to, and make sure it both removes units from groups and prevents them being added.

[saurtron]

What is optional through a parameter? I don't see one.

Right, I thought I put a bool at the function to control that but seems like I didn't, sorry about that.

Leaving units in groups seems like the non-harmful default behaviour because you're not going to be selecting those units anyway.

True, but I think the unit would still affect the group centroid.

Add Spring.SetUnitNoGroup if you have to, and make sure it both removes units from groups and prevents them being added.

Ok, here it is: #1853

[saurtron]

Closing this in favour of #1853