Skip to content

Add Game State Management & Validation #204

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.

Sign up for GitHub

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

Merged
KevinMB0220 merged 2 commits into from
Sep 12, 2025
Merged

Add Game State Management & Validation #204

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
163c0c6
added player in game state validation
truthixify Sep 11, 2025
29d8b14
made some changes according to review
truthixify Sep 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions contract/src/models/player.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ pub struct Player {
pub level: u8,
pub xp: u32,
pub current_wave: u32,
pub in_game: bool,
}

#[derive(Copy, Drop, Serde, Debug)]
Expand Down Expand Up @@ -46,7 +47,7 @@ pub impl PlayerAssert of AssertTrait {
pub impl ZeroablePlayerTrait of Zero<Player> {
#[inline(always)]
fn zero() -> Player {
Player { address: ZERO_ADDRESS(), level: 0, xp: 0, current_wave: 0 }
Player { address: ZERO_ADDRESS(), level: 0, xp: 0, current_wave: 0, in_game: false }
}

#[inline(always)]
Expand All @@ -61,7 +62,7 @@ pub impl ZeroablePlayerTrait of Zero<Player> {
}

pub fn spawn_player(address: ContractAddress) -> Player {
Player { address, level: 1, xp: 0, current_wave: 1 }
Player { address, level: 1, xp: 0, current_wave: 1, in_game: true }
}

#[cfg(test)]
Expand All @@ -78,12 +79,14 @@ mod tests {
assert(player.level == 1, 'Invalid level');
assert(player.xp == 0, 'Invalid xp');
assert(player.current_wave == 1, 'Invalid wave');
assert(player.in_game == true, 'Invalid in_game');
}

#[test]
fn test_zero_player() {
let zero_player = ZeroablePlayerTrait::zero();
assert(zero_player.is_zero(), 'Should be zero');
assert(zero_player.level == 0, 'Level should be zero');
assert(zero_player.in_game == false, 'in_game should be false');
}
}
77 changes: 69 additions & 8 deletions contract/src/systems/game.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,12 @@ pub trait IBrawlGame<T> {
) -> u256;
}

#[derive(Copy, Drop, Serde)]
#[derive(Copy, Default, Drop, Serde)]
pub enum PlayerStatus {
Alive,
Dead,
InGame,
#[default]
Waiting,
}

Expand All @@ -47,6 +48,8 @@ pub mod brawl_game {
use stark_brawl::store::{Store, StoreImpl};
use stark_brawl::systems::player::{IPlayerSystemDispatcher, IPlayerSystemDispatcherTrait};

use core::num::traits::Bounded;

#[storage]
struct Storage {
game_counter: u32,
Expand All @@ -63,10 +66,17 @@ pub mod brawl_game {
fn join_game(ref self: ContractState) {
let mut world = self.world_default();
let caller = get_caller_address();
let existing_player: Player = world.read_model(caller);

let player = spawn_player(caller);

world.write_model(@player);
if existing_player.is_zero() {
let player = spawn_player(caller);
world.write_model(@player);
} else {
assert(!existing_player.in_game, 'Player already in active game');
let mut player = existing_player;
player.in_game = true;
world.write_model(@player);
}

Comment on lines +69 to 80
Copy link

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Rejoin path: verify semantics of initialize() and dead players.

If initialize(caller) resets run state, calling it when re-using an existing player may wipe progress. Also, re-joining while dead isn’t blocked here.

Proposed tweak:

         if existing_player.is_zero() {
             let player = spawn_player(caller);
             world.write_model(@player);
         } else {
             assert(!existing_player.in_game, 'Player already in active game');
+            // Optional: prevent rejoin while dead, or call a dedicated respawn API.
+            let ps = self.player_system_dispatcher();
+            assert(ps.is_alive(caller), 'Cannot rejoin while dead');
             let mut player = existing_player;
             player.in_game = true;
             world.write_model(@player);
         }

🏁 Script executed:

#!/bin/bash
# Inspect player-system initialize() to confirm idempotency vs. reset semantics.
rg -n -C3 -P '\bimpl\b|\bfn\s+initialize\s*\(' --type=cairo

Length of output: 29

🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf "PWD: %s\n\n" "$(pwd)"

printf "----- contract/src/systems/game.cairo (first 240 lines if exists) -----\n"
if [ -f contract/src/systems/game.cairo ]; then
  sed -n '1,240p' contract/src/systems/game.cairo || true
else
  printf "contract/src/systems/game.cairo not found\n"
fi

printf "\n----- Ripgrep searches (showing matches) -----\n"

printf "\nspawn_player occurrences:\n"
rg -n -S 'spawn_player\(' || true

printf "\nis_alive occurrences:\n"
rg -n -S 'is_alive\(' || true

printf "\ninitialize occurrences:\n"
rg -n -S '\binitialize\(' || true

printf "\nplayer_system_dispatcher occurrences:\n"
rg -n -S 'player_system_dispatcher' || true

printf "\nstruct Player occurrences:\n"
rg -n -S 'struct\s+Player|struct Player' || true

printf "\nin_game occurrences:\n"
rg -n -S '\bin_game\b' || true

printf "\nis_zero occurrences:\n"
rg -n -S 'is_zero\(' || true

# If spawn_player matches were found, show the files (first 240 lines) for inspection.
files=$(rg -n -S 'spawn_player\(' | cut -d: -f1 | sort -u || true)
if [ -n "$files" ]; then
  for f in $files; do
    printf "\n----- File: %s (first 240 lines) -----\n" "$f"
    sed -n '1,240p' "$f" || true
  done
fi

# Show files containing initialize and is_alive (first 240 lines) if any.
files_init=$(rg -n -S '\binitialize\(' | cut -d: -f1 | sort -u || true)
if [ -n "$files_init" ]; then
  for f in $files_init; do
    printf "\n----- File: %s (first 240 lines) -----\n" "$f"
    sed -n '1,240p' "$f" || true
  done
fi

files_alive=$(rg -n -S 'is_alive\(' | cut -d: -f1 | sort -u || true)
if [ -n "$files_alive" ]; then
  for f in $files_alive; do
    printf "\n----- File: %s (first 240 lines) -----\n" "$f"
    sed -n '1,240p' "$f" || true
  done
fi

Length of output: 120

🏁 Script executed:

#!/bin/bash
set -e
echo "PWD: $(pwd)"

echo
echo "----- contract/src/systems/game.cairo (first 240 lines) -----"
if [ -r contract/src/systems/game.cairo ]; then
  sed -n '1,240p' contract/src/systems/game.cairo || true
else
  echo "contract/src/systems/game.cairo not found"
fi

echo
echo "----- Searching for symbols (rg if available, else grep) -----"
PATTERNS=(
  'spawn_player\('
  'is_alive\('
  '\binitialize\('
  'player_system_dispatcher'
  'struct\s+Player'
  '\bin_game\b'
  'is_zero\('
)

if command -v rg >/dev/null 2>&1; then
  echo "Using rg to search"
  for pat in "${PATTERNS[@]}"; do
    echo
    echo "=== PATTERN: $pat ==="
    rg -n -S --hidden --no-ignore-vcs -e "$pat" || echo "no matches"
  done
else
  echo "rg not available, using grep"
  for pat in "${PATTERNS[@]}"; do
    echo
    echo "=== PATTERN: $pat ==="
    grep -RIn --include='*.cairo' -E "$pat" . || echo "no matches"
  done
fi

# Show first 240 lines of files that contain key symbols (if any)
if command -v rg >/dev/null 2>&1; then
  files=$(rg -l -S -e 'spawn_player\(|\bis_alive\(|\binitialize\(|player_system_dispatcher' || true)
else
  files=$(grep -RIl --include='*.cairo' -E 'spawn_player\(|\bis_alive\(|\binitialize\(|player_system_dispatcher' . || true)
fi

if [ -n "$files" ]; then
  echo
  echo "----- Showing first 240 lines of matching files -----"
  for f in $files; do
    echo
    echo "----- File: $f -----"
    sed -n '1,240p' "$f" || true
  done
else
  echo
  echo "No matching files found for spawn_player/is_alive/initialize/player_system_dispatcher"
fi

Length of output: 59507

Rejoin path: initialize() resets player-system state — block or change behavior.

player_system.initialize(...) writes hp, max_hp, coins, gems, equipped_ability, active_towers, mana, max_mana (contract/src/systems/player.cairo: initialize). join_game calls initialize(caller) for both new and existing players (contract/src/systems/game.cairo: join_game), so rejoining will wipe those player-system fields and will also revive dead players. Fix options: call initialize() only when spawn_player() creates a player, make initialize non-destructive / add a dedicated respawn API, or deny rejoin while dead by checking is_alive before setting in_game.

         } else {
             assert(!existing_player.in_game, 'Player already in active game');
+            let ps = self.player_system_dispatcher();
+            assert(ps.is_alive(caller), 'Cannot rejoin while dead');
             let mut player = existing_player;
             player.in_game = true;
             world.write_model(@player);
         }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
let existing_player: Player = world.read_model(caller);
let player = spawn_player(caller);
world.write_model(@player);
if existing_player.is_zero() {
let player = spawn_player(caller);
world.write_model(@player);
} else {
assert(!existing_player.in_game, 'Player already in active game');
let mut player = existing_player;
player.in_game = true;
world.write_model(@player);
}
let existing_player: Player = world.read_model(caller);
if existing_player.is_zero() {
let player = spawn_player(caller);
world.write_model(@player);
} else {
assert(!existing_player.in_game, 'Player already in active game');
let ps = self.player_system_dispatcher();
assert(ps.is_alive(caller), 'Cannot rejoin while dead');
let mut player = existing_player;
player.in_game = true;
world.write_model(@player);
}
🤖 Prompt for AI Agents 
In contract/src/systems/game.cairo around lines 69–80, join_game currently calls
initialize(caller) for both new and existing players which overwrites
hp/coins/etc and can revive dead players; change join_game so initialize(...) is
only invoked when spawn_player() creates a new player (i.e., when
existing_player.is_zero()), and for existing players do NOT call initialize;
instead assert the player is alive (or deny rejoin if not) before setting
in_game, or add a separate respawn API that explicitly resets hp/max_hp and
other fields rather than reusing initialize.

let player_system_dispatcher = self.player_system_dispatcher();
player_system_dispatcher.initialize(caller);
Expand All @@ -81,6 +91,7 @@ pub mod brawl_game {
// Check if the player exists
let player: Player = world.read_model(caller);
assert(!player.is_zero(), 'Player does not exist');
assert(player.in_game, 'Player not in active game');

// Check if the ability exists
let ability: Ability = world.read_model(ability_id);
Expand Down Expand Up @@ -136,15 +147,37 @@ pub mod brawl_game {
}

fn take_damage(ref self: ContractState, amount: u32) {
// let mut world = self.world_default();
let caller = get_caller_address();
let mut world = self.world_default();
let player: Player = world.read_model(caller);

assert(!player.is_zero(), 'Player does not exist');
assert(player.in_game, 'Player not in active game');

let player_system_dispatcher = self.player_system_dispatcher();

player_system_dispatcher.take_damage(get_caller_address(), amount.try_into().unwrap());
assert(amount <= Bounded::MAX, 'Damage too large');

let damage_amount: u16 = amount.try_into().unwrap();
player_system_dispatcher.take_damage(caller, damage_amount);

// Check if died and cleanup game session
if !player_system_dispatcher.is_alive(caller) {
let mut dead_player = player;
dead_player.in_game = false;
world.write_model(@dead_player);
}
}

fn attack_enemy(ref self: ContractState, enemy_id: u64, damage: u32) {
let caller = get_caller_address();
let world = self.world_default();
let player: Player = world.read_model(caller);
assert(!player.is_zero(), 'Player does not exist');
assert(player.in_game, 'Player not in active game');

let player_system_dispatcher = self.player_system_dispatcher();
assert(player_system_dispatcher.is_alive(caller), 'Player not alive');

// Instantiate the Store to interact with models
let mut store: Store = StoreImpl::new(world);
Expand Down Expand Up @@ -173,9 +206,20 @@ pub mod brawl_game {

fn get_player_status(ref self: ContractState) -> PlayerStatus {
let caller = get_caller_address();
let world = self.world_default();
let player: Player = world.read_model(caller);

if player.is_zero() {
return PlayerStatus::Waiting;
}

let player: Player = world.read_model(caller);
let player_system_dispatcher = self.player_system_dispatcher();
let alive = player_system_dispatcher.is_alive(caller);

if player_system_dispatcher.is_alive(caller) {
if player.in_game {
PlayerStatus::InGame
} else if alive {
PlayerStatus::Alive
} else {
PlayerStatus::Dead
Expand All @@ -189,6 +233,7 @@ pub mod brawl_game {
// Validate player exists
let player: Player = world.read_model(caller);
assert(!player.is_zero(), 'Player does not exist');
assert(player.in_game, 'Player not in active game');

// Validate player is alive
let player_system_dispatcher = self.player_system_dispatcher();
Expand Down Expand Up @@ -362,7 +407,7 @@ pub mod brawl_game {
}

fn apply_ability_effect(
self: @ContractState,
ref self: ContractState,
effect_type: AbilityEffectType,
effect_amount: u32,
target: ContractAddress,
Expand All @@ -378,6 +423,14 @@ pub mod brawl_game {
AbilityEffectType::Damage => {
let damage_amount: u16 = effect_amount.try_into().unwrap();
player_system_dispatcher.take_damage(target, damage_amount);

// Check if died and cleanup game session
if !player_system_dispatcher.is_alive(target) {
let mut world = self.world_default();
let mut target_player: Player = world.read_model(target);
target_player.in_game = false;
world.write_model(@target_player);
}
},
AbilityEffectType::Heal => {
let heal_amount: u16 = effect_amount.try_into().unwrap();
Expand All @@ -394,6 +447,14 @@ pub mod brawl_game {
AbilityEffectType::DamageOverTime => {
let damage_amount: u16 = effect_amount.try_into().unwrap();
player_system_dispatcher.take_damage(target, damage_amount);

// Check if died and cleanup game session
if !player_system_dispatcher.is_alive(target) {
let mut world = self.world_default();
let mut target_player: Player = world.read_model(target);
target_player.in_game = false;
world.write_model(@target_player);
}
},
}
}
Expand Down
Loading