Hello Gun: * Adressed some comments * Get the tribe and check whether it exists before you push it -> I have no idea (yet) what this code does :-) * Playercommand is not for current player , not sure if this can normally happen lets wait If I ever see this, then we can make it an assert. * The This / this confusion is because I moved some fucntion into Impl, but they need the main class. I could use "outer" or some other name to avoid it. * send_player_command(Widelands::PlayerCommand* pc) making this a unique_ptr: * Will affect a lot of code * Some aspects of this code are not clear to me (e.g. Commnands a queued for networking) -> lets adress this in a seperate branch, I already regret I touched it :-) * I would leave "GameClientImpl* d" as is: * it is used very often * it is used in the local scope only -> no risc to loose control * commonly used Pattern in Widelands * d->settings.usernum == -2,-1, n seems to be used to define the "hello" handshake what about "kPreHello" and "kAfterHello" constants? Maybe we should move the two remaining variables to GameClientImpl, too?
Please check the logic of the main switch statement, I changed some control flow for better readbility. I intend to do a similar refactoring with gamehost, too. -- https://code.launchpad.net/~widelands-dev/widelands/refactor_gameclient/+merge/366743 Your team Widelands Developers is requested to review the proposed merge of lp:~widelands-dev/widelands/refactor_gameclient into lp:widelands. _______________________________________________ Mailing list: https://launchpad.net/~widelands-dev Post to : widelands-dev@lists.launchpad.net Unsubscribe : https://launchpad.net/~widelands-dev More help : https://help.launchpad.net/ListHelp