widelands-dev team mailing list archive

[Klaus Halfmann <klaus.halfmann@xxxxxxxx>]

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.