[Kde-games-devel] Review Request 127489: KBlocks: Stop playing sounds on pause

[Julian Helfferich]

> On March 25, 2016, 9:46 a.m., Frederik Schwarzer wrote:
> > There should be a pause state already, no? Why not just use that in the sound playing method?
> > 
> > I did not look at the sources carefully, but grepping it for "pause" gives quite some results. Does KBlocks already track the pause state in other places? Do you now add another instance of tracking?
> > 
> > Other than that sublte feeling or irritation, the patch looks fine to me. :)

There is indeed already KBlocks_Game_State enum, but it is unused because no class actually keeps track of the current state. However, this made me realize that there is much simpler solution to the issue: KBlocks does indeed keep track whether the game is paused in the KeyboardPlayer class, ignoring all keyboard input when the game is paused. However, nobody told the KeyboardPlayer when the game was paused. Fixing this is trivial and the ignored keyboard input also means no sounds played.

Thank you for having me take a closer look :)


- Julian


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


On March 25, 2016, 11:35 a.m., Julian Helfferich wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127489/
> -----------------------------------------------------------
> 
> (Updated March 25, 2016, 11:35 a.m.)
> 
> 
> Review request for KDE Games.
> 
> 
> Repository: kblocks
> 
> 
> Description
> -------
> 
> Testing the last patch to activate sounds in KBlocks, I realized another small bug: The sounds tied to keystrokes continue playing when the game is paused.
> 
> To fix this issue, KBlocksScene now keeps track of the state of the game. To this end, the bool mGameStarted is replaced with an enum class GameState taking the values Stopped, Running or Paused. When the state changes, the signal gameStateChanged() is emitted. This signal is connected to the slot updateSoundsEnabled() which sets the sound to enabled if GameState is Running and sounds are enabled in the system settings.
> 
> In summary, I think the approach is more elaborate than what would be necessary, but more preferable to change the sound separately in startGame(), stopGame() and pauseGame().
> 
> 
> Diffs
> -----
> 
>   KBlocksWin.cpp 522bfdd 
> 
> Diff: https://git.reviewboard.kde.org/r/127489/diff/
> 
> 
> Testing
> -------
> 
> Played the game.
> 
> * Enabled and disabled the sound while game is running.
> * Enabled and disabled the sound when game is paused.
> 
> 
> Thanks,
> 
> Julian Helfferich
> 
>

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