INDAPlus21 / INDAPlus21/murnion-chess-gui

Pass

Open
#2 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.