-
Notifications
You must be signed in to change notification settings - Fork 37
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.
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
Fix/balance #1599
Fix/balance #1599
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Preparing PR description... |
Preparing review... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request introduces a new TravelStaminaCostConfig model and related functions to make travel costs configurable. It removes hardcoded travel types and costs in favor of this more flexible system. The changes are well-implemented and improve the system's adaptability.
However, there are significant balance changes, particularly in resource production rates and troop costs. These changes should be carefully reviewed to ensure they align with the intended game design and don't negatively impact player experience.
The code changes look solid, but I recommend adding more comments explaining the rationale behind the balance adjustments. Additionally, consider adding specific tests for the new travel stamina cost config functionality to ensure it works as expected in various scenarios.
Thanks for using MentatBot. Give comments a 👍 or 👎 to help me improve!
@@ -31,23 +31,6 @@ const RESOURCE_PRECISION: u128 = 1_000; | |||
// TODO: Move to Onchain config |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider moving MAX_PILLAGE_TRIAL_COUNT to on-chain config for easier adjustments.
@@ -314,3 +297,8 @@ | |||
const DEFAULT: u8 = 0; | |||
const ARMIES: u8 = 1; | |||
} | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good addition of TravelTypes enum. This makes the code more maintainable and less error-prone.
@@ -126,6 +126,16 @@ impl CapacityConfigCustomImpl of CapacityConfigCustomTrait { | |||
} | |||
} | |||
|
|||
#[derive(Copy, Drop, Serde)] | |||
#[dojo::model] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new TravelStaminaCostConfig model looks good. It allows for flexible configuration of travel costs.
@@ -17,12 +17,11 @@ pub struct Stamina { | |||
|
|||
#[generate_trait] | |||
impl StaminaCustomImpl of StaminaCustomTrait { | |||
fn handle_stamina_costs(army_entity_id: ID, travel: TravelTypes, world: IWorldDispatcher) { | |||
fn handle_stamina_costs(army_entity_id: ID, stamina_cost: u16, world: IWorldDispatcher) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The updated handle_stamina_costs function now uses the configurable stamina cost. This is a good improvement for flexibility.
@@ -95,6 +95,11 @@ trait IProductionConfig { | |||
fn set_production_config(ref world: IWorldDispatcher, resource_type: u8, amount: u128, cost: Span<(u8, u128)>); | |||
} | |||
|
|||
#[dojo::interface] | |||
trait ITravelStaminaCostConfig { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The new ITravelStaminaCostConfig trait and its implementation allow for easy configuration of travel stamina costs.
@@ -97,7 +97,8 @@ mod map_systems { | |||
// ensure unit is not in transit | |||
get!(world, unit_id, ArrivalTime).assert_not_travelling(); | |||
|
|||
StaminaCustomImpl::handle_stamina_costs(unit_id, TravelTypes::Explore, world); | |||
let stamina_cost = get!(world, (WORLD_CONFIG_ID, TravelTypes::EXPLORE), TravelStaminaCostConfig).cost; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good use of the new TravelStaminaCostConfig for exploration stamina cost.
@@ -100,7 +100,10 @@ mod travel_systems { | |||
let travelling_entity_coord: Coord = travelling_entity_position.into(); | |||
|
|||
let num_moves = directions.len().try_into().unwrap(); | |||
StaminaCustomImpl::handle_stamina_costs(travelling_entity_id, TravelTypes::Travel(num_moves), world); | |||
let mut stamina_cost = get!(world, (WORLD_CONFIG_ID, TravelTypes::TRAVEL), TravelStaminaCostConfig).cost; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The travel stamina cost calculation now uses the configurable cost. This is more flexible than the previous hardcoded values.
@@ -3,23 +3,14 @@ import { CapacityConfigCategory } from "./structures"; | |||
|
|||
export const EternumGlobalConfig = { | |||
stamina: { | |||
travelCost: 5, | |||
exploreCost: 15, | |||
travelCost: 10, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The stamina costs have been increased. Make sure this change is intentional and balanced.
[ResourcesIds.Wheat]: 300, | ||
[ResourcesIds.Fish]: 300, | ||
[ResourcesIds.Earthenshard]: 100, | ||
[ResourcesIds.Wood]: 10, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resource outputs have been significantly reduced. Ensure this is intentional and doesn't negatively impact game balance.
{ resource: ResourcesIds.Wheat, amount: 0.001 }, | ||
{ resource: ResourcesIds.Fish, amount: 0.002 }, | ||
{ resource: ResourcesIds.Wheat, amount: 0.01 }, | ||
{ resource: ResourcesIds.Fish, amount: 0.02 }, | ||
{ resource: ResourcesIds.Diamonds, amount: 0.001 }, | ||
], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Troop costs have been increased. This will slow down army production. Make sure this aligns with the intended game balance.
No description provided.