widelands-dev team mailing list archive

[GunChleoc <fios@xxxxxxxxxxxxxxxxxxx>]

* Get the tribe and check whether it exists before you push it
  -> I have no idea (yet) what this code does :-)

It's for the tips in the game loading screen. We have tribe-specific lists of tips.

There is a function  bool Widelands::tribe_exists(const std::string& tribename) that you can use.

* 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.

I see now - it's just an ugly variable name, yes, please change the name to "game_client" or "parent" or some such.


* send_player_command(Widelands::PlayerCommand* pc) making this a unique_ptr:
  -> lets adress this in a seperate branch, I already regret I touched it :-)

LOL fair enough. Cans or worms are best dealt with separately.


* 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

That's because it was written before we switched to C++11, so there was no unique_ptr available. You don't have to do it in this branch though.

* d->settings.usernum == -2,-1, n seems to be used to define the "hello" handshake
  what about "kPreHello" and "kAfterHello" constants?

I have found this comment:
  /**
   * Checks if client \ref name exists and \returns int32_t :
   *   -   the client number if found
   *   -   -1 if no client was found
   *   -   -2 if the host is the client (has no client number)
   */

So, kUnknownClient and kUnknownHost?

* Maybe we should move the two remaining variables to GameClientImpl, too?

Consistency is good :)
-- 
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.