[Kde-games-devel] Review Request 113113: add local spectator player

[Nemanja Hirsl]

> On Oct. 16, 2013, 9:23 p.m., Albert Astals Cid wrote:
> > How hard would be to check on start that not all players are spectators and complain if they are all?
> 
> Nemanja Hirsl wrote:
>     Newgamedialog already have some code for enabling/disabling Ok/Cancel buttons. Maybe this code should be improved and not allow game with all spectators?
> 
> Albert Astals Cid wrote:
>     Makes sense to me. Alexander?
> 
> Alexander Schuch wrote:
>     I implemented this check. If you know how to do it better using signals, please update. :)
>     
>     During tests I noticed more problems, but I am not sure where they belong to - to the bug tracker or as @todo in the code?
>     
>     For example, set the neutral player production to any high value (anything above 10) and disable cumulative production. No player will be able to conquer any planet and the game will take forever. If playing without any human player (AI match), the game will "freeze". Or play a game with zero neutral planets - same issue. Or set the starting planets of each player to "vacant".
> 
> Albert Astals Cid wrote:
>     What do you mean that the game will freeze if there's no human player? That works just fine here (in version from KDE 4.11.2)

Alexander, issues which are reproducible on 4.11 should be reported as bugs, others, if there are any, are regressions. For now, after testing and reviewing code, I believe (and hope) we didn't create any. Some of issues you mentioned I didn't succeed to reproduce (issue related to high neutral player production with disabled cumulative production) by using code from master and this patch. However, on both 4.11.2 and master there is a "freeze" - actually infinite loop in case of AI match without neutral planets. So, there are obviously some problems and it would be good if we could filter them out. If you have time, please report bugs for appropriate versions.

Just small notes for this code and current behavior:
1. Setup where one of two players is spectator is allowed and OK button can be pressed. The outcome is obvious - the winner is non-spectator.
2. If player is spectator should we allow kill percentage and production to be set up?

Everything else looks good.


- Nemanja


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/113113/#review41842
-----------------------------------------------------------


On Oct. 19, 2013, 11:33 p.m., Alexander Schuch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113113/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2013, 11:33 p.m.)
> 
> 
> Review request for KDE Games.
> 
> 
> Repository: konquest
> 
> 
> Description
> -------
> 
> For AI development it is useful to watch the AIs play. This patch adds a new local human spectator player which does not have any planets and does not count as active player. The game is not allowed to start when all player controllers are spectators.
> 
> 
> Diffs
> -----
> 
>   CMakeLists.txt b67b4ba 
>   dialogs/newgamedlg.h 70e8a1a 
>   dialogs/newgamedlg.cc 6f376ed 
>   game.cpp 5746b22 
>   gameview.cc ebba726 
>   players/computerplayer.h 29b6b77 
>   players/computerplayer.cpp f4fd7a2 
>   players/localplayer.h a12b46f 
>   players/localplayer.cpp dbb47f3 
>   players/localplayer_gui.cpp 44a04b5 
>   players/player.h a6d91c4 
>   players/player.cpp 7135127 
>   players/spectatorplayer.h PRE-CREATION 
>   players/spectatorplayer.cpp PRE-CREATION 
>   players/spectatorplayer_gui.h PRE-CREATION 
>   players/spectatorplayer_gui.cpp PRE-CREATION 
> 
> Diff: http://git.reviewboard.kde.org/r/113113/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Alexander Schuch
> 
>

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