Improvements (continuous) #10
Conversation
skiminki
left a comment
skiminki
left a comment
There was a problem hiding this comment.
Not sure this is the cleanest way
| if (!tab.m_tournament | ||
| && !game->pgn()->isNull() | ||
| && ( !game->pgn()->moves().isEmpty() // ignore empty games | ||
| || !game->pgn()->result().isNone())) // without adjudication | ||
| auto pgn = game->pgn(); | ||
|
|
||
| if (!tab.m_tournament && !pgn->isNull() | ||
| && (!pgn->moves().isEmpty() // ignore empty games | ||
| || !pgn->result().isNone())) // without adjudication |
There was a problem hiding this comment.
The indentation changes look unnecessary
| && !game->pgn()->isNull() | ||
| && ( !game->pgn()->moves().isEmpty() // ignore empty games | ||
| || !game->pgn()->result().isNone())) // without adjudication | ||
| auto pgn = game->pgn(); |
There was a problem hiding this comment.
This probably should be a reference (auto const &pgn, for example), although I wouldn't make this change unless necessary
|
|
||
| QString evalString(const MoveEvaluation& eval, const Chess::Move& move); | ||
| QString statusString(const Chess::Move& move, bool doMove); | ||
| QString statusString(const Chess::Move& move, bool doMove); |
There was a problem hiding this comment.
Change looks unnecessary
| m_restart_score(0), | ||
| m_strikes(0), | ||
| m_cuteseal(false) | ||
| m_restart_score(0), |
There was a problem hiding this comment.
Unless there were warnings related to ordering, I wouldn't change anything here. (And if these fix warnings, better to isolate in a separate commit.)
| m_strikes(0), | ||
| m_cuteseal(false) | ||
| m_restart_score(0), | ||
| m_cuteseal(false) |
| int secondScore = pair->secondScore() + Tournament::playerAt(iBlack).builder()->resumescore(); | ||
| int leadScore = qMax(firstScore, secondScore); | ||
| int pointsInProgress = pair->gamesInProgress() * 2; | ||
| // int pointsInProgress = pair->gamesInProgress() * 2; |
| } | ||
|
|
||
| QTextStream& operator<<(QTextStream& out, const PgnGame& game) | ||
| QTextStream& operator<<(QTextStream& out, PgnGame& game) |
There was a problem hiding this comment.
I wouldn't remove the const. It's confusing when operator << modifies its input parameters.
| const EngineConfiguration& config = m_engineManager->engineAt(engineIndex); | ||
|
|
||
| for (auto player : m_players) | ||
| for (auto &player : m_players) |
| file.resize(0); | ||
| } | ||
| // remove the "\n*\n\n" at the end of the file | ||
| else if (m_last_result == "*") { |
There was a problem hiding this comment.
IIRC, the PGN standard says that an unterminated game ends in '*'
| { | ||
| lockCurrentGame(); | ||
| bool ok = m_tabs.at(m_tabBar->currentIndex()).m_pgn->write(fileName); | ||
| auto pgn = m_tabs.at(m_tabBar->currentIndex()).m_pgn; |
2 participants
I haven't tested it.