From 8f784243a134b1ec9319bc78d7f5f45e6ed14fb9 Mon Sep 17 00:00:00 2001 From: dgaston Date: Thu, 2 May 2024 22:09:45 -0400 Subject: [PATCH] Chess: Improve PGN importing This commit replaces the `import_pgn` implementation with a more resiliant parser to handle various edge cases and give helpful error messages instead of crashing. --- Userland/Games/Chess/ChessWidget.cpp | 269 +++++++++++++++++++-------- Userland/Games/Chess/ChessWidget.h | 25 ++- Userland/Games/Chess/main.cpp | 10 +- 3 files changed, 221 insertions(+), 83 deletions(-) diff --git a/Userland/Games/Chess/ChessWidget.cpp b/Userland/Games/Chess/ChessWidget.cpp index de8db6ba798..6e032c8988b 100644 --- a/Userland/Games/Chess/ChessWidget.cpp +++ b/Userland/Games/Chess/ChessWidget.cpp @@ -2,12 +2,14 @@ * Copyright (c) 2020-2022, the SerenityOS developers. * Copyright (c) 2023, Tim Ledbetter * Copyright (c) 2023, Sam Atkins + * Copyright (c) 2024, Daniel Gaston * * SPDX-License-Identifier: BSD-2-Clause */ #include "ChessWidget.h" #include "PromotionDialog.h" +#include #include #include #include @@ -536,91 +538,200 @@ ErrorOr ChessWidget::get_fen() const return TRY(m_playback ? m_board_playback.to_fen() : m_board.to_fen()); } -ErrorOr ChessWidget::import_pgn(Core::File& file) +ErrorOr ChessWidget::import_pgn(Core::File& file) { - m_board = Chess::Board(); + reset(); + enum class TokenType { + Move, + TagSymbol, + TagString, + Comment, + GameTerminator, + RAVStart, + RAVEnd, + Nag + }; - ByteBuffer bytes = TRY(file.read_until_eof()); - StringView content = bytes; - auto lines = content.lines(); - StringView line; - size_t i = 0; + struct Token { + TokenType type; + StringView value; + }; + auto maybe_file = file.read_until_eof(); + if (maybe_file.is_error()) { + return PGNParseError::from_string_formatted(String::formatted("Could not read file")); + } + ByteString bytes = ByteString { maybe_file.release_value().bytes() }; + Vector tokens; + Vector rav_stack; + // FIXME: Engine cannot parse suffixes ? and !. + StringView suffix_characters = "+#"sv; + StringView closing_characters = "]})"sv; + StringView opening_characters = "({["sv; + LineTrackingLexer lexer { StringView { bytes } }; - // Tag Pair Section - // FIXME: Parse these tags when they become relevant - do { - line = lines.at(i++); - } while (!line.is_empty() || i >= lines.size()); + while (!lexer.is_eof()) { - // Movetext Section - bool skip = false; - bool recursive_annotation = false; - bool future_expansion = false; + if (lexer.next_is(is_any_of(closing_characters))) { + return PGNParseError::from_string_formatted(String::formatted("Unexpected character: {}.\n(line {} column {})", lexer.consume(1), lexer.current_position().line + 1, lexer.current_position().column)); + } + if (lexer.next_is('[')) { + lexer.consume_specific('['); + auto value = lexer.consume_until(" "); + tokens.append(Token { TokenType::TagSymbol, value }); + lexer.ignore_while(is_ascii_space); + if (!lexer.consume_specific('"')) { + return PGNParseError::from_string_formatted(String::formatted("Expected opening \".\n(line {} column {})", lexer.current_position().line + 1, lexer.current_position().column)); + } + // Parsing will only succeed if a " is reached, if the lexer goes to a ] the next + // consume_specific will fail. + value = lexer.consume_until(is_any_of("\"]\n"sv)); + tokens.append(Token { TokenType::TagString, value }); + if (!lexer.consume_specific('"')) { + return PGNParseError::from_string_formatted(String::formatted("Expected closing \".\n(line {} column {})", lexer.current_position().line + 1, lexer.current_position().column)); + } + // The end quote must be followed by a closing bracket. + if (!lexer.consume_specific(']')) { + return PGNParseError::from_string_formatted(String::formatted("Expected closing bracket.\n(line {} column {})", lexer.current_position().line + 1, lexer.current_position().column)); + } + // Deal with trailing white space + lexer.ignore_while(is_ascii_space); + continue; + } + + if (lexer.next_is('{')) { + lexer.consume_specific('{'); + // Deal with leading white space + lexer.ignore_while(is_ascii_space); + auto value = lexer.consume_until('}'); + tokens.append(Token { TokenType::Comment, value }); + if (!lexer.consume_specific('}')) { + return PGNParseError::from_string_formatted(String::formatted("Expected closing brace.\n(line {} column {})", lexer.current_position().line + 1, lexer.current_position().column)); + } + // Deal with trailing white space + lexer.ignore_while(is_ascii_space); + continue; + } + + if (lexer.next_is('(')) { + // FIXME: Actually implement RAV instead of just ignoring them. + rav_stack.append(lexer.consume(1)); + while (!rav_stack.is_empty() && !lexer.is_eof()) { + lexer.ignore_until(is_any_of("()"sv)); + if (lexer.next_is('(')) { + rav_stack.append(lexer.consume(1)); + tokens.append(Token { TokenType::RAVStart, rav_stack.last() }); + } else { + rav_stack.take_last(); + tokens.append(Token { TokenType::RAVStart, lexer.consume(1) }); + } + } + if (!rav_stack.is_empty() || lexer.is_eof()) { + return PGNParseError::from_string_formatted(String::formatted("Unclosed recursive annotation.\n(line {} column {})", lexer.current_position().line + 1, lexer.current_position().column)); + } + continue; + } + + if (lexer.next_is("1-0"sv)) { + auto value = lexer.consume(3); + tokens.append(Token { TokenType::GameTerminator, value }); + break; + } + if (lexer.next_is("0-1"sv)) { + auto value = lexer.consume(3); + tokens.append(Token { TokenType::GameTerminator, value }); + break; + } + + if (lexer.next_is("1/2-1/2"sv)) { + auto value = lexer.consume(3); + tokens.append(Token { TokenType::GameTerminator, value }); + break; + } + + if (lexer.next_is("*"sv)) { + auto value = lexer.consume(1); + tokens.append(Token { TokenType::GameTerminator, value }); + break; + } + + if (lexer.next_is(is_ascii_alpha)) { + // Parse move. + auto value = lexer.consume_while([&](char c) { return is_ascii_alphanumeric(c) || suffix_characters.contains(c) || c == '=' || c == '-'; }); + tokens.append(Token { TokenType::Move, value }); + + if (!lexer.next_is(is_any_of(" ({$"sv)) && !lexer.next_is(is_ascii_space)) { + return PGNParseError::from_string_formatted(String::formatted("Unexpected character {}.\n(line {} column {})", lexer.consume(1), lexer.current_position().line + 1, lexer.current_position().column)); + } + + // Deal with any extra trailing white space + lexer.ignore_while(is_ascii_space); + continue; + } + + if (lexer.next_is("$"sv)) { + lexer.consume_specific('$'); + if (!lexer.next_is(is_ascii_digit)) { + return PGNParseError::from_string_formatted(String::formatted("Unexpected character {}.\n(line {} column {})", lexer.consume(1), lexer.current_position().line + 1, lexer.current_position().column)); + } + + auto value = lexer.consume_while(is_ascii_digit); + tokens.append(Token { TokenType::Nag, value }); + + // Ensure that a number has been parsed and it's in range 0-255. + auto optional_number = value.to_number(); + if (!optional_number.has_value()) { + return PGNParseError::from_string_formatted(String::formatted("Could not parse Nag.\n(line {} column {})", lexer.current_position().line + 1, lexer.current_position().column)); + } + + auto number = optional_number.value(); + if (number < 0 || number > 255) { + return PGNParseError::from_string_formatted(String::formatted("Nag must be number between 0-255 but got {}.\n(line {} column {})", number, lexer.current_position().line + 1, lexer.current_position().column)); + } + + // Ensure that the Nag is followed by a whitespace. + if (!lexer.consume_specific(' ')) { + return PGNParseError::from_string_formatted(String::formatted("Unexpected character {}.\n(line {} column {})", lexer.consume(1), lexer.current_position().line + 1, lexer.current_position().column)); + } + + // Deal with trailing white space + lexer.ignore_while(is_ascii_space); + continue; + } + + // Advance to the start of a token or to an invalid closing character to be dealt with. + lexer.ignore_until([&](char c) { return is_ascii_alpha(c) || opening_characters.contains(c) || closing_characters.contains(c) || c == '$'; }); + } + + // FIXME: Display more of this information in the UI. Chess::Color turn = Chess::Color::White; - String movetext; - - for (size_t j = i; j < lines.size(); j++) - movetext = TRY(String::formatted("{}{}", movetext, lines.at(i))); - - for (auto token : TRY(movetext.split(' '))) { - token = TRY(token.trim_ascii_whitespace()); - - // FIXME: Parse all of these tokens when we start caring about them - if (token.ends_with('}')) { - skip = false; - continue; - } - if (skip) - continue; - if (token.starts_with('{')) { - if (token.ends_with('}')) - continue; - skip = true; - continue; - } - if (token.ends_with(')')) { - recursive_annotation = false; - continue; - } - if (recursive_annotation) - continue; - if (token.starts_with('(')) { - if (token.ends_with(')')) - continue; - recursive_annotation = true; - continue; - } - if (token.ends_with('>')) { - future_expansion = false; - continue; - } - if (future_expansion) - continue; - if (token.starts_with('<')) { - if (token.ends_with('>')) - continue; - future_expansion = true; - continue; - } - if (token.starts_with('$')) - continue; - if (token.contains('*')) - break; - // FIXME: When we become able to set more of the game state, fix these end results - if (token.contains("1-0"sv)) { - m_board.set_resigned(Chess::Color::Black); - break; - } - if (token.contains("0-1"sv)) { - m_board.set_resigned(Chess::Color::White); - break; - } - if (token.contains("1/2-1/2"sv)) { - break; - } - if (!token.ends_with('.')) { - m_board.apply_move(Chess::Move::from_algebraic(token, turn, m_board)); + for (auto& token : tokens) { + switch (token.type) { + case TokenType::Move: + // FIXME: Add some move validation so the engine doesn't crash. + // FIXME: Engine crashes with pawn promotion notation (e.g. fxg8=Q). + m_board.apply_move(Chess::Move::from_algebraic(token.value, turn, m_board)); turn = Chess::opposing_color(turn); + break; + case TokenType::TagSymbol: + break; + case TokenType::TagString: + break; + case TokenType::GameTerminator: + if (token.value == "1-0"sv) { + m_board.set_resigned(Chess::Color::Black); + } + if (token.value == "0-1"sv) { + m_board.set_resigned(Chess::Color::White); + } + break; + case TokenType::Comment: + break; + case TokenType::RAVStart: + break; + case TokenType::RAVEnd: + break; + case TokenType::Nag: + break; } } diff --git a/Userland/Games/Chess/ChessWidget.h b/Userland/Games/Chess/ChessWidget.h index 6e803e707a6..8577b062bba 100644 --- a/Userland/Games/Chess/ChessWidget.h +++ b/Userland/Games/Chess/ChessWidget.h @@ -2,6 +2,7 @@ * Copyright (c) 2020-2022, the SerenityOS developers. * Copyright (c) 2023, Sam Atkins * Copyright (c) 2023, Tim Ledbetter + * Copyright (c) 2024, Daniel Gaston * * SPDX-License-Identifier: BSD-2-Clause */ @@ -17,6 +18,28 @@ #include #include +class PGNParseError { +public: + PGNParseError() = default; + PGNParseError(String&& message) + : m_message(move(message)) + { + } + + String const& message() const { return m_message; } + + static PGNParseError from_string_formatted(ErrorOr maybe_formatted_string) + { + if (maybe_formatted_string.is_error()) { + return PGNParseError {}; + } + return PGNParseError { maybe_formatted_string.release_value() }; + } + +private: + String m_message; +}; + class ChessWidget final : public GUI::Frame , public Config::Listener { @@ -54,7 +77,7 @@ public: void set_show_available_moves(bool e) { m_show_available_moves = e; } ErrorOr get_fen() const; - ErrorOr import_pgn(Core::File&); + ErrorOr import_pgn(Core::File&); ErrorOr export_pgn(Core::File&) const; int resign(); diff --git a/Userland/Games/Chess/main.cpp b/Userland/Games/Chess/main.cpp index b3a420e3e48..c97f8fde6ac 100644 --- a/Userland/Games/Chess/main.cpp +++ b/Userland/Games/Chess/main.cpp @@ -2,6 +2,7 @@ * Copyright (c) 2020, the SerenityOS developers. * Copyright (c) 2023, Sam Atkins * Copyright (c) 2023, Tim Ledbetter + * Copyright (c) 2024, Daniel Gaston * * SPDX-License-Identifier: BSD-2-Clause */ @@ -111,10 +112,13 @@ ErrorOr serenity_main(Main::Arguments arguments) if (result.is_error()) return; - if (auto maybe_error = widget->import_pgn(*result.value().release_stream()); maybe_error.is_error()) - dbgln("Failed to import PGN: {}", maybe_error.release_error()); - else + if (auto maybe_error = widget->import_pgn(*result.value().release_stream()); maybe_error.is_error()) { + auto error_message = maybe_error.release_error().message(); + dbgln("Failed to import PGN: {}", error_message); + GUI::MessageBox::show(window, error_message, "Import Error"sv, GUI::MessageBox::Type::Information); + } else { dbgln("Imported PGN file from {}", result.value().filename()); + } })); game_menu->add_action(GUI::Action::create("&Export PGN...", { Mod_Ctrl, Key_S }, [&](auto&) { auto result = FileSystemAccessClient::Client::the().save_file(window, "Untitled", "pgn");