Change PieceUpdated, add new events #63
Conversation
primitiveType
commented
Apr 22, 2023
primitiveType
commented
- Break PieceUpdated into 3 events to separate movement, removal and addition of pieces into separate events.
- Change PieceUpdated Action to use event semantics.
- Break PieceUpdated into 3 events to separate movement, removal and addition of pieces into separate events.
|
I am attempting to write a UI for this library and having some difficulties getting started. The primary issue this pr tries to resolve is the fact that, as it was written, the MovePiece function always invokes PieceUpdated with nullPiece (it was grabbing the piece value from the now-empty square). I debated whether it should just simply be updated to get the correct piece, or whether it should invoke 2 events, etc. Ultimately I decided that it will be easier on the UI side to deal with things If I have separate events for each type of change to the board. Otherwise there would be ambiguity (particularly when moving a piece) about which piece was moved (two rooks could move to the same square for instance). By treating them as separate events I can add from/to squares to the MovedEvent, so each PieceView in the UI knows exactly whether it was the thing affected. |
|
Looks interesting, what are you using to make the UI? WinForms? I now remember why I did it that way, because I had the rest of the information in the UI already, so I only needed some difference to the data I already had. I'm not entirely sure why you wouldn't know the from-square of a moving piece, since either you would know it's location on the board or you would get the information elsewhere (like an uci engine). |
|
At the moment I'm using Godot 4, but I might use Unity3d. My interest in this library is to essentially use it as the model, in a very roughly MVC approach. I'm hoping to use a chess engine as the "controller", and godot/unity as the view. In general it seems cleaner to me, if this library is going to manage the state of the board, that its events should include enough information that any view can be completely thin. Typically this would mean that my view for a Piece would basically refer to its model, and listen to changes in the model. ChessLib already holds the board state for me, I'd rather not have multiple things trying to track the state. This way, the chess engine will drive changes to the model, and the view will respond. I could do as you say, but then the view will have to know more about what the chess engine is doing (otherwise there would be ambiguity in cases where for example two rooks can move to the same square) and generally be more stateful. This approach seemed reasonable to me, hopefully you don't find it to be inappropriate for this library. I have a feeling I will end up wanting more events (if they don't already exist) for things like castling, checkmate etc, but I haven't looked that far ahead to see if an event is necessary (and my view is still very much in its infancy). |
| _testOutputHelper.WriteLine($"{args.MovedPiece.Value} moved from {args.From} to {args.To}."); | ||
| } | ||
| void PieceRemoved(object sender, PieceRemovedEventArgs args) |
There was a problem hiding this comment.
Spacing between these local methods for readability
| public void PieceMovedGeneratesEventWithPieceInfo() | ||
| { | ||
| int moveEventsReceived = 0; |
There was a problem hiding this comment.
Prefer var instead of int for these
|
|
||
| namespace Rudzoft.ChessLib.Events; | ||
|
|
||
| public struct PieceAddedEventArgs |
There was a problem hiding this comment.
If possible, use 'record struct', else 'readonly struct' since the properties are not changed after object creation.
|
|
||
| namespace Rudzoft.ChessLib.Events; | ||
|
|
||
| public struct PieceMovedEventArgs |
There was a problem hiding this comment.
If possible, use 'record struct', else 'readonly struct' since the properties are not changed after object creation.
|
|
||
| namespace Rudzoft.ChessLib.Events; | ||
|
|
||
| public struct PieceRemovedEventArgs |
There was a problem hiding this comment.
If possible, use 'record struct', else 'readonly struct' since the properties are not changed after object creation.
| if (!IsProbing) | ||
| PieceUpdated?.Invoke( new PieceSquareEventArgs(Piece.EmptyPiece, sq)); | ||
| { |
