Battle fixes #949
Battle fixes #949
Conversation
credence0x
commented
Jun 17, 2024
credence0x
commented
- force remove enemy from current battle if they lost. This prevents any error that doesnt allow a structure to be pillaged because their resources are locked in battle. even though theyve lost
- remove "send home after pillage" feature
- remove "send home after pillage"
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
![mentatbot[bot]](https://avatars.githubusercontent.com/in/837875?s=60&v=4){kind=small}
There was a problem hiding this comment.
The pull request introduces some good refactoring and attempts to streamline the battle logic. However, there are concerns about the conditions under which armies are allowed to leave battles and the handling of army states. It's crucial to ensure that these changes do not introduce new bugs or allow for unintended behaviors in the combat system.
Butler is in closed beta. Reply with feedback or to ask Butler to review other parts of the PR. Please give feedback with emoji reacts.
| @@ -819,26 +767,21 @@ mod combat_systems { | |||
| claimer_army_position.assert_same_location(structure_position.into()); | |||
|
|
|||
| // ensure structure has no army protecting it | |||
| // or it has lost the battle it is currently in | |||
| let structure_army_id: u128 = get!(world, structure_id, Protector).army_id; | |||
There was a problem hiding this comment.
It seems that the leave_battle function is being called without checking if the army has actually won the battle. This could potentially allow armies to leave battles prematurely. Consider adding a check to ensure that the army has won before allowing it to leave.
| @@ -876,39 +819,17 @@ mod combat_systems { | |||
|
|
|||
| let mut structure_army: Army = Default::default(); | |||
| let mut structure_army_health: Health = Default::default(); | |||
| let mut can_pillage_only_once = false; | |||
| if structure_army_id.is_non_zero() { | |||
There was a problem hiding this comment.
The logic to check if the structure's army is dead seems to be outside of the condition checking if the army is in battle. This could lead to incorrect behavior if the army is not in battle but is somehow still considered dead. It might be safer to include this check inside the if structure_army.is_in_battle() condition to ensure consistency.
| @@ -819,26 +767,21 @@ mod combat_systems { | |||
| claimer_army_position.assert_same_location(structure_position.into()); | |||
|
|
|||
| // ensure structure has no army protecting it | |||
| // or it has lost the battle it is currently in | |||
| let structure_army_id: u128 = get!(world, structure_id, Protector).army_id; | |||
There was a problem hiding this comment.
The refactoring of battle and army updates into the update_battle_and_army function is a good improvement for modularity. However, ensure that all necessary state changes are being committed back to the world state to avoid any inconsistencies.
