Skip to content

[Feat]:: core.cairo optimize and remove all warnings from code base #157

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 1 commit into from
Sep 12, 2025
Merged

[Feat]:: core.cairo optimize and remove all warnings from code base #157

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
3 changes: 1 addition & 2 deletions src/gear/GearActions.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,9 @@
use starknet::{ContractAddress, get_caller_address};
use core::num::traits::Zero;
use crate::models::gear::{Gear};
use crate::models::player::{Player, Body, PlayerTrait, Errors};
use crate::models::player::{Player, Body, Errors};
use crate::models::core::Contract;
use crate::erc1155::erc1155::{IERC1155Dispatcher, IERC1155DispatcherTrait};
use crate::helpers::gear::{get_high};
use crate::helpers::body::BodyTrait;
use dojo::event::EventStorage;
use dojo::model::{ModelStorage, Model};
Expand Down
2 changes: 1 addition & 1 deletion src/helpers/body.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use crate::models::player::Body;
use crate::models::gear::GearType;
use crate::helpers::gear::{parse_id, count_gear_in_array, contains_gear_type, get_high};
use crate::models::player::{DEFAULT_MAX_EQUIPPABLE_SLOT, Errors};
use crate::models::player::Errors;
use core::num::traits::Zero;

Comment on lines +4 to 6
Copy link
Contributor

@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.

⚠️ Potential issue

Missing ArrayTrait import for array methods.

This module calls .len(), .at(), .append() extensively; bring the trait into scope.

 use crate::models::player::Errors;
 use core::num::traits::Zero;
+use core::array::ArrayTrait;
📝 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
use crate::models::player::Errors;
use core::num::traits::Zero;
use crate::models::player::Errors;
use core::num::traits::Zero;
use core::array::ArrayTrait;
🤖 Prompt for AI Agents 
In src/helpers/body.cairo around lines 4 to 6, the module uses array methods
like .len(), .at(), and .append() but does not import the ArrayTrait; bring the
trait into scope by adding a use for ArrayTrait (for example: use
array::ArrayTrait;) at the top of the file alongside the other use statements so
those array extension methods resolve correctly, and remove or adjust any
imports if they become unused.


Expand Down
2 changes: 1 addition & 1 deletion src/helpers/gear.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
use crate::models::gear::{GearType, GearDetails, GearDetailsImpl};
use origami_random::dice::{Dice, DiceTrait};
use origami_random::dice::DiceTrait;
Copy link
Contributor

@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.

⚠️ Potential issue

Fix Dice construction and bring required traits into scope.

Calling DiceTrait::new without the Dice type import is brittle and may fail resolution; also .len()/.at()/.append() and unwrap_or need their extension traits in scope.

Apply:

- use origami_random::dice::DiceTrait;
+ use origami_random::dice::{Dice, DiceTrait};
+ use core::array::ArrayTrait;
+ use core::option::OptionTrait;

And switch to constructing the concrete type and bounding the roll:

-    let mut dice = DiceTrait::new(17, 'SEED');
-    let idx = dice.roll();
+    let mut dice = Dice::new(17, 'SEED');
+    let idx = dice.roll() % 18;
-    let mut dice = DiceTrait::new(90, 'DAMAGE_SEED');
+    let mut dice = Dice::new(90, 'DAMAGE_SEED');

Also applies to: 99-101, 183-185


// Helper function to calculate upgrade multipliers based on level
pub fn calculate_level_multiplier(level: u64) -> u64 {
Expand Down
1 change: 0 additions & 1 deletion src/market/marketplace.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ pub mod MarketplaceActions {
use starknet::{ContractAddress, get_caller_address, get_block_timestamp, get_contract_address};
use dojo::model::ModelStorage;
use dojo::event::EventStorage;
use core::num::traits::Zero;


use crate::models::marketplace::{
Expand Down
8 changes: 4 additions & 4 deletions src/models/gear.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,10 @@

use crate::helpers::base::ContractAddressDefault;
use core::num::traits::Zero;
use openzeppelin::token::erc1155::interface::{IERC1155Dispatcher, IERC1155DispatcherTrait};
use crate::erc1155::erc1155::{IERC1155MintableDispatcher, IERC1155MintableDispatcherTrait};
// use openzeppelin::token::erc1155::interface::{IERC1155Dispatcher, IERC1155DispatcherTrait};
// use crate::erc1155::erc1155::{IERC1155MintableDispatcher, IERC1155MintableDispatcherTrait};
use starknet::ContractAddress;
use dojo::world::WorldStorage;
// use dojo::world::WorldStorage;
use core::traits::{Into, TryInto};

#[dojo::model]
Expand Down Expand Up @@ -517,7 +517,7 @@ impl OptionArrayTupleImpl of Clone<Option<Array<(u256, u256)>>> {
// Tests for GearDetails struct
#[cfg(test)]
mod tests {
use super::{GearDetails, GearDetailsImpl, GearType};
use super::{GearDetailsImpl, GearType};

#[test]
fn test_valid_gear_details() {
Expand Down
4 changes: 2 additions & 2 deletions src/systems/armour.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,8 @@ pub trait IArmour<TContractState> {

#[dojo::contract]
pub mod ArmourActions {
use starknet::{ContractAddress, get_caller_address};
use crate::models::armour::{Armour, ArmourTrait};
use starknet::ContractAddress;
use crate::models::armour::Armour;
use super::IArmour;

fn dojo_init(ref self: ContractState, admin: ContractAddress) {}
Expand Down
160 changes: 86 additions & 74 deletions src/systems/core.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ pub mod CoreActions {
use dojo::world::WorldStorage;
use coa::helpers::gear::{parse_id, random_geartype, get_max_upgrade_level, get_min_xp_needed};
use coa::models::player::{Player, PlayerTrait};
use core::num::traits::Zero;
use core::traits::Into;

const GEAR: felt252 = 'GEAR';
Expand Down Expand Up @@ -79,22 +78,31 @@ pub mod CoreActions {
let caller = get_caller_address();
let mut world = self.world_default();
let contract: Contract = world.read_model(COA_CONTRACTS);

assert(caller == contract.admin, 'Only admin can spawn items');

let erc1155_dispatcher = IERC1155MintableDispatcher {
contract_address: contract.erc1155,
};

let mut items = array![];
let gear_len = gear_details.len();

let mut i = 0;
loop {
if i >= gear_len {
break;
}

while i < gear_details.len() {
let details = *gear_details.at(i);

assert(details.validate(), 'Invalid gear details');

let item_id: u256 = self.generate_incremental_ids(details.gear_type.into());
// Generate ID once and reuse
let item_id = self.generate_incremental_ids(details.gear_type.into());
let item_type: felt252 = details.gear_type.into();

// Create gear struct with computed values
let mut gear = Gear {
id: item_id,
item_type,
Expand All @@ -106,21 +114,19 @@ pub mod CoreActions {
owner: contract_address_const::<0>(),
max_upgrade_level: details.max_upgrade_level,
min_xp_needed: details.min_xp_needed,
spawned: false,
spawned: true,
};

assert(!gear.spawned, 'Gear already spawned');
gear.spawned = true;
world.write_model(@gear);
items.append(item_id);

items.append(gear.id);
erc1155_dispatcher
.mint(contract.warehouse, gear.id, details.total_count.into(), array![].span());
.mint(contract.warehouse, item_id, details.total_count.into(), array![].span());

i += 1;
};

let event = GearSpawned { admin: caller, items };
world.emit_event(@event);
world.emit_event(@GearSpawned { admin: caller, items });
}

// move to market only items that have been spawned.
Expand All @@ -144,11 +150,13 @@ pub mod CoreActions {

let gear_type = random_geartype();
let item_type: felt252 = gear_type.into();
let item_id: u256 = self.generate_incremental_ids(gear_type.into());
let max_upgrade_level: u64 = get_max_upgrade_level(gear_type);
let min_xp_needed: u256 = get_min_xp_needed(gear_type);
let item_id = self.generate_incremental_ids(item_type.into());

let (max_upgrade_level, min_xp_needed) = (
get_max_upgrade_level(gear_type), get_min_xp_needed(gear_type),
);
Comment on lines +153 to +157
Copy link
Contributor

@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

Confirm Into mapping for ID generation is keyed by type “high”

generate_incremental_ids indexes GearCounter by item_id.high. If felt252→u256 maps into low (common), counters collapse under high=0. Ensure GearType/ felt252 Into sets the gear type in high.

🏁 Script executed:

#!/bin/bash
# Verify Into<u256> implementations and where gear type is placed.
rg -nP --type=cairo -C2 'impl\s+Into\s*<\s*u256\s*>\s+for\s+(GearType|felt252)' 
rg -nP --type=cairo -C2 'struct\s+u256\s*\{'
rg -nP --type=cairo -C3 'GearCounter'

Length of output: 89

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "=== .cairo files ==="
rg --files -g '**/*.cairo' || true

echo
echo "=== Search Into<u256> implementations in .cairo files ==="
rg -n -C2 -g '**/*.cairo' 'Into\s*<\s*u256\s*>\s*for' || true
rg -n -C2 -g '**/*.cairo' 'Into\s*<\s*u256\s*>' || true
rg -n -C2 -g '**/*.cairo' 'impl\s+Into' || true

echo
echo "=== Search for u256 / Uint256 definitions ==="
rg -n -C2 -g '**/*.cairo' 'struct\s+u256\b|type\s+u256\b|struct\s+Uint256\b|type\s+Uint256' || true

echo
echo "=== Search GearCounter, GearType, generate_incremental_ids, felt252 ==="
rg -n -C3 -g '**/*.cairo' 'GearCounter' || true
rg -n -C3 -g '**/*.cairo' 'generate_incremental_ids' || true
rg -n -C3 -g '**/*.cairo' 'GearType' || true
rg -n -C3 -g '**/*.cairo' 'felt252' || true

echo
echo "=== Fallback: search entire repo (no -g) ==="
rg -n -C2 'Into\s*<\s*u256\s*>\s*for' || true
rg -n -C2 'struct\s+u256\b|type\s+u256\b|struct\s+Uint256\b|type\s+Uint256' || true
rg -n -C3 'GearCounter' || true
rg -n -C3 'generate_incremental_ids' || true
rg -n -C3 'GearType' || true
rg -n -C3 'felt252' || true

Length of output: 276689

Fix felt252→u256 conversion used for ID generation — ensure gear type is in u256.high

generate_incremental_ids keys GearCounter by item_id.high; models/gear.cairo implements Into<GearType, u256> as u256 { low: 0, high: } (src/models/gear.cairo:257–277). But the spawn path converts gear_type → felt252 then calls item_type.into() (src/systems/core.cairo:151–153), which will produce u256 with the value in low (high==0) and collapse counters. Replace the felt252→u256 conversion with a direct GearType→u256 (e.g. use gear_type.into()) or ensure the felt252→u256 conversion populates high. Locations: src/systems/core.cairo:101–104 and 151–153; src/models/gear.cairo:257–277.

🤖 Prompt for AI Agents 
In src/systems/core.cairo around lines 101–104 and 151–153 (and specifically
153–157), the code converts gear_type to felt252 then to u256 for item_id
generation which places the gear type in u256.low (high==0) and collapses
GearCounter keys; replace those felt252→u256 conversions with a direct
GearType→u256 conversion (e.g. use gear_type.into()) or otherwise construct the
u256 with the gear type in the high field so item_id.high contains the gear
type; update both spawn-path sites and any related calls so they pass a u256
where high == gear_type, matching the Into<GearType, u256> implementation in
src/models/gear.cairo (lines ~257–277).


let gear = Gear {
Gear {
id: item_id,
item_type,
asset_id: item_id,
Expand All @@ -160,87 +168,94 @@ pub mod CoreActions {
max_upgrade_level,
min_xp_needed,
spawned: false,
};

gear
}
}

//@ryzen-xp
fn pick_items(ref self: ContractState, item_ids: Array<u256>) -> Array<u256> {
let mut world = self.world_default();
let caller = get_caller_address();

// Cache contract + dispatcher
let contract: Contract = world.read_model(COA_CONTRACTS);
let mut player: Player = world.read_model(caller);
let erc1155 = IERC1155Dispatcher { contract_address: contract.erc1155 };

let mut player: Player = world.read_model(caller);
player.init('default');

let mut successfully_picked: Array<u256> = array![];

let mut successfully_picked = ArrayTrait::<u256>::new();
let mut gears_to_update = ArrayTrait::<Gear>::new();
let mut events_to_emit = ArrayTrait::<ItemPicked>::new();
let mut has_vehicle = player.has_vehicle_equipped();

let item_count = item_ids.len();
let mut i = 0;
while i < item_ids.len() {
let item_id = *item_ids.at(i);
let mut gear: Gear = world.read_model(item_id);

assert(gear.is_available_for_pickup(), 'Item not available');

if player.xp < gear.min_xp_needed {
i += 1;
continue;
loop {
if i >= item_count {
break;
}

let mut equipped = false;
let mut mint_item = false;
let item_id = *item_ids.at(i);
let mut gear: Gear = world.read_model(item_id);

if has_vehicle {
// if player has vehicle, mint all items directly to inventory !!!!!!!!!!!!
mint_item = true;
} else {
if player.is_equippable(item_id) {
// Early filter-out
if gear.is_available_for_pickup() && player.clone().xp >= gear.min_xp_needed {
let mut equipped = false;
let can_pick = if has_vehicle {
true
} else if player.is_equippable(item_id) {
PlayerTrait::equip(ref player, item_id);
equipped = true;
mint_item = true;
} else if player.has_free_inventory_slot() {
mint_item = true;
true
} else {
player.has_free_inventory_slot()
};

if can_pick {
// ERC1155 transfer
erc1155
.safe_transfer_from(
contract.warehouse,
caller,
item_id,
1,
ArrayTrait::<felt252>::new().span(),
);

// Defer gear update + event
gear.transfer_to(caller);
gears_to_update.append(gear);
successfully_picked.append(item_id);

events_to_emit
.append(
ItemPicked {
player_id: caller, item_id, equipped, via_vehicle: has_vehicle,
},
);

// If just equipped a vehicle, persist for next items
if equipped && parse_id(item_id) == GearType::Vehicle {
has_vehicle = true;
}
}
}

if mint_item {
// Transfer the pre-minted item from warehouse to the player

let erc1155 = IERC1155Dispatcher { contract_address: contract.erc1155 };
erc1155
.safe_transfer_from(
contract.warehouse, caller, item_id, 1, array![].span(),
);

gear.transfer_to(caller);
world.write_model(@gear);

// Add to successfully picked array
successfully_picked.append(item_id);

world
.emit_event(
@ItemPicked {
player_id: caller,
item_id: item_id,
equipped: equipped,
via_vehicle: has_vehicle,
},
);
}

i += 1;
};

// if we just equipped a vehicle, enable hands-free pickup
if equipped && parse_id(item_id) == GearType::Vehicle {
has_vehicle = true;
let updates_len = gears_to_update.len();
let mut j = 0;
loop {
if j >= updates_len {
break;
}
world.write_model(gears_to_update.at(j));
world.emit_event(events_to_emit.at(j));
j += 1;
};

// Update Player state
world.write_model(@player);

successfully_picked
Expand All @@ -257,15 +272,12 @@ pub mod CoreActions {
// Generates an incremental u256 ID based on gear_id.high.
fn generate_incremental_ids(ref self: ContractState, item_id: u256) -> u256 {
let mut world = self.world_default();

let mut gear_counter: GearCounter = world.read_model(item_id.high);

let data = GearCounter { id: item_id.high, counter: gear_counter.counter + 1 };

world.write_model(@data);
gear_counter.counter += 1;
world.write_model(@gear_counter);

let id = u256 { high: data.id, low: data.counter };
id
u256 { high: item_id.high, low: gear_counter.counter }
}
}
}
8 changes: 3 additions & 5 deletions src/systems/gear.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,7 +261,7 @@ pub mod GearActions {
self._assert_admin();

// Create a new gear instance based on the asset type
let new_gear_id = generate_id(GEAR, ref world);
let _new_gear_id = generate_id(GEAR, ref world);
// Implementation would create gear based on asset type
}

Expand Down Expand Up @@ -1072,8 +1072,6 @@ pub mod GearActions {
}

fn _get_ownership_status(self: @ContractState, gear: @Gear) -> OwnershipStatus {
let caller = get_caller_address();

OwnershipStatus {
is_owned: !gear.owner.is_zero(),
owner: *gear.owner,
Expand All @@ -1087,7 +1085,7 @@ pub mod GearActions {
fn _calculate_upgrade_preview(
self: @ContractState, gear: @Gear, target_level: u64,
) -> UpgradeInfo {
let gear_type = parse_id(*gear.asset_id);
let _gear_type = parse_id(*gear.asset_id);
let total_costs = self._calculate_total_upgrade_costs(gear, target_level);

// Create gear at target level for stats calculation
Expand Down Expand Up @@ -1778,7 +1776,7 @@ pub mod GearActions {

fn _initialize_gear_assets(ref self: ContractState, ref world: WorldStorage) {
// Weapons - using ERC1155 token IDs as primary keys
let weapon_1_id = u256 { low: 0x0001, high: 0x1 };
let _weapon_1_id = u256 { low: 0x0001, high: 0x1 };
// let weapon_1_gear = Gear {
// id: generate_id(GEAR, ref world), // WEAPON_1 from ERC1155
// item_type: 'WEAPON',
Expand Down
3 changes: 1 addition & 2 deletions src/systems/player.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ pub mod PlayerActions {
Player, PlayerTrait, DamageDealt, PlayerDamaged, FactionStats, PlayerInitialized,
CombatSessionStarted, BatchDamageProcessed, CombatSessionEnded, DamageAccumulator,
};
use crate::models::gear::{Gear, GearTrait, GearLevelStats, ItemRarity, GearType};
use crate::models::gear::{Gear, GearLevelStats, ItemRarity, GearType};
use crate::models::armour::{Armour, ArmourTrait};
use crate::erc1155::erc1155::{IERC1155Dispatcher, IERC1155DispatcherTrait};
use super::IPlayer;
Expand Down Expand Up @@ -846,7 +846,6 @@ pub mod PlayerActions {
}

fn calculate_gear_damage(self: @ContractState, gear: Gear) -> u256 {
let world = self.world_default();
let rarity = self.get_item_rarity(gear.asset_id);
let base_damage = get_base_damage_for_type(parse_id(gear.asset_id));

Expand Down
3 changes: 1 addition & 2 deletions src/systems/tournament.cairo
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ pub mod TournamentActions {
use starknet::{ContractAddress, get_caller_address, get_block_timestamp, get_contract_address};
use dojo::model::ModelStorage;
// use dojo::world::WorldStorage;
use dojo::world::IWorld;
use dojo::event::EventStorage;
use core::num::traits::Zero;
use core::num::traits::Pow;
Expand Down Expand Up @@ -326,7 +325,7 @@ pub mod TournamentActions {
assert(match_.match_id != 0, Errors::MATCH_NOT_FOUND);
assert(!match_.is_completed, Errors::MATCH_COMPLETED);

let loser_id = if winner_id == match_.player1 {
let _loser_id = if winner_id == match_.player1 {
match_.player2.unwrap()
} else {
match_.player1
Expand Down
Loading
Loading