feat: add admin award winner command #23
15
src/main.rs
@@ -168,6 +168,21 @@ async fn handle_connection(
|
||||
let _ = send(&tx, "ERROR:INVALID:TERMINATE");
|
||||
}
|
||||
}
|
||||
} else if parts.get(1) == Some(&"AWARD") && parts.len() > 3 {
|
||||
match parts[2].parse::<u32>() {
|
||||
Ok(match_id) => {
|
||||
let winner = parts[3].to_string();
|
||||
if let Err(e) =
|
||||
sd.handle_game_award(addr, match_id, winner).await
|
||||
{
|
||||
error!("handle_game_award: {}", e);
|
||||
let _ = send(&tx, e.to_string().as_str());
|
||||
}
|
||||
}
|
||||
Err(_) => {
|
||||
let _ = send(&tx, "ERROR:INVALID:AWARD");
|
||||
}
|
||||
}
|
||||
} else {
|
||||
let _ = send(&tx, "ERROR:INVALID:GAME");
|
||||
}
|
||||
|
||||
134
src/server.rs
@@ -818,6 +818,140 @@ impl Server {
|
||||
Ok(())
|
||||
|
|
||||
}
|
||||
|
Winner validation is comparing Winner validation is comparing `winner_username` (a username from the command) to `the_match.player1/2.to_string()` (socket addresses). This will reject valid usernames and/or accept address strings. Resolve winner identity via the `usernames` map or by reading the `Client.username` for `player1/player2`, then verify the winner belongs to this match.
```suggestion
// Validate that the declared winner is actually one of the players in this match
let clients_guard = self.clients.read().await;
let player1_client = clients_guard.get(&the_match.player1);
let player2_client = clients_guard.get(&the_match.player2);
// If we cannot resolve both players, or the winner username doesn't match either, reject
if let (Some(p1_arc), Some(p2_arc)) = (player1_client, player2_client) {
let p1 = p1_arc.read().await;
let p2 = p2_arc.read().await;
if winner_username != p1.username && winner_username != p2.username {
return Err(anyhow!("ERROR:INVALID:AWARD"));
}
} else {
return Err(anyhow!("ERROR:INVALID:AWARD"));
}
drop(clients_guard);
```
|
||||
|
||||
pub async fn handle_game_award(
|
||||
&self,
|
||||
addr: SocketAddr,
|
||||
match_id: u32,
|
||||
winner_username: String,
|
||||
) -> Result<(), anyhow::Error> {
|
||||
if !self.auth_check(addr).await {
|
||||
return Err(anyhow::anyhow!("ERROR:INVALID:AUTH"));
|
||||
}
|
||||
|
||||
let server_player_addr: SocketAddr = SERVER_PLAYER_ADDR.to_string().parse()?;
|
||||
|
||||
let (player1_addr, player2_addr, viewers, demo_mode) = {
|
||||
let mut matches_guard = self.matches.write().await;
|
||||
let the_match = matches_guard
|
||||
.get(&match_id)
|
||||
.ok_or_else(|| anyhow::anyhow!("ERROR:INVALID:AWARD"))?
|
||||
.clone();
|
||||
let mut the_match = the_match.write().await;
|
||||
|
||||
if let Some(wait_thread) = &the_match.wait_thread {
|
||||
wait_thread.abort();
|
||||
}
|
||||
|
||||
if let Some(timeout_thread) = &the_match.timeout_thread {
|
||||
timeout_thread.abort();
|
||||
}
|
||||
|
||||
let player1_addr = the_match.player1;
|
||||
let player2_addr = the_match.player2;
|
||||
let viewers = the_match.viewers.clone();
|
||||
let demo_mode = the_match.demo_mode;
|
||||
|
||||
matches_guard.remove(&match_id);
|
||||
|
||||
(player1_addr, player2_addr, viewers, demo_mode)
|
||||
};
|
||||
|
||||
let clients_guard = self.clients.read().await;
|
||||
let player1_name = if player1_addr == server_player_addr {
|
||||
SERVER_PLAYER_USERNAME.to_string()
|
||||
} else {
|
||||
clients_guard
|
||||
|
Potential deadlock: this function holds a read-lock on the match ( Potential deadlock: this function holds a read-lock on the match (`found_match.read().await`) and then acquires a write-lock on `self.matches` to remove the match. Elsewhere (e.g., `handle_play`) the lock order is `matches.write()` -> `match.write()`, which can deadlock with `match.read()` -> `matches.write()`. Reorder to avoid holding the match lock while taking `self.matches.write()`, e.g. remove/take the match entry from the map first (under `matches.write()`), drop the map lock, then operate on the match.
|
||||
.get(&player1_addr)
|
||||
.ok_or_else(|| anyhow::anyhow!("ERROR:INVALID:AWARD"))?
|
||||
.read()
|
||||
.await
|
||||
.username
|
||||
.clone()
|
||||
};
|
||||
let player2_name = if player2_addr == server_player_addr {
|
||||
SERVER_PLAYER_USERNAME.to_string()
|
||||
} else {
|
||||
clients_guard
|
||||
.get(&player2_addr)
|
||||
.ok_or_else(|| anyhow::anyhow!("ERROR:INVALID:AWARD"))?
|
||||
.read()
|
||||
.await
|
||||
.username
|
||||
.clone()
|
||||
};
|
||||
drop(clients_guard);
|
||||
|
||||
let winner_username = winner_username.trim().to_string();
|
||||
let winner_is_player1 = winner_username == player1_name;
|
||||
let winner_is_player2 = winner_username == player2_name;
|
||||
|
||||
if !winner_is_player1 && !winner_is_player2 {
|
||||
return Err(anyhow::anyhow!("ERROR:INVALID:AWARD"));
|
||||
}
|
||||
|
||||
let winner_addr = if winner_is_player1 {
|
||||
player1_addr
|
||||
} else {
|
||||
player2_addr
|
||||
};
|
||||
let loser_addr = if winner_is_player1 {
|
||||
player2_addr
|
||||
|
This uses multiple This uses multiple `unwrap()`s when fetching players from `clients_guard` (and similarly in the demo-mode branch). If a player disconnects before an admin awards the match, the client entry may already be removed (see `handle_disconnect`), causing a panic and leaving tournament/match cleanup incomplete. Prefer returning an `ERROR:INVALID:AWARD` (or terminating the match) when a client is missing, while still ensuring the match is removed and the tournament is advanced based on the stored `SocketAddr`s.
|
||||
} else {
|
||||
player1_addr
|
||||
};
|
||||
|
||||
self.broadcast_message(&viewers, &format!("GAME:WIN:{}", winner_username))
|
||||
.await;
|
||||
|
||||
let mut clients_guard = self.clients.write().await;
|
||||
if winner_addr != server_player_addr {
|
||||
let mut winner = clients_guard
|
||||
.get_mut(&winner_addr)
|
||||
.ok_or_else(|| anyhow::anyhow!("ERROR:INVALID:AWARD"))?
|
||||
.write()
|
||||
.await;
|
||||
let _ = send(&winner.connection, "GAME:WINS");
|
||||
winner.current_match = None;
|
||||
winner.color = Color::None;
|
||||
}
|
||||
|
||||
if loser_addr != server_player_addr {
|
||||
let mut loser = clients_guard
|
||||
.get_mut(&loser_addr)
|
||||
.ok_or_else(|| anyhow::anyhow!("ERROR:INVALID:AWARD"))?
|
||||
.write()
|
||||
.await;
|
||||
let _ = send(&loser.connection, "GAME:LOSS");
|
||||
loser.current_match = None;
|
||||
loser.color = Color::None;
|
||||
}
|
||||
drop(clients_guard);
|
||||
|
||||
if self.tournament.read().await.is_some() && self.matches.read().await.is_empty() {
|
||||
let mut tournament_guard = self.tournament.write().await;
|
||||
let tourney = tournament_guard.as_mut().unwrap();
|
||||
tourney.write().await.inform_winner(winner_addr, false);
|
||||
tourney.write().await.next(&self).await;
|
||||
if tourney.read().await.is_completed() {
|
||||
*tournament_guard = None;
|
||||
}
|
||||
} else if !demo_mode && self.tournament.read().await.is_none() {
|
||||
let clients_guard = self.clients.read().await;
|
||||
if winner_addr != server_player_addr {
|
||||
if let Some(winner) = clients_guard.get(&winner_addr) {
|
||||
let _ = send(&winner.read().await.connection, "TOURNAMENT:END");
|
||||
}
|
||||
|
For non-tournament matches, For non-tournament matches, `handle_play` sends `TOURNAMENT:END` to both players when the match finishes, but `handle_game_award_winner` only does this in the demo-mode branch. Without this, clients may not transition back to the post-match state after an admin-awarded result. Consider mirroring `handle_play` behavior by sending `TOURNAMENT:END` to both players when `self.tournament` is `None`.
|
||||
}
|
||||
if loser_addr != server_player_addr {
|
||||
if let Some(loser) = clients_guard.get(&loser_addr) {
|
||||
let _ = send(&loser.read().await.connection, "TOURNAMENT:END");
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub async fn handle_tournament_start(
|
||||
&self,
|
||||
addr: SocketAddr,
|
||||
|
||||
The match is removed from
self.matchesbefore playercurrent_matchfields are cleared. Sincehandle_playdoesmatches_guard.get(...).unwrap(), a player sending a move during this window can cause a server panic. To avoid this race, clear both players'current_match(and abort threads) while the match is still present, or makehandle_playtolerate a missing match instead of unwrapping.In the non-tournament path, this only sends
TOURNAMENT:ENDwhen!demo_mode. However, normal match completion inhandle_playsendsTOURNAMENT:ENDeven for demo games (to the human player). Awarding a demo match currently won’t notify the client that the match/tournament flow ended, which can leave the client state inconsistent. Align this behavior with the existing match-end logic (sendTOURNAMENT:ENDto the human player even in demo mode, and only skip the server-opponent send).matches_guard.remove(&match_id)happens before validating thatwinner_usernamematches one of the players (and before any later fallible lookups). If validation fails (or a client lookup fails), this will abort threads and drop the match even though the command returns an error. Consider deferring match removal/state teardown until after winner/participants are validated (or use a two-phase approach where you only remove once you know the award will succeed).