[PATCH] Fix unit selection cheat

[historic_bruno]

As reported on the forum, it's possible to add enemy units to a selection of your own units, and thus control them (make them build things for you, attack themselves, etc.)

This patch adds two checks to prevent this: one is in the GUI so it's not actually possible to add enemy units to your selection, but this is not foolproof. The other check is in the simulation code (Commands.js), which filters out the list of selected entities according to the player's ownership. The patch also tests the state of the "Control all units" dev command, so it's still possible to use that for testing.

[Philip Taylor]

• "TODO: Should we handle "control all units" here as well?" - I think we should for bandboxing, since I find it inconvenient having to click them all individually. Double-clicking a unit would ideally select units of the same type belonging to that unit's player, I think, since that seems to make most sense to me. I wouldn't mind if this was all left as a TODO until later, though.
• g_ControlAllUnits affects the simulation so it needs to be serialised/deserialised, and global variables aren't. I can't think of anywhere entirely great to store it, but maybe adding it as a property of Player components would be okay.
• Commands.js: Instead of repeating if (g_ControlAllUnits[player] || IsOwnedByPlayer(cmd.target, player)) several times, maybe move that condition into a separate function (CanControlUnit or something) to make things more explicit.
• Maybe repair should allow repairing allies' buildings?
• returnresource should check ownership of the dropsite.
• defeat-player (used when a player chooses to quit via the GUI) should check the player so you can only quit yourself. (Or maybe don't pass playerId at all and always use the player argument.)
• promote is already a cheat, it doesn't need to check that the player owns the units (and the check makes it harder to test promotion).
• FilterEntityList could use return entities.filter(...); instead of looping, though either way seems fine.
• The script indentation in session.xml is weird.

[Erik Johansson]

Imho it should be possible to repair your allies buildings, what are friends for if you can't help each other. Not sure if it's important enough to do a forum topic about, but if you're uncertain you can create one to get the rest of the team's opinion :)

[historic_bruno]

Thanks for the feedback, Philip :) That's why I posted a patch, it's important to get this right. Moving g_ControlAllUnits to the player component makes sense to me and I've added some helper functions for diplomacy.

I do think allies should be able to repair each other's buildings, it's not hard to add.

[ben]

(In [9726]) Adds ownership checks to UI selections. Adds ownership and diplomacy checks to Commands.js (fixes #880). Adds control all units setting to Player component (network synced). Adds helpers for diplomacy checks - use these instead of directly accessing diplomacy array. Fixes tests according to these changes.