INDAPlus21 / INDAPlus21/murnion-chess-gui
Pass
- Dominant language
- Rust
- Stars
- 0
- Forks
- 0
- PR merge metrics
- No merged PRs in 30d
Description
**Very well done Felix!**
Spacious interface. I like your additional features!
The code is generally good structured, but would benefit greatly from variables for both readability and optimisation. You suffer from copy-pasta syndrome.
_One of MANY examples:_
```rust
if self.black_mods.contains(&Mods::Sniper(self.board.board[&Position { file: self.selected_pos.0 as u8, rank: self.selected_pos.1 as u8}])) {
sniper = true;
}
if self.black_mods.contains(&Mods::Atomic(self.board.board[&Position { file: self.selected_pos.0 as u8, rank: self.selected_pos.1 as u8}])) {
for x in 0..=2 {
for y in 0..=2 {
if x == 1 && y == 1 { continue; }
if self.board.board.contains_key(&Position { file: (pos_x + x as f32 - 1f32) as u8, rank: (pos_y + y as f32 - 1f32) as u8 }) {
match self.board.board[&Position { file: (pos_x + x as f32 - 1f32) as u8, rank: (pos_y + y as f32 - 1f32) as u8 }] {
PieceType::Pawn(_colour) => (),
_ => {
self.taken_white_pieces.push(self.board.board[&Position { file: (pos_x + x as f32 - 1f32) as u8, rank: (pos_y + y as f32 - 1f32) as u8 }]);
self.board.board.remove(&Position { file: (pos_x + x as f32 - 1f32) as u8, rank: (pos_y + y as f32 - 1f32) as u8 });
},
}
}
}
}
if !self.board.board.values().any(|x| x == &PieceType::King(Colour::White)) && !self.board.board.values().any(|x| x == &PieceType::King(Colour::White)) {
self.end_game(None);
} else if !self.board.board.values().any(|x| x == &PieceType::King(Colour::White)) {
self.end_game(Some(Colour::Black));
} else if !self.board.board.values().any(|x| x == &PieceType::King(Colour::White)) {
self.end_game(Some(Colour::White));
}
}
if self.black_mods.contains(&Mods::Extinction(self.board.board[&Position { file: pos_x as u8, rank: pos_y as u8 }])) {
let mut theoretical_board = self.board.board.clone();
theoretical_board.remove(&Position { file: pos_x as u8, rank: pos_y as u8 });
if theoretical_board.values().any(|x| x == &self.board.board[&Position { file: pos_x as u8, rank: pos_y as u8 }]) {
self.board.make_move(Position { file: self.selected_pos.0 as u8, rank: self.selected_pos.1 as u8 }.to_string(), Position { file: pos_x as u8, rank: pos_y as u8 }.to_string());
self.end_game(Some(Colour::Black));
}
}
```
_One of MANY examples:_
```rust
let selected_position = Position { file: self.selected_pos.0 as u8, rank: self.selected_pos.1 as u8 };
let selected_piecetype = self.board.board[&selected_position];
if self.black_mods.contains(&Mods::Sniper(selected_piecetype)) {
sniper = true;
}
if self.black_mods.contains(&Mods::Atomic(selected_piecetype)) {
for x in 0..=2 {
for y in 0..=2 {
if x == 1 && y == 1 { continue; }
let target_position = Position { file: (pos_x + x as f32 - 1f32) as u8, rank: (pos_y + y as f32 - 1f32) as u8 };
if self.board.board.contains_key(&target_position) {
match self.board.board[&target_position] {
PieceType::Pawn(_) => (),
_piecetype => {
self.taken_white_pieces.push(_piecetype);
self.board.board.remove(&target_position);
},
}
}
}
}
let is_white_king_alive = !self.board.board.values().any(|x| x == &PieceType::King(Colour::White));
if is_white_king_alive && is_white_king_alive {
self.end_game(None);
} else if is_white_king_alive {
self.end_game(Some(Colour::Black));
} else if is_white_king_alive {
self.end_game(Some(Colour::White));
}
}
let focus_position = Position { file: pos_x as u8, rank: pos_y as u8 };
let focus_piecetype = self.board.board[&focus_position];
if self.black_mods.contains(&Mods::Extinction(focus_piecetype)) {
let mut theoretical_board = self.board.board.clone();
theoretical_board.remove(&focus_position);
if theoretical_board.values().any(|x| x == &focus_piecetype) {
self.board.make_move(selected_position.to_string(), focus_position.to_string());
self.end_game(Some(Colour::Black));
}
}
```
Pleace observe the lethal symptome of copy-pasta syndrome called: _I forgot to replace the original values correctly_. Like WTF dude.
**This**:
```rust
if !self.board.board.values().any(|x| x == &PieceType::King(Colour::White)) && !self.board.board.values().any(|x| x == &PieceType::King(Colour::White)) {
self.end_game(None);
} else if !self.board.board.values().any(|x| x == &PieceType::King(Colour::White)) {
self.end_game(Some(Colour::Black));
} else if !self.board.board.values().any(|x| x == &PieceType::King(Colour::White)) {
self.end_game(Some(Colour::White));
}
```
**is funcitonally equal to**:
```rust
let is_white_king_alive = !self.board.board.values().any(|x| x == &PieceType::King(Colour::White));
if is_white_king_alive && is_white_king_alive {
self.end_game(None);
} else if is_white_king_alive {
self.end_game(Some(Colour::Black));
} else if is_white_king_alive {
self.end_game(Some(Colour::White));
}
```
Contributor guide
No contributing guide indexed for this repository
Research direction
Locate the move and modifier-handling code represented by the snippets, then inspect how positions, piece types, and king-state checks are repeated. Refactor the repeated lookups into variables and verify that each white and black king condition remains distinct. Done means the duplicated expressions are removed without changing the chess behavior.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- game-dev
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 35/100