-
Notifications
You must be signed in to change notification settings - Fork 26
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
change enum.Piece to add color and language #158
Conversation
madcpf
commented
Sep 29, 2023
•
edited
Loading
edited
- Add Language and Color as new enum classes;
- Rename enum class Piece to be Type;
- In a new file define class Piece inheriting from QuantumObject.
GENERAL = ["g", "G", "将", "帥"] | ||
|
||
@staticmethod | ||
def type_of(c: str) -> Optional["Type"]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This function is no longer correct, it won't handle the Chinese symbols, correct?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Doug. This method (currently) is only used for convert FEN symbols into board pieces. I will rewrite it if further needs merge.
|
||
def black_symbol(self) -> str: | ||
return self.value | ||
EMPTY = [".", ".", ".", "."] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably use tuples instead of lists here, since you do not want them to be mutable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Doug. It's updated.
self.type_ = type_ | ||
self.color = color | ||
|
||
def symbol(self, lang: Language = Language.EN) -> str: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it worth adding color as an argument then moving this as part of Type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Doug. It's updated.
return type_.value[2] | ||
elif color == Color.BLACK: | ||
return type_.value[3] | ||
return "Unexpected combinations" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would probably raise an ValueError rather than silently pass a weird string.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Doug. It's updated.