Skip to content

Commit

Permalink
Load plain improvements (#3422)
Browse files Browse the repository at this point in the history 
* improve loading from plain text

Fixes the loadFromStream_Plain function that is used to load plain text
decklists.
The rewritten function uses more regexes and is a bit cleaner.
This fixes multiple bugs with loading the various sources of decklists.
Note that the new function still has a few issues that are shared with
the original version like creating duplicate cards.

* clang format comments

apparently clang-format even complains about the spacing in your
comments

* refactor loading_from_clipboard tests

Remove all heap allocation and use references.
Use std::pair and std::string so gtest will show the cardnames in error messages.
(note that using QPair or QString does not work with gtest)
Improve the last two testcases to include weird names; and name and
comments testing.
Remove empty header file.

* fix compatibility with more formats

skip "sideboard" line
include everything in mainboard when there are multiple empty lines
add removal of the mwdeck cardversion selector in round braces
add replacal of lowercase ae combination that should never occur
Set cardname to lowercase as apparently our checks are hardcoded to only
accept lowercase.

* remove bugged test

The current load from plain is simply broken, removed checking the
comments for correct contents.

* rework load_from_clipboard tests again

rework the test to have less code duplication
add more tests and more special cases
note that text is still all lowercase

* improve loading from plain text

Fixes the loadFromStream_Plain function that is used to load plain text
decklists.
The rewritten function uses more regexes and is a bit cleaner.
This fixes multiple bugs with loading the various sources of decklists.
Note that the new function still has a few issues that are shared with
the original version like creating duplicate cards.

* clang format comments

apparently clang-format even complains about the spacing in your
comments

* refactor loading_from_clipboard tests

Remove all heap allocation and use references.
Use std::pair and std::string so gtest will show the cardnames in error messages.
(note that using QPair or QString does not work with gtest)
Improve the last two testcases to include weird names; and name and
comments testing.
Remove empty header file.

* fix compatibility with more formats

skip "sideboard" line
include everything in mainboard when there are multiple empty lines
add removal of the mwdeck cardversion selector in round braces
add replacal of lowercase ae combination that should never occur
Set cardname to lowercase as apparently our checks are hardcoded to only
accept lowercase.

* remove bugged test

The current load from plain is simply broken, removed checking the
comments for correct contents.

* rework load_from_clipboard tests again

rework the test to have less code duplication
add more tests and more special cases
note that text is still all lowercase

* remove forcing of lowercase cardnames

Cardnames in DeckList::loadFromStream_Plain will no longer be forced
lowercase if they aren't found in the database.
Empty lines in the comments of plaintext decklists will not be skipped.
The loading_from_clipboard_test gets its functions declared in a
separate header "clipboard_testing.h".
Add more edgecase tests.
Refactor code.

* add old QHash version support

QT 5.5 does not support using initializer lists for QHash.
Implement a preprocessor version check for conditionally using inserts
instead of a const with initializer list.

* add old QHash version support

QT 5.5 does not support using initializer lists for QHash.
Implement a preprocessor version check for conditionally using [] access
instead of a const with initializer list.

* add qHash on QRegularExpression below QT 5.6

Apparently QRegularExpression can't be hashed in lower QT versions,
so we add our own hash function, and everyone lived happily ever after,
and none the wiser.

* add header guards to clipboard_testing.h
  • Loading branch information
@ebbit1q @ZeldaZach
ebbit1q authored and ZeldaZach committed Nov 7, 2018
1 parent 8b7a287 commit e1e9caf
Show file tree
Hide file tree
Showing 6 changed files with 300 additions and 328 deletions.
239 changes: 109 additions & 130 deletions common/decklist.cpp
View file
Expand Up @@ -2,8 +2,17 @@
#include <QCryptographicHash>
#include <QDebug>
#include <QFile>
#include <QRegularExpression>
#include <QTextStream>

#if QT_VERSION < 0x050600
// qHash on QRegularExpression was added in 5.6, FIX IT
uint qHash(const QRegularExpression &key, uint seed) noexcept
{
return qHash(key.pattern(), seed); // call qHash on pattern QString instead
}
#endif

SideboardPlan::SideboardPlan(const QString &_name, const QList<MoveCard_ToZone> &_moveList)
: name(_name), moveList(_moveList)
{
Expand Down Expand Up @@ -477,161 +486,131 @@ bool DeckList::saveToFile_Native(QIODevice *device)

bool DeckList::loadFromStream_Plain(QTextStream &in)
{
cleanList();
QVector<QString> inputs; // QTextStream -> QVector

bool priorEntryIsBlank = true, isAtBeginning = true;
int blankLines = 0;
while (!in.atEnd()) {
QString line = in.readLine().simplified().toLower();

/*
* Removes all blank lines at start of inputs
* Ex: ("", "", "", "Card1", "Card2") => ("Card1", "Card2")
*
* This will also concise multiple blank lines in a row to just one blank
* Ex: ("Card1", "Card2", "", "", "", "Card3") => ("Card1", "Card2", "", "Card3")
*/
if (line.isEmpty()) {
if (priorEntryIsBlank || isAtBeginning) {
continue;
}

priorEntryIsBlank = true;
blankLines++;
} else {
isAtBeginning = false;
priorEntryIsBlank = false;
}
const QRegularExpression reCardLine("^\\s*[\\w\\[\\(\\{].*$", QRegularExpression::UseUnicodePropertiesOption);
const QRegularExpression reEmpty("^\\s*$");
const QRegularExpression reComment("[\\w\\[\\(\\{].*$", QRegularExpression::UseUnicodePropertiesOption);
const QRegularExpression reSBMark("^\\s*sb:\\s*(.+)", QRegularExpression::CaseInsensitiveOption);
const QRegularExpression reSBComment("sideboard", QRegularExpression::CaseInsensitiveOption);

// simplified matches
const QRegularExpression reMultiplier("^[xX\\(\\[]*(\\d+)[xX\\*\\)\\]]* ?(.+)");
const QRegularExpression reBrace(" ?[\\[\\{][^\\]\\}]*[\\]\\}] ?"); // not nested
const QRegularExpression reRoundBrace("^\\([^\\)]*\\) ?"); // () are only matched at start of string
const QRegularExpression reDigitBrace(" ?\\(\\d*\\) ?"); // () are matched if containing digits
const QHash<QRegularExpression, QString> differences{{QRegularExpression("’"), QString("'")},
{QRegularExpression("Æ"), QString("Ae")},
{QRegularExpression("æ"), QString("ae")},
{QRegularExpression(" ?[|/]+ ?"), QString(" // ")},
{QRegularExpression("(?<![A-Z]) ?& ?"), QString(" // ")}};

inputs.push_back(line);
}
cleanList();

/*
* Removes blank line at end of inputs (if applicable)
* Ex: ("Card1", "Card2", "") => ("Card1", "Card2")
* NOTE: Any duplicates were taken care of above, so there can be
* at most one blank line at the very end
*/
if (!inputs.empty() && inputs.last().isEmpty()) {
blankLines--;
inputs.erase(inputs.end() - 1);
}
QStringList inputs = in.readAll().trimmed().split('\n');
int max_line = inputs.size();

// If "Sideboard" line appears in inputs, then blank lines mean nothing
if (inputs.contains("sideboard")) {
blankLines = 2;
// start at the first empty line before the first cardline
int deckStart = inputs.indexOf(reCardLine);
if (deckStart == -1) { // there are no cards?
if (inputs.indexOf(reComment) == -1)
return false; // input is empty
deckStart = max_line;
} else {
deckStart = inputs.lastIndexOf(reEmpty, deckStart);
if (deckStart == -1) {
deckStart = 0;
}
}

bool inSideboard = false, titleFound = false, isSideboard;
int okRows = 0;

foreach (QString line, inputs) {
// This is a comment line, ignore it
if (line.startsWith("//")) {
if (!titleFound) // Set the title to the first comment
{
name = line.mid(2).trimmed();
titleFound = true;
} else if (okRows == 0) // We haven't processed any cards yet
{
comments += line.mid(2).trimmed() + "\n";
// find sideboard position, if marks are used this won't be needed
int sBStart = -1;
if (inputs.indexOf(reSBMark, deckStart) == -1) {
sBStart = inputs.indexOf(reSBComment, deckStart);
if (sBStart == -1) {
sBStart = inputs.indexOf(reEmpty, deckStart + 1);
if (sBStart == -1) {
sBStart = max_line;
}
int nextCard = inputs.indexOf(reCardLine, sBStart + 1);
if (inputs.indexOf(reEmpty, nextCard + 1) != -1) {
sBStart = max_line; // if there is another empty line all cards are mainboard
}

continue;
}

// If we have a blank line and it's the _ONLY_ blank line in the paste
// and it follows at least one valid card
// Then we assume it means to start the sideboard section of the paste.
// If we have the word "Sideboard" appear on any line, then that will
// also indicate the start of the sideboard.
if ((line.isEmpty() && blankLines == 1 && okRows > 0) || line.startsWith("sideboard")) {
inSideboard = true;
continue; // The line isn't actually a card
}
}

isSideboard = inSideboard;
int index = 0;
QRegularExpressionMatch match;

if (line.startsWith("sb:")) {
line = line.mid(3).trimmed();
isSideboard = true;
// parse name and comments
while (index < deckStart) {
const QString current = inputs.at(index++);
if (!current.contains(reEmpty)) {
match = reComment.match(current);
name = match.captured();
break;
}

if (line.trimmed().isEmpty()) {
continue; // The line was " " instead of "\n"
}
while (index < deckStart) {
const QString current = inputs.at(index++);
if (!current.contains(reEmpty)) {
match = reComment.match(current);
comments += match.captured() + '\n';
}
}
comments.chop(1); // remove last newline

// parse decklist
for (; index < max_line; ++index) {

// Filter out MWS edition symbols and basic land extras
QRegExp rx("\\[.*\\]\\s?");
line.remove(rx);
rx.setPattern("\\s?\\(.*\\)");
line.remove(rx);

// Filter out post card name editions
rx.setPattern("\\|.*$");
line.remove(rx);

// If the user inputs "Quicksilver Elemental" then it will cut it off
// 1x Squishy Treaker
int i = line.indexOf(' ');
int cardNameStart = i + 1;

if (i > 0) {
// If the count ends with an 'x', ignore it. For example,
// "4x Storm Crow" will count 4 correctly.
if (line.at(i - 1) == 'x') {
i--;
} else if (!line.at(i - 1).isDigit()) {
// If the user inputs "Quicksilver Elemental" then it will work as 1x of that card
cardNameStart = 0;
// check if line is a card
match = reCardLine.match(inputs.at(index));
if (!match.hasMatch())
continue;
QString cardName = match.captured().simplified();

// check if card should be sideboard
bool sideboard = false;
if (sBStart < 0) {
match = reSBMark.match(cardName);
if (match.hasMatch()) {
sideboard = true;
cardName = match.captured(1);
}
} else {
if (index == sBStart) // skip sideboard line itself
continue;
sideboard = index > sBStart;
}

bool ok;
int number = line.left(i).toInt(&ok);

if (!ok) {
number = 1; // If input is "cardName" assume it's "1x cardName"
// check if a specific amount is mentioned
int amount = 1;
match = reMultiplier.match(cardName);
if (match.hasMatch()) {
amount = match.capturedRef(1).toInt();
cardName = match.captured(2);
}

QString cardName = line.mid(cardNameStart);

// Common differences between Cockatrice's card names
// and what's commonly used in decklists
rx.setPattern("’");
cardName.replace(rx, "'");
rx.setPattern("Æ");
cardName.replace(rx, "Ae");
rx.setPattern("\\s*[|/]{1,2}\\s*");
cardName.replace(rx, " // ");

// Replace only if the ampersand is preceded by a non-capital letter,
// as would happen with acronyms. So 'Fire & Ice' is replaced but not
// 'R&D' or 'R & D'.
// Qt regexes don't support lookbehind so we capture and replace instead.
rx.setPattern("([^A-Z])\\s*&\\s*");
if (rx.indexIn(cardName) != -1) {
cardName.replace(rx, QString("%1 // ").arg(rx.cap(1)));
// remove stuff inbetween braces
cardName.remove(reBrace);
cardName.remove(reRoundBrace); // I'll be entirely honest here, these are split to accommodate just three cards
cardName.remove(reDigitBrace); // all cards from un-sets that have a word in between round braces at the end

// replace common differences in cardnames
for (auto diff = differences.constBegin(); diff != differences.constEnd(); ++diff) {
cardName.replace(diff.key(), diff.value());
}

// We need to get the name of the card from the database,
// but we can't do that until we get the "real" name
// (name stored in database for the card)
// and establish a card info that is of the card, then it's
// a simple getting the _real_ name of the card
// (i.e. "STOrm, CrOW" => "Storm Crow")
// get cardname, this function does nothing if the name is not found
cardName = getCompleteCardName(cardName);

// Look for the correct card zone of where to place the new card
QString zoneName = getCardZoneFromName(cardName, isSideboard ? DECK_ZONE_SIDE : DECK_ZONE_MAIN);
// get zone name based on if it's in sideboard
QString zoneName = getCardZoneFromName(cardName, sideboard ? DECK_ZONE_SIDE : DECK_ZONE_MAIN);

okRows++;
new DecklistCardNode(cardName, number, getZoneObjFromName(zoneName));
// make new entry in decklist
new DecklistCardNode(cardName, amount, getZoneObjFromName(zoneName));
}

updateDeckHash();
return (okRows > 0);
return true;
}

InnerDecklistNode *DeckList::getZoneObjFromName(const QString zoneName)
Expand Down
3 changes: 2 additions & 1 deletion tests/loading_from_clipboard/CMakeLists.txt
View file
@@ -1,6 +1,7 @@
ADD_DEFINITIONS("-DCARDDB_DATADIR=\"${CMAKE_CURRENT_SOURCE_DIR}/data/\"")
add_executable(loading_from_clipboard_test
loading_from_clipboard_test.cpp
clipboard_testing.cpp
../../common/decklist.cpp
)

Expand All @@ -12,4 +13,4 @@ find_package(Qt5 COMPONENTS Concurrent Network Widgets REQUIRED)
set(TEST_QT_MODULES Qt5::Concurrent Qt5::Network Qt5::Widgets)

target_link_libraries(loading_from_clipboard_test cockatrice_common ${GTEST_BOTH_LIBRARIES} ${TEST_QT_MODULES})
add_test(NAME loading_from_clipboard_test COMMAND loading_from_clipboard_test)
add_test(NAME loading_from_clipboard_test COMMAND loading_from_clipboard_test)
40 changes: 40 additions & 0 deletions tests/loading_from_clipboard/clipboard_testing.cpp
View file
@@ -0,0 +1,40 @@
#include "clipboard_testing.h"
#include <QTextStream>

void Result::operator()(const InnerDecklistNode *innerDecklistNode, const DecklistCardNode *card)
{
if (innerDecklistNode->getName() == DECK_ZONE_MAIN) {
mainboard.append({card->getName().toStdString(), card->getNumber()});
} else if (innerDecklistNode->getName() == DECK_ZONE_SIDE) {
sideboard.append({card->getName().toStdString(), card->getNumber()});
} else {
FAIL();
}
}

void testEmpty(const QString &clipboard)
{
QString cp(clipboard);
DeckList deckList;
QTextStream stream(&cp); // text stream requires local copy
deckList.loadFromStream_Plain(stream);

ASSERT_TRUE(deckList.getCardList().isEmpty());
}

void testDeck(const QString &clipboard, const Result &result)
{
QString cp(clipboard);
DeckList deckList;
QTextStream stream(&cp); // text stream requires local copy
deckList.loadFromStream_Plain(stream);

ASSERT_EQ(result.name, deckList.getName().toStdString());
ASSERT_EQ(result.comments, deckList.getComments().toStdString());

Result decklistBuilder;
deckList.forEachCard(decklistBuilder);

ASSERT_EQ(result.mainboard, decklistBuilder.mainboard);
ASSERT_EQ(result.sideboard, decklistBuilder.sideboard);
}
32 changes: 32 additions & 0 deletions tests/loading_from_clipboard/clipboard_testing.h
View file
@@ -0,0 +1,32 @@
#ifndef CLIPBOARD_TESTING_H
#define CLIPBOARD_TESTING_H

#include "../../common/decklist.h"
#include "gtest/gtest.h"

struct Result
{
// using std types because qt types aren't understood by gtest (without this you'll get less nice errors)
using CardRows = QVector<std::pair<std::string, int>>;
std::string name;
std::string comments;
CardRows mainboard;
CardRows sideboard;

Result()
{
}

Result(std::string _name, std::string _comments, CardRows _mainboard, CardRows _sideboard)
: name(_name), comments(_comments), mainboard(_mainboard), sideboard(_sideboard)
{
}

void operator()(const InnerDecklistNode *innerDecklistNode, const DecklistCardNode *card);
};

void testEmpty(const QString &clipboard);

void testDeck(const QString &clipboard, const Result &result);

#endif // CLIPBOARD_TESTING_H

0 comments on commit e1e9caf

Please sign in to comment.