Unverified Commit 45d7ed4e authored by Yichao Yu's avatar Yichao Yu
Browse files

Fix accidental revealing of items when the game is restarted after lost from a middle click.

Summary:
When a flag is misplaced, a middle click could cause the game to lost.
Currently, if the user choose to restart the game in this case, there is a high chance
that a few pieces around the one middle clicked are revealed right after the game restarted.

The reason is that when the user restart the game in such case,
it happens within `checkLost`/`onItemRevealed` which means that the
game might not be the same one anymore after `onItemRevealed` returns
and the loop handling all neighboring pieces are now operating on the now restarted game.

The fix is to make sure the loop stops when the game is finished. Checking `m_gameOver` is
unreliable and won't work in this case since the restart of the game would have reset it.
Instead, the finished status is returned from all functions that trigger such a condition
and the information is propagated back to the caller to terminate the loop appropriately.
This also improved another unreliable use of `m_gameOver` that is currently harmless.

Debugging was possible thanks to `rr`.

Reviewers: mlaurent, yurchor, #kde_games, aacid

Reviewed By: yurchor, aacid

Subscribers: aacid, kde-games-devel

Differential Revision: https://phabricator.kde.org/D27716
parent 464e12bf
......@@ -289,7 +289,7 @@ void MineFieldItem::adjustItemPositions()
}
}
void MineFieldItem::onItemRevealed(int row, int col)
bool MineFieldItem::onItemRevealed(int row, int col)
{
m_numUnrevealed--;
if(itemAt(row,col)->hasMine())
......@@ -301,9 +301,9 @@ void MineFieldItem::onItemRevealed(int row, int col)
revealEmptySpace(row,col);
}
// now let's check for possible win/loss
checkLost();
if(!m_gameOver) // checkLost might set it
checkWon();
if(checkLost())
return true;
return checkWon();
}
void MineFieldItem::revealEmptySpace(int row, int col)
......@@ -439,8 +439,12 @@ void MineFieldItem::mouseReleaseEvent( QGraphicsSceneMouseEvent * ev)
{
// force=true to omit Pressed check
item->release(true);
if(item->isRevealed())
onItemRevealed(item);
// If revealing the item ends the game, stop the loop,
// since everything that needs to be done for the current game is finished.
// Otherwise, if the user has restarted the game, we'll be revealing
// items for the new game.
if(item->isRevealed() && onItemRevealed(item))
break;
}
}
}
......@@ -554,21 +558,21 @@ void MineFieldItem::revealAllMines()
}
}
void MineFieldItem::onItemRevealed(CellItem* item)
bool MineFieldItem::onItemRevealed(CellItem* item)
{
int idx = m_cells.indexOf(item);
if(idx == -1)
{
qCDebug(KMINES_LOG) << "really strange - item not found";
return;
return false;
}
int row = idx / m_numCols;
int col = idx - row*m_numCols;
onItemRevealed(row,col);
return onItemRevealed(row,col);
}
void MineFieldItem::checkLost()
bool MineFieldItem::checkLost()
{
// for loss...
foreach( CellItem* item, m_cells )
......@@ -577,12 +581,13 @@ void MineFieldItem::checkLost()
{
m_gameOver = true;
emit gameOver(false);
break;
return true;
}
}
return false;
}
void MineFieldItem::checkWon()
bool MineFieldItem::checkWon()
{
// this also takes into account the trivial case when
// only some cells left unflagged and they
......@@ -601,7 +606,9 @@ void MineFieldItem::checkWon()
// now all mines should be flagged, notify about this
emit flaggedMinesCountChanged(m_minesCount);
emit gameOver(true);
return true;
}
return false;
}
QList<FieldPos> MineFieldItem::adjacentRowColsFor(int row, int col)
......
......@@ -129,13 +129,14 @@ private:
*/
QList<FieldPos> adjacentRowColsFor(int row, int col);
/**
* Checks if player lost the game
* Checks if player lost the game. Return `true` if lost.
* A `true` return value and a `false` `m_gameOver` indicates that the game is restarted.
*/
void checkLost();
bool checkLost();
/**
* Checks if player won the game
* Checks if player won the game. Return `true` if won.
*/
void checkWon();
bool checkWon();
/**
* Reveals all unmarked items containing mines
*/
......@@ -158,9 +159,12 @@ private:
*/
void setupBorderItems();
void onItemRevealed(int row, int col);
/**
* Return `true` if the game is finished (and possibly restarted) after the call.
*/
bool onItemRevealed(int row, int col);
// overload
void onItemRevealed(CellItem* item);
bool onItemRevealed(CellItem* item);
// note: in member functions use itemAt (see above )
// instead of hand-computing index from row & col!
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment