Unit group creation moved from logic to client which can introduce mismatches

[Caball009]

PR #1471 moved the group creation from logic to client side, but this can introduce mismatches under very precise conditions.

Code

{
        TheMessageStream->appendMessage(GameMessage::MSG_DESTROY_SELECTED_GROUP);
}
{
        TheMessageStream->appendMessage(GameMessage::MSG_SELECT_TEAM1);
}
{
        static Coord3D coord = { 0.0f, 0.0f, 0.0f };
        GameMessage* msg = TheMessageStream->appendMessage(GameMessage::MSG_DO_MOVETO);
        msg->appendLocationArgument(coord);
}
{
        // because of #1471 this happens locally before all other logic messages!!!
        GameMessage* msg = TheMessageStream->appendMessage(GameMessage::MSG_CREATE_TEAM1);

        for (Object* obj = TheGameLogic->getFirstObject(); obj; obj = obj->getNextObject())
        {
                if (obj->isKindOf(KINDOF_DOZER) && obj->isLocallyControlled()) // requires at least one dozer
                {
                    msg->appendObjectIDArgument(obj->getID());
                    break;
                }
        }
}

Executing the code above will result in a mismatch, as will following the reproduction below (requires pause):

01. start new game
02. build another dozer
03. pause game (with SHIFT + P)
04. select dozer 1
05. press SHIFT + 1
06. issue move order
07. deselect
08. select dozer 2
09. press CTRL + 1
10. deselect
---
11. unpause
12. exit game, save replay, watch it and notice the mismatch

During live gameplay this will move both dozers to their destination, but in the replay only the first one will move. The mismatch occurs because the group creation takes place locally before all logic is executed. So, locally the group creation has already happened, but not for other players or the replay observer. If a move order is given to a group that exists locally, but not on other clients, the mismatch occurs.

This doesn't seem to be a huge concern for multiplayer because so far pause seems to be required to achieve this.

[Stubbjax]

This doesn't seem to be a huge concern for multiplayer because so far pause seems to be required to achieve this.

I'd even be inclined to simply use the old logic for single-player games as the latency issue that the original change was addressing does not apply there anyway.