Resolve 8 issues

.gitignore

@@ -18,6 +18,6 @@
 */__pycache__
 */*/__pycache__
 coverage.xml
-.coverage
+.coverage*
 docs/
 .vscode/

README.md

@@ -3,7 +3,7 @@
 > [!NOTE]
 > This section refers to the technical application. If you are looking for information regarding the status of this project and the original repo, please look [here](https://github.com/py-stockfish/stockfish/tree/master#status-of-the-project).
 
-Implements an easy-to-use Stockfish class to integrates the Stockfish chess engine with Python.
+Wraps the open-source Stockfish chess engine for easy integration into python.
 
 ## Install
 
@@ -79,6 +79,12 @@ These parameters can also be updated at any time by calling the "update_engine_p
 stockfish.update_engine_parameters({"Hash": 2048, "UCI_Chess960": True}) # Gets stockfish to use a 2GB hash table, and also to play Chess960.
 ```
 
+As for the depth, it can also be updated, by using the following function. Note that if you don't set depth to a value yourself, the python module will initialize it to 15 by default.
+
+```python
+stockfish.set_depth(12)
+```
+
 When you're done using the Stockfish engine process, you can send the "quit" uci command to it with:
 
 ```python
@@ -90,19 +96,25 @@ The `__del__()` method of the Stockfish class will call send_quit_command(), but
 ### Set position by a sequence of moves from the starting position
 
 ```python
-stockfish.set_position(["e2e4", "e7e6"])
+stockfish.make_moves_from_start(["e2e4", "e7e6"])
+```
+If you'd just like to set up the starting position without making any moves from it, just call this function without sending an argument:
+```python
+stockfish.make_moves_from_start()
 ```
 
 ### Update position by making a sequence of moves from the current position
 
+Function takes a list of strings as its argument. Each string represents a move, and must have the format of the starting coordinate followed by the ending coordinate. If a move leads to a pawn promoting, then an additional character must be appended at the end (to indicate what piece the pawn promotes into).
+Other types of special moves (e.g., checks, captures, checkmates, en passants) do not need any special notation; the starting coordinate followed by the ending coordinate is all the information that's needed. Note that castling is represented by the starting coordinate of the king followed by the ending coordinate of the king. So "e1g1" would be used for white castling kingside, assuming the white king is still on e1 and castling is legal.
+Example call (assume in the current position, it is White's turn):
 ```python
-stockfish.make_moves_from_current_position(["g4d7", "a8b8", "f1d1"])
+stockfish.make_moves_from_current_position(["g4d7", "a8b8", "f1d1", "b2b1q"]) # Moves the white piece on g4 to d7, then the black piece on a8 to b8, then the white piece on f1 to d1, and finally pushes the black b2-pawn to b1, promoting it into a queen.
 ```
 
 ### Set position by Forsyth–Edwards Notation (FEN)
 
-If you'd like to first check if your fen is valid, call the is_fen_valid() function below.
-Also, if you want to play Chess960, it's recommended you first update the "UCI_Chess960" engine parameter to be True, before calling set_fen_position.
+Note that if you want to play Chess960, it's recommended you first update the "UCI_Chess960" engine parameter to be True, before calling set_fen_position.
 
 ```python
 stockfish.set_fen_position("rnbqkbnr/pppp1ppp/4p3/8/4P3/8/PPPP1PPP/RNBQKBNR w KQkq - 0 2")
@@ -115,6 +127,10 @@ The function isn't perfect and won't catch all cases, but generally it should re
 For example, one exception is positions which are legal, but have no legal moves.
 I.e., for checkmates and stalemates, this function will incorrectly say the fen is invalid.
 
+Note that the function checks whether a position is legal by temporarily creating a new Stockfish process, and
+then seeing if it can return a best move (and also not crash). Whatever the outcome may be though, this
+temporary SF process should terminate after the function call.
+
 ```python
 stockfish.is_fen_valid("rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1")
 ```
@@ -161,6 +177,8 @@ e2e4
 
 ### Check if a move is legal in the current position
 
+Returns True if the passed in move is legal in the current position.
+
 ```python
 stockfish.is_move_correct('a2a3')
 ```
@@ -171,15 +189,21 @@ True
 
 ### Get info on the top n moves
 
-Get moves, centipawns, and mates for the top n moves. If the move is a mate, the Centipawn value will be None, and vice versa. Note that if you have stockfish on a weaker elo or skill level setting, the top moves returned by this
-function will still be for full strength.
+Returns a list of dictionaries, where each dictionary represents a move's info. Each dictionary will contain a value for the 'Move' key, and either the 'Centipawn' or 'Mate' value will be a number (the other will be None).
+Positive values mean advantage white, negative advantage black (unless you're using the turn perspective setting).
+
+Positive values mean advantage White, negative values mean advantage Black (unless you're using the turn perspective option, in which case positive is for the side to move).
+
+Note that if you have stockfish on a weaker elo or skill level setting, the top moves returned by this function will still be for full strength.
+
+Let's consider an example where Black is to move, and the top 3 moves are a mate, winning material, or being slightly worse. We'll assume the turn perspective setting is off.
 
 ```python
 stockfish.get_top_moves(3)
 # [
-#   {'Move': 'f5h7', 'Centipawn': None, 'Mate': 1},
-#   {'Move': 'f5d7', 'Centipawn': 713, 'Mate': None},
-#   {'Move': 'f5h5', 'Centipawn': -31, 'Mate': None}
+#   {'Move': 'f5h3', 'Centipawn': None, 'Mate': -1}, # The move f5h3 leads to a mate in 1 for Black.
+#   {'Move': 'f5d7', 'Centipawn': -713, 'Mate': None}, # f5d7 leads to an evaluation of 7.13 in Black's favour.
+#   {'Move': 'f5h5', 'Centipawn': 31, 'Mate': None} # f5h5 leads to an evaluation of 0.31 in White's favour.
 # ]
 ```
 
@@ -285,7 +309,7 @@ stockfish.set_elo_rating(1350)
 stockfish.resume_full_strength()
 ```
 
-### Set the engine's depth
+### Set the engine's search depth
 
 ```python
 stockfish.set_depth(15)
@@ -485,17 +509,30 @@ stockfish.is_development_build_of_engine()
 False
 ```
 
+### Send the "ucinewgame" command to the Stockfish engine process.
+This command will clear Stockfish's hash table, which is relatively expensive and should generally only be done if the new position will be completely unrelated to the current one (such as a new game).
+```python
+stockfish.send_ucinewgame_command()
+```
+
 ### Find what is on a certain square
 
 If the square is empty, the None object is returned. Otherwise, one of 12 enum members of a custom
 Stockfish.Piece enum will be returned. Each of the 12 members of this enum is named in the following pattern:
 _colour_ followed by _underscore_ followed by _piece name_, where the colour and piece name are in all caps.
+The value of each enum member is a char representing the piece (uppercase is white, lowercase is black).
+For white, it will be one of "P", "N", "B", "R", "Q", or "K". For black the same chars, except lowercase.
 For example, say the current position is the starting position:
 
 ```python
 stockfish.get_what_is_on_square("e1") # returns Stockfish.Piece.WHITE_KING
+stockfish.get_what_is_on_square("e1").value # result is "K"
 stockfish.get_what_is_on_square("d8") # returns Stockfish.Piece.BLACK_QUEEN
+stockfish.get_what_is_on_square("d8").value # result is "q"
 stockfish.get_what_is_on_square("h2") # returns Stockfish.Piece.WHITE_PAWN
+stockfish.get_what_is_on_square("h2").value # result is "P"
+stockfish.get_what_is_on_square("g8") # returns Stockfish.Piece.BLACK_KNIGHT
+stockfish.get_what_is_on_square("g8").value # result is "n"
 stockfish.get_what_is_on_square("b5") # returns None
 ```
 

stockfish/models.py

@@ -15,6 +15,7 @@
 import re
 import datetime
 import warnings
+import platform
 
 
 class Stockfish:
@@ -42,7 +43,7 @@ class Stockfish:
     _PARAM_RESTRICTIONS: Dict[str, Tuple[type, Optional[int], Optional[int]]] = {
         "Debug Log File": (str, None, None),
         "Threads": (int, 1, 1024),
-        "Hash": (int, 1, 2048),
+        "Hash": (int, 1, 2 ** (25 if "64" in platform.machine() else 11)),
         "Ponder": (bool, None, None),
         "MultiPV": (int, 1, 500),
         "Skill Level": (int, 0, 20),
@@ -118,7 +119,8 @@ def __init__(
         if self.does_current_engine_version_have_wdl_option():
             self._set_option("UCI_ShowWDL", True, False)
 
-        self._prepare_for_new_position(True)
+        self._prepare_for_new_position()
+        self._is_ready()
 
     def set_debug_view(self, activate: bool) -> None:
         self._debug_view = activate
@@ -199,7 +201,7 @@ def update_engine_parameters(self, parameters: Optional[dict]) -> None:
 
         for name, value in new_param_values.items():
             self._set_option(name, value)
-        self.set_fen_position(self.get_fen_position(), False)
+        self.set_fen_position(self.get_fen_position())
         # Getting SF to set the position again, since UCI option(s) have been updated.
 
     def reset_engine_parameters(self) -> None:
@@ -210,16 +212,26 @@ def reset_engine_parameters(self) -> None:
         """
         self.update_engine_parameters(self._DEFAULT_STOCKFISH_PARAMS)
 
-    def _prepare_for_new_position(self, send_ucinewgame_token: bool = True) -> None:
-        if send_ucinewgame_token:
+    def send_ucinewgame_command(self) -> None:
+        """Sends the `ucinewgame` command to the Stockfish engine. This will clear Stockfish's
+        hash table, which is relatively expensive and should generally only be done if the
+        new position will be completely unrelated to the current one (such as a new game).
+        """
+        if self._stockfish.poll() is None:
             self._put("ucinewgame")
-        self._is_ready()
+            self._is_ready()
+
+    def _prepare_for_new_position(self) -> None:
         self.info = ""
 
     def _put(self, command: str) -> None:
+        """Sends a command to the Stockfish engine. Note that this function shouldn't be called if
+        there's any existing output in stdout that's still needed."""
         if not self._stockfish.stdin:
             raise BrokenPipeError()
         if self._stockfish.poll() is None and not self._has_quit_command_been_sent:
+            if command != "isready":
+                self._is_ready()
             if self._debug_view:
                 print(f">>> {command}\n")
             self._stockfish.stdin.write(f"{command}\n")
@@ -266,6 +278,8 @@ def _validate_param_val(self, name: str, value: Any) -> None:
             raise ValueError(f"{value} is over {name}'s maximum value of {maximum}")
 
     def _is_ready(self) -> None:
+        """Waits if the engine is busy. Note that this function shouldn't be called if
+        there's any existing output in stdout that's still needed."""
         self._put("isready")
         while self._read_line() != "readyok":
             pass
@@ -300,31 +314,24 @@ def _weaker_setting_warning(self, message: str) -> None:
         """Will issue a warning, referring to the function that calls this one."""
         warnings.warn(message, stacklevel=3)
 
-    def set_fen_position(
-        self, fen_position: str, send_ucinewgame_token: bool = True
-    ) -> None:
-        """Sets current board position in Forsyth-Edwards notation (FEN).
+    def set_fen_position(self, fen_position: str) -> None:
+        """Sets the current board position from Forsyth-Edwards notation (FEN).
 
         Args:
             fen_position:
               FEN string of board position.
 
-            send_ucinewgame_token:
-              Whether to send the `ucinewgame` token to the Stockfish engine.
-              The most prominent effect this will have is clearing Stockfish's transposition table,
-              which should be done if the new position is unrelated to the current position.
-
         Returns:
             `None`
 
         Example:
             >>> stockfish.set_fen_position("1nb1k1n1/pppppppp/8/6r1/5bqK/6r1/8/8 w - - 2 2")
         """
-        self._prepare_for_new_position(send_ucinewgame_token)
+        self._prepare_for_new_position()
         self._put(f"position fen {fen_position}")
 
-    def set_position(self, moves: Optional[List[str]] = None) -> None:
-        """Sets current board position.
+    def make_moves_from_start(self, moves: Optional[List[str]] = None) -> None:
+        """Sets the position by making a sequence of moves from the starting position of chess.
 
         Args:
             moves:
@@ -335,10 +342,10 @@ def set_position(self, moves: Optional[List[str]] = None) -> None:
             `None`
 
         Example:
-            >>> stockfish.set_position(['e2e4', 'e7e5'])
+            >>> stockfish.make_moves_from_start(['e2e4', 'e7e5'])
         """
         self.set_fen_position(
-            "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1", True
+            "rnbqkbnr/pppppppp/8/8/8/8/PPPPPPPP/RNBQKBNR w KQkq - 0 1"
         )
         self.make_moves_from_current_position(moves)
 
@@ -358,11 +365,8 @@ def make_moves_from_current_position(self, moves: Optional[List[str]]) -> None:
         """
         if not moves:
             return
-        self._prepare_for_new_position(False)
-        for move in moves:
-            if not self.is_move_correct(move):
-                raise ValueError(f"Cannot make move: {move}")
-            self._put(f"position fen {self.get_fen_position()} moves {move}")
+        self._prepare_for_new_position()
+        self._put(f"position fen {self.get_fen_position()} moves {' '.join(moves)}")
 
     def get_board_visual(self, perspective_white: bool = True) -> str:
         """Returns a visual representation of the current board position.
@@ -639,14 +643,12 @@ def _is_fen_syntax_valid(fen: str) -> bool:
 
         fen_fields = fen.split()
 
-        if any(
-            (
-                len(fen_fields) != 6,
-                len(fen_fields[0].split("/")) != 8,
-                any(x not in fen_fields[0] for x in "Kk"),
-                any(not fen_fields[x].isdigit() for x in (4, 5)),
-                int(fen_fields[4]) >= int(fen_fields[5]) * 2,
-            )
+        if (
+            len(fen_fields) != 6
+            or len(fen_fields[0].split("/")) != 8
+            or any(x not in fen_fields[0] for x in "Kk")
+            or any(not fen_fields[x].isdigit() for x in (4, 5))
+            or int(fen_fields[4]) >= int(fen_fields[5]) * 2
         ):
             return False
 
@@ -683,7 +685,7 @@ def is_fen_valid(self, fen: str) -> bool:
         # Using a new temporary SF instance, in case the fen is an illegal position that causes
         # the SF process to crash.
         best_move: Optional[str] = None
-        temp_sf.set_fen_position(fen, False)
+        temp_sf.set_fen_position(fen)
         try:
             temp_sf._put("go depth 10")
             best_move = temp_sf._get_best_move_from_sf_popen_process()
@@ -768,7 +770,7 @@ def does_current_engine_version_have_wdl_option(self) -> bool:
             splitted_text = self._read_line().split(" ")
             if splitted_text[0] == "uciok":
                 return False
-            elif "UCI_ShowWDL" in splitted_text:
+            if "UCI_ShowWDL" in splitted_text:
                 self._discard_remaining_stdout_lines("uciok")
                 return True
 
@@ -836,10 +838,10 @@ def get_static_eval(self) -> Optional[float]:
                     self._read_line()
                     # Consume the remaining line (for some reason `eval` outputs an extra newline)
                 if static_eval == "none":
-                    assert "(in check)" in text
+                    if "(in check)" not in text:
+                        raise RuntimeError()
                     return None
-                else:
-                    return float(static_eval) * compare
+                return float(static_eval) * compare
 
     def get_top_moves(
         self,
@@ -1012,7 +1014,8 @@ def get_perft(self, depth: int) -> Tuple[int, dict[str, int]]:
                 num_nodes = int(line.split(":")[1])
                 break
             move, num = line.split(":")
-            assert move not in move_possibilities
+            if move in move_possibilities:
+                raise RuntimeError()
             move_possibilities[move] = int(num)
         self._read_line()  # Consumes the remaining newline stockfish outputs.
 
@@ -1085,25 +1088,22 @@ def will_move_be_a_capture(self, move_value: str) -> Capture:
         if ending_square_piece is not None:
             if not self._parameters["UCI_Chess960"]:
                 return Stockfish.Capture.DIRECT_CAPTURE
-            else:
-                # Check for Chess960 castling:
-                castling_pieces = [
-                    [Stockfish.Piece.WHITE_KING, Stockfish.Piece.WHITE_ROOK],
-                    [Stockfish.Piece.BLACK_KING, Stockfish.Piece.BLACK_ROOK],
-                ]
-                if [starting_square_piece, ending_square_piece] in castling_pieces:
-                    return Stockfish.Capture.NO_CAPTURE
-                else:
-                    return Stockfish.Capture.DIRECT_CAPTURE
-        elif move_value[2:4] == self.get_fen_position().split()[
+            # Check for Chess960 castling:
+            castling_pieces = [
+                [Stockfish.Piece.WHITE_KING, Stockfish.Piece.WHITE_ROOK],
+                [Stockfish.Piece.BLACK_KING, Stockfish.Piece.BLACK_ROOK],
+            ]
+            if [starting_square_piece, ending_square_piece] in castling_pieces:
+                return Stockfish.Capture.NO_CAPTURE
+            return Stockfish.Capture.DIRECT_CAPTURE
+        if move_value[2:4] == self.get_fen_position().split()[
             3
         ] and starting_square_piece in [
             Stockfish.Piece.WHITE_PAWN,
             Stockfish.Piece.BLACK_PAWN,
         ]:
             return Stockfish.Capture.EN_PASSANT
-        else:
-            return Stockfish.Capture.NO_CAPTURE
+        return Stockfish.Capture.NO_CAPTURE
 
     def get_stockfish_full_version(self) -> float:
         """Returns Stockfish engine full version."""
@@ -1196,7 +1196,7 @@ def _parse_stockfish_version(self, version_text: str = "") -> None:
         except Exception as e:
             raise Exception(
                 "Unable to parse Stockfish version. You may be using an unsupported version of Stockfish."
-            )
+            ) from e
 
     def _get_stockfish_version_from_build_date(
         self, date_string: str = ""