[Kde-games-devel] Review Request 116483: Restart button works and uses the current game configuration

[Roney Gomes]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/116483/#review52480
-----------------------------------------------------------


Thank you very much for you current work on KNavalBattle. Besides the small issues I pointed below, everything seems to be OK. Didn't notice anything abnormal while running the game.

Cheers.


src/controller.h
<https://git.reviewboard.kde.org/r/116483/#comment37115>

    Could you please use camel case for m_battle_ships_configuration?



src/controller.h
<https://git.reviewboard.kde.org/r/116483/#comment37116>

    I'm not sure what "restartC" means. "Restart Controller" perhaps? If so, considering we're at the controller class already, why not just call it "restart"?



src/controller.cpp
<https://git.reviewboard.kde.org/r/116483/#comment37117>

    Please, to maintain the current code style, put spaces between binary operators and its operands. I mean "m_ready = 0"



src/networkentity.cpp
<https://git.reviewboard.kde.org/r/116483/#comment37118>

    Again, the spacing problem.



src/networkentity.cpp
<https://git.reviewboard.kde.org/r/116483/#comment37119>

    The same spacing problem once again. ;-)



src/playfield.h
<https://git.reviewboard.kde.org/r/116483/#comment37120>

    What does "restartPF" mean? Restart Playfield I suppose. If so, what's the need of this change? A restart method at the Playfield class is meaningful enough for me -- at least -- to have a glimpse about what it does.


- Roney Gomes


On Feb. 28, 2014, 4:22 p.m., Jaime Torres Amate wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/116483/
> -----------------------------------------------------------
> 
> (Updated Feb. 28, 2014, 4:22 p.m.)
> 
> 
> Review request for KDE Games and Roney Gomes.
> 
> 
> Repository: knavalbattle
> 
> 
> Description
> -------
> 
> The restart button made the network game use the wrong configuration (it created a new controller), now it uses the current game configuration (and controller). It restarted the chat and sent again the nick, now it is not needed and is not done.
> 
> I have introduced, I needed it desperately, the name of the message interchanged as an XML attribute (totally backwards compatible). in protocol.cpp and message.h
> 
> 
> Diffs
> -----
> 
>   src/controller.h eb5f63f 
>   src/controller.cpp 3a26170 
>   src/entity.h c6fcdbc 
>   src/entity.cpp 3104029 
>   src/mainwindow.cpp 7ef4482 
>   src/message.h 58b5beb 
>   src/networkentity.h aa66543 
>   src/networkentity.cpp f19b1d5 
>   src/playerentity.cpp c6b0a1d 
>   src/playfield.h 5766022 
>   src/playfield.cpp 5ebfbc2 
>   src/protocol.cpp 2416164 
>   src/sea.cpp 77a8864 
> 
> Diff: https://git.reviewboard.kde.org/r/116483/diff/
> 
> 
> Testing
> -------
> 
> Lots of network games against myself (and some local games against AI too).
> Place half the ships to see if it was possible to start to shoot too soon, make one of them unable to finish placing the remaining ships several times, then finish it... waiting for the opposite to finish placing ships. Do the same with the other player.. Start shooting (some times finishing the game), and then press restart. 
> 
> 
> Thanks,
> 
> Jaime Torres Amate
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20140310/eabb2b99/attachment.html>