From 29c41e953b010c8c31843164afa5245882baaca5 Mon Sep 17 00:00:00 2001 From: Sam Atkins Date: Fri, 21 Apr 2023 15:17:33 +0100 Subject: [PATCH] LibChess: Convert Commands to use String and propagate errors --- Userland/Libraries/LibChess/UCICommand.cpp | 138 +++++++++--------- Userland/Libraries/LibChess/UCICommand.h | 52 +++---- Userland/Libraries/LibChess/UCIEndpoint.cpp | 5 +- Userland/Services/ChessEngine/ChessEngine.cpp | 4 +- 4 files changed, 101 insertions(+), 98 deletions(-) diff --git a/Userland/Libraries/LibChess/UCICommand.cpp b/Userland/Libraries/LibChess/UCICommand.cpp index f68b7c1792..c87a43119e 100644 --- a/Userland/Libraries/LibChess/UCICommand.cpp +++ b/Userland/Libraries/LibChess/UCICommand.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2020, the SerenityOS developers. + * Copyright (c) 2023, Sam Atkins * * SPDX-License-Identifier: BSD-2-Clause */ @@ -17,9 +18,9 @@ ErrorOr> UCICommand::from_string(StringView command) return adopt_nonnull_own_or_enomem(new (nothrow) UCICommand); } -DeprecatedString UCICommand::to_deprecated_string() const +ErrorOr UCICommand::to_string() const { - return "uci\n"; + return "uci\n"_short_string; } ErrorOr> DebugCommand::from_string(StringView command) @@ -35,12 +36,12 @@ ErrorOr> DebugCommand::from_string(StringView comman VERIFY_NOT_REACHED(); } -DeprecatedString DebugCommand::to_deprecated_string() const +ErrorOr DebugCommand::to_string() const { if (flag() == Flag::On) { - return "debug on\n"; + return "debug on\n"_string; } else { - return "debug off\n"; + return "debug off\n"_string; } } @@ -52,9 +53,9 @@ ErrorOr> IsReadyCommand::from_string(StringView co return adopt_nonnull_own_or_enomem(new (nothrow) IsReadyCommand); } -DeprecatedString IsReadyCommand::to_deprecated_string() const +ErrorOr IsReadyCommand::to_string() const { - return "isready\n"; + return "isready\n"_string; } ErrorOr> SetOptionCommand::from_string(StringView command) @@ -92,20 +93,22 @@ ErrorOr> SetOptionCommand::from_string(StringVie VERIFY(!name.is_empty()); - return adopt_nonnull_own_or_enomem(new (nothrow) SetOptionCommand(name.to_deprecated_string().trim_whitespace(), value.to_deprecated_string().trim_whitespace())); + return adopt_nonnull_own_or_enomem(new (nothrow) SetOptionCommand( + TRY(String::from_utf8(name.string_view().trim_whitespace())), + TRY(String::from_utf8(value.string_view().trim_whitespace())))); } -DeprecatedString SetOptionCommand::to_deprecated_string() const +ErrorOr SetOptionCommand::to_string() const { StringBuilder builder; - builder.append("setoption name "sv); - builder.append(name()); + TRY(builder.try_append("setoption name "sv)); + TRY(builder.try_append(name())); if (value().has_value()) { - builder.append(" value "sv); - builder.append(value().value()); + TRY(builder.try_append(" value "sv)); + TRY(builder.try_append(value().value())); } - builder.append('\n'); - return builder.to_deprecated_string(); + TRY(builder.try_append('\n')); + return builder.to_string(); } ErrorOr> PositionCommand::from_string(StringView command) @@ -115,9 +118,9 @@ ErrorOr> PositionCommand::from_string(StringView VERIFY(tokens[0] == "position"); VERIFY(tokens[2] == "moves"); - Optional fen; + Optional fen; if (tokens[1] != "startpos") - fen = tokens[1]; + fen = TRY(String::from_utf8(tokens[1])); Vector moves; for (size_t i = 3; i < tokens.size(); ++i) { @@ -126,22 +129,22 @@ ErrorOr> PositionCommand::from_string(StringView return adopt_nonnull_own_or_enomem(new (nothrow) PositionCommand(fen, moves)); } -DeprecatedString PositionCommand::to_deprecated_string() const +ErrorOr PositionCommand::to_string() const { StringBuilder builder; - builder.append("position "sv); + TRY(builder.try_append("position "sv)); if (fen().has_value()) { - builder.append(fen().value()); + TRY(builder.try_append(fen().value())); } else { - builder.append("startpos "sv); + TRY(builder.try_append("startpos "sv)); } - builder.append("moves"sv); + TRY(builder.try_append("moves"sv)); for (auto& move : moves()) { - builder.append(' '); - builder.append(move.to_long_algebraic()); + TRY(builder.try_append(' ')); + TRY(builder.try_append(move.to_long_algebraic())); } - builder.append('\n'); - return builder.to_deprecated_string(); + TRY(builder.try_append('\n')); + return builder.to_string(); } ErrorOr> GoCommand::from_string(StringView command) @@ -190,44 +193,44 @@ ErrorOr> GoCommand::from_string(StringView command) return go_command; } -DeprecatedString GoCommand::to_deprecated_string() const +ErrorOr GoCommand::to_string() const { StringBuilder builder; - builder.append("go"sv); + TRY(builder.try_append("go"sv)); if (searchmoves.has_value()) { - builder.append(" searchmoves"sv); + TRY(builder.try_append(" searchmoves"sv)); for (auto& move : searchmoves.value()) { - builder.append(' '); - builder.append(move.to_long_algebraic()); + TRY(builder.try_append(' ')); + TRY(builder.try_append(move.to_long_algebraic())); } } if (ponder) - builder.append(" ponder"sv); + TRY(builder.try_append(" ponder"sv)); if (wtime.has_value()) - builder.appendff(" wtime {}", wtime.value()); + TRY(builder.try_appendff(" wtime {}", wtime.value())); if (btime.has_value()) - builder.appendff(" btime {}", btime.value()); + TRY(builder.try_appendff(" btime {}", btime.value())); if (winc.has_value()) - builder.appendff(" winc {}", winc.value()); + TRY(builder.try_appendff(" winc {}", winc.value())); if (binc.has_value()) - builder.appendff(" binc {}", binc.value()); + TRY(builder.try_appendff(" binc {}", binc.value())); if (movestogo.has_value()) - builder.appendff(" movestogo {}", movestogo.value()); + TRY(builder.try_appendff(" movestogo {}", movestogo.value())); if (depth.has_value()) - builder.appendff(" depth {}", depth.value()); + TRY(builder.try_appendff(" depth {}", depth.value())); if (nodes.has_value()) - builder.appendff(" nodes {}", nodes.value()); + TRY(builder.try_appendff(" nodes {}", nodes.value())); if (mate.has_value()) - builder.appendff(" mate {}", mate.value()); + TRY(builder.try_appendff(" mate {}", mate.value())); if (movetime.has_value()) - builder.appendff(" movetime {}", movetime.value()); + TRY(builder.try_appendff(" movetime {}", movetime.value())); if (infinite) - builder.append(" infinite"sv); + TRY(builder.try_append(" infinite"sv)); - builder.append('\n'); - return builder.to_deprecated_string(); + TRY(builder.try_append('\n')); + return builder.to_string(); } ErrorOr> StopCommand::from_string(StringView command) @@ -238,9 +241,9 @@ ErrorOr> StopCommand::from_string(StringView command) return adopt_nonnull_own_or_enomem(new (nothrow) StopCommand); } -DeprecatedString StopCommand::to_deprecated_string() const +ErrorOr StopCommand::to_string() const { - return "stop\n"; + return "stop\n"_short_string; } ErrorOr> IdCommand::from_string(StringView command) @@ -256,25 +259,25 @@ ErrorOr> IdCommand::from_string(StringView command) } if (tokens[1] == "name") { - return adopt_nonnull_own_or_enomem(new (nothrow) IdCommand(Type::Name, value.to_deprecated_string())); + return adopt_nonnull_own_or_enomem(new (nothrow) IdCommand(Type::Name, TRY(value.to_string()))); } else if (tokens[1] == "author") { - return adopt_nonnull_own_or_enomem(new (nothrow) IdCommand(Type::Author, value.to_deprecated_string())); + return adopt_nonnull_own_or_enomem(new (nothrow) IdCommand(Type::Author, TRY(value.to_string()))); } VERIFY_NOT_REACHED(); } -DeprecatedString IdCommand::to_deprecated_string() const +ErrorOr IdCommand::to_string() const { StringBuilder builder; - builder.append("id "sv); + TRY(builder.try_append("id "sv)); if (field_type() == Type::Name) { - builder.append("name "sv); + TRY(builder.try_append("name "sv)); } else { - builder.append("author "sv); + TRY(builder.try_append("author "sv)); } - builder.append(value()); - builder.append('\n'); - return builder.to_deprecated_string(); + TRY(builder.try_append(value())); + TRY(builder.try_append('\n')); + return builder.to_string(); } ErrorOr> UCIOkCommand::from_string(StringView command) @@ -285,9 +288,9 @@ ErrorOr> UCIOkCommand::from_string(StringView comman return adopt_nonnull_own_or_enomem(new (nothrow) UCIOkCommand); } -DeprecatedString UCIOkCommand::to_deprecated_string() const +ErrorOr UCIOkCommand::to_string() const { - return "uciok\n"; + return "uciok\n"_short_string; } ErrorOr> ReadyOkCommand::from_string(StringView command) @@ -298,9 +301,9 @@ ErrorOr> ReadyOkCommand::from_string(StringView co return adopt_nonnull_own_or_enomem(new (nothrow) ReadyOkCommand); } -DeprecatedString ReadyOkCommand::to_deprecated_string() const +ErrorOr ReadyOkCommand::to_string() const { - return "readyok\n"; + return "readyok\n"_string; } ErrorOr> BestMoveCommand::from_string(StringView command) @@ -311,13 +314,13 @@ ErrorOr> BestMoveCommand::from_string(StringView return adopt_nonnull_own_or_enomem(new (nothrow) BestMoveCommand(Move(tokens[1]))); } -DeprecatedString BestMoveCommand::to_deprecated_string() const +ErrorOr BestMoveCommand::to_string() const { StringBuilder builder; - builder.append("bestmove "sv); - builder.append(move().to_long_algebraic()); - builder.append('\n'); - return builder.to_deprecated_string(); + TRY(builder.try_append("bestmove "sv)); + TRY(builder.try_append(move().to_long_algebraic())); + TRY(builder.try_append('\n')); + return builder.to_string(); } ErrorOr> InfoCommand::from_string([[maybe_unused]] StringView command) @@ -326,11 +329,10 @@ ErrorOr> InfoCommand::from_string([[maybe_unused]] St VERIFY_NOT_REACHED(); } -DeprecatedString InfoCommand::to_deprecated_string() const +ErrorOr InfoCommand::to_string() const { // FIXME: Implement this. VERIFY_NOT_REACHED(); - return "info"; } ErrorOr> QuitCommand::from_string(StringView command) @@ -341,9 +343,9 @@ ErrorOr> QuitCommand::from_string(StringView command) return adopt_nonnull_own_or_enomem(new (nothrow) QuitCommand); } -DeprecatedString QuitCommand::to_deprecated_string() const +ErrorOr QuitCommand::to_string() const { - return "quit\n"; + return "quit\n"_short_string; } } diff --git a/Userland/Libraries/LibChess/UCICommand.h b/Userland/Libraries/LibChess/UCICommand.h index 6df7d80aa0..0c0f48ca71 100644 --- a/Userland/Libraries/LibChess/UCICommand.h +++ b/Userland/Libraries/LibChess/UCICommand.h @@ -7,8 +7,8 @@ #pragma once -#include #include +#include #include #include @@ -39,7 +39,7 @@ public: Info, Option, }; - virtual DeprecatedString to_deprecated_string() const = 0; + virtual ErrorOr to_string() const = 0; virtual ~Command() = default; @@ -59,7 +59,7 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; }; class DebugCommand : public Command { @@ -77,7 +77,7 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; Flag flag() const { return m_flag; } @@ -94,12 +94,12 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; }; class SetOptionCommand : public Command { public: - explicit SetOptionCommand(StringView name, Optional value = {}) + explicit SetOptionCommand(String name, Optional value = {}) : Command(Command::Type::SetOption) , m_name(name) , m_value(value) @@ -108,19 +108,19 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; - DeprecatedString const& name() const { return m_name; } - Optional const& value() const { return m_value; } + String const& name() const { return m_name; } + Optional const& value() const { return m_value; } private: - DeprecatedString m_name; - Optional m_value; + String m_name; + Optional m_value; }; class PositionCommand : public Command { public: - explicit PositionCommand(Optional const& fen, Vector const& moves) + explicit PositionCommand(Optional const& fen, Vector const& moves) : Command(Command::Type::Position) , m_fen(fen) , m_moves(moves) @@ -129,13 +129,13 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; - Optional const& fen() const { return m_fen; } + Optional const& fen() const { return m_fen; } Vector const& moves() const { return m_moves; } private: - Optional m_fen; + Optional m_fen; Vector m_moves; }; @@ -148,7 +148,7 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; Optional> searchmoves; bool ponder { false }; @@ -173,7 +173,7 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; }; class IdCommand : public Command { @@ -183,7 +183,7 @@ public: Author, }; - explicit IdCommand(Type field_type, StringView value) + explicit IdCommand(Type field_type, String value) : Command(Command::Type::Id) , m_field_type(field_type) , m_value(value) @@ -192,14 +192,14 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; Type field_type() const { return m_field_type; } - DeprecatedString const& value() const { return m_value; } + String const& value() const { return m_value; } private: Type m_field_type; - DeprecatedString m_value; + String m_value; }; class UCIOkCommand : public Command { @@ -211,7 +211,7 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; }; class ReadyOkCommand : public Command { @@ -223,7 +223,7 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; }; class BestMoveCommand : public Command { @@ -236,7 +236,7 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; Chess::Move move() const { return m_move; } @@ -253,7 +253,7 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; Optional depth; Optional seldepth; @@ -278,7 +278,7 @@ public: static ErrorOr> from_string(StringView command); - virtual DeprecatedString to_deprecated_string() const override; + virtual ErrorOr to_string() const override; }; } diff --git a/Userland/Libraries/LibChess/UCIEndpoint.cpp b/Userland/Libraries/LibChess/UCIEndpoint.cpp index 21fefffb42..b2bbd7d692 100644 --- a/Userland/Libraries/LibChess/UCIEndpoint.cpp +++ b/Userland/Libraries/LibChess/UCIEndpoint.cpp @@ -22,8 +22,9 @@ Endpoint::Endpoint(NonnullRefPtr in, NonnullRefPtrwrite(command.to_deprecated_string()); + auto command_string = command.to_string().release_value_but_fixme_should_propagate_errors(); + dbgln_if(UCI_DEBUG, "{} Sent UCI Command: {}", class_name(), command_string); + m_out->write(command_string); } void Endpoint::event(Core::Event& event) diff --git a/Userland/Services/ChessEngine/ChessEngine.cpp b/Userland/Services/ChessEngine/ChessEngine.cpp index 70a3a5d554..b1195590a1 100644 --- a/Userland/Services/ChessEngine/ChessEngine.cpp +++ b/Userland/Services/ChessEngine/ChessEngine.cpp @@ -14,8 +14,8 @@ using namespace Chess::UCI; void ChessEngine::handle_uci() { - send_command(IdCommand(IdCommand::Type::Name, "ChessEngine"sv)); - send_command(IdCommand(IdCommand::Type::Author, "the SerenityOS developers"sv)); + send_command(IdCommand(IdCommand::Type::Name, "ChessEngine"_string.release_value_but_fixme_should_propagate_errors())); + send_command(IdCommand(IdCommand::Type::Author, "the SerenityOS developers"_string.release_value_but_fixme_should_propagate_errors())); send_command(UCIOkCommand()); }