Add doc comments & tests. Some refactoring #28
Conversation
| let bb = to_bb(vec![SQ_6H, SQ_8F]); | ||
| assert_eq!( | ||
| bb | to_bb(vec![SQ_6G, SQ_7F]), | ||
| bb.sliding_positives(&[ |
There was a problem hiding this comment.
Wanted to make sure where the positives are
| #[test] | ||
| fn evasion_moves() { | ||
| // TODO: add more cases | ||
| // Behind RY |
There was a problem hiding this comment.
Cover the line 57, described as
龍が斜め位置から王手している場合のみ、他の駒の裏に逃がれることができる可能性がある
src/position.rs
Outdated
| @@ -346,17 +348,24 @@ impl From<shogi_core::PartialPosition> for PartialPosition { | |||
|
|
|||
| #[derive(Debug, Clone)] | |||
| struct State { | |||
| /// Zobrist hashes for (board ^ side, hand) | |||
| /// TODO: It seems it's always used by XOR-ing two keys. Can we only store a single XOR-ed key? | |||
There was a problem hiding this comment.
This is a question for you and probably should be revised before merging
There was a problem hiding this comment.
Although I'm not using it specifically, I think that it would be easier to calculate the judgment when searching for positions with a "superiority relationship" if it is possible to independently obtain the Hash of the board only, not including the hand pieces, for example, when searching for Tsume-Shogi.
There was a problem hiding this comment.
nit: grammar
| /// TODO: It seems it's always used by XOR-ing two keys. Can we only store a single XOR-ed key? | |
| /// TODO: It seems whenever we use this field its components (two keys) are always XOR-ed. Can we only store a single XOR-ed key? |
| @@ -43,50 +43,49 @@ impl Position { | |||
| /// Generate moves to evade check, optimized using AttackInfo. | |||
| fn generate_evasions(&self, av: &mut ArrayVec<Move, MAX_LEGAL_MOVES>) { | |||
| let c = self.side_to_move(); | |||
| if let Some(king) = self.king_position(c) { | |||
There was a problem hiding this comment.
Unwrapped king_position() as this method should be called during evading
| let mut checkers_attacks = Bitboard::empty(); | ||
| let mut checkers_count = 0; | ||
| for ch in self.checkers() { | ||
| if let Some(p) = self.piece_at(ch) { |
There was a problem hiding this comment.
Unwrapped piece_at(ch) as checkers() should only return positions where pieces exist
| self.generate_for_ki(av, &target_move); | ||
| self.generate_for_um(av, &target_move); | ||
| self.generate_for_ry(av, &target_move); | ||
| if !target_drop.is_empty() { |
There was a problem hiding this comment.
I'm guessing generate_drop is somewhat costly/redundant in case target_drop is empty, which is fairly often like in case attacker is not 飛び駒.
| } | ||
| let pk = piece.piece_kind(); | ||
| let pk = if promote { | ||
| pk.promote().unwrap_or(pk) |
There was a problem hiding this comment.
I did a few refactor with unwrap_or to improve readability. Hoping if this is not harmful for performance.
| return !(BETWEEN_TABLE[sq.array_index()][from.array_index()].contains(to) | ||
| || BETWEEN_TABLE[sq.array_index()][to.array_index()].contains(from)); | ||
| } | ||
| let c = self.inner.side.flip(); |
There was a problem hiding this comment.
When only flip side is used for multiple times, I rewrote to flipping it once than doing it in every occurrence.
| @@ -393,14 +381,14 @@ impl AttackInfo { | |||
| let ka = ATTACK_TABLE.ka.attack(sq, &occ); | |||
| let hi = ATTACK_TABLE.hi.attack(sq, &occ); | |||
| let ki = ATTACK_TABLE.ki.attack(sq, opp); | |||
| let ou = ATTACK_TABLE.ou.attack(sq, opp); | |||
There was a problem hiding this comment.
Inspired by calculate_checkers(), omit one table lookup for OU.
Apply `cargo fmt`
Co-authored-by: Hiroki Kobayashi <[email protected]>
Thanks for checking this! That looks good and I merged it to the PR. That makes sense, I removed that TODO comment from code base. Also applied the suggested grammar fixes. Could you check this now? Thanks! |
I'm interested in using (and potentially extending) this library and read the codebase to understand how it works. Along the way,
Please be patient if I do stuff wrong: I'm not very fluent in Rust. And thanks for the great library!