D20802: Add a layer for showing the removed tiles of the game.

Albert Astals Cid noreply at phabricator.kde.org
Thu Apr 25 22:41:26 BST 2019


aacid added a comment.


  Is there any chance you could use arc in the future to upload the patches? Makes for easier reviewing since instead of the "Context not available" you get the option to extend the code to read up/down
  
  When starting a new game sometimes it goes wonky
  Before pressing new game https://i.imgur.com/AWZAb7o.jpg
  After pressing new game https://i.imgur.com/m9Talax.jpg

INLINE COMMENTS

> gamebackground.cpp:54
> +{
> +    //painter->setBackground(*m_background);
> +    painter->fillRect(QRectF(0, 0, m_width, m_height), *m_background);

remove commented line?

> gamebackground.h:48
> +     */
> +    void setBackground(QBrush &background);
> +

const QBrush

> gamebackground.h:80
> +private:
> +    QBrush *m_background;
> +

Any reason this is a pointer and not just a QBrush?

> gamescene.cpp:85
> +    // If a removedtiles object already exist, delete it from scene
> +    if (nullptr != m_gameRemovedTiles) {
> +        QGraphicsScene::removeItem(m_gameRemovedTiles);

don't do yoda comparisons (unless it's the style in the file, which i think it's not)

REPOSITORY
  R403 KMahjongg

REVISION DETAIL
  https://phabricator.kde.org/D20802

To: krippendorf, #kde_games
Cc: aacid, kde-games-devel, krippendorf
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-games-devel/attachments/20190425/4baf6a6c/attachment-0001.html>


More information about the kde-games-devel mailing list