fix pillage successful bug

[aymericdelab]

User description

small fix

---

PR Type

Bug fix

---

Description

• Fixed the logic to determine if a pillage was successful by checking if the winner is the attacker.
• Updated the formatPillageEvent function to correctly map the winner to the BattleSide enum.
• Added the BattleSide import to ensure proper type checking and usage.

---

Changes walkthrough 📝

	Relevant files
Bug fix	Battle.tsx Fix pillage success determination logic in Battle component client/src/ui/components/military/Battle.tsx Added BattleSide import from @bibliothecadao/eternum. Updated isPillageSucess function to check if the winner is the attacker. Modified formatPillageEvent function to map the winner to BattleSide. +4/-4

---

💡 PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
eternum	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Jun 21, 2024 1:49pm

[github-actions[bot]]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	2
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Logic Change: Ensure that the new logic in isPillageSuccess correctly identifies a successful pillage based solely on the winner being the attacker. This changes the behavior from also considering pillaged resources and destroyed buildings.
	Enum Mapping: Verify that the mapping of winner to BattleSide[winner] in formatPillageEvent correctly handles all possible enum values and does not result in undefined behavior or errors.

[github-actions[bot]]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Add a check for component mount status before state updates in asynchronous operations ___ **Consider adding a check to ensure the component is still mounted before updating the state in asynchronous operations to avoid potential memory leaks or React state update warnings.** [client/src/ui/components/military/Battle.tsx [77]](https://github.com/BibliothecaDAO/eternum/pull/978/files#diff-9f3726f9bbcb484108a0b6184660bcd0f0c5ffcdbffaf734ceb72ee17f95ed75R77-R77) ```diff -setPillageHistory((prev) => [newPillage, ...prev]); +if (isMounted.current) { + setPillageHistory((prev) => [newPillage, ...prev]); +} ``` Suggestion importance[1-10]: 9 Why: Adding a check for component mount status before state updates in asynchronous operations is a best practice that can prevent memory leaks and React state update warnings, which is crucial for maintaining application stability.	9
Possible bug	Improve the robustness of enum value comparison to handle undefined or null cases ___ **Replace the direct access of the BattleSide enum with a more robust check to handle potential undefined or null values. This will prevent runtime errors if the winner value does not match any enum keys.** [client/src/ui/components/military/Battle.tsx [92]](https://github.com/BibliothecaDAO/eternum/pull/978/files#diff-9f3726f9bbcb484108a0b6184660bcd0f0c5ffcdbffaf734ceb72ee17f95ed75R92-R92) ```diff -return history.winner === BattleSide[BattleSide.Attack]; +return BattleSide[BattleSide.Attack] ? history.winner === BattleSide[BattleSide.Attack] : false; ``` Suggestion importance[1-10]: 8 Why: This suggestion improves the robustness of the code by handling potential undefined or null values, which can prevent runtime errors. This is a significant improvement for reliability.	8
Enhancement	Improve variable naming for clarity and readability ___ **Use a more descriptive variable name instead of winner to indicate that it stores an index or key of BattleSide.** [client/src/ui/components/military/Battle.tsx [161]](https://github.com/BibliothecaDAO/eternum/pull/978/files#diff-9f3726f9bbcb484108a0b6184660bcd0f0c5ffcdbffaf734ceb72ee17f95ed75R161-R161) ```diff -const winner = Number(event.data[0]); +const battleSideIndex = Number(event.data[0]); ``` Suggestion importance[1-10]: 7 Why: Using a more descriptive variable name improves code readability and clarity, which is beneficial for maintainability and understanding the code, especially for new developers.	7
Maintainability	Remove unnecessary empty lines to improve code cleanliness ___ **Remove the unnecessary empty line to keep the code clean and maintainable.** [client/src/ui/components/military/Battle.tsx [162]](https://github.com/BibliothecaDAO/eternum/pull/978/files#diff-9f3726f9bbcb484108a0b6184660bcd0f0c5ffcdbffaf734ceb72ee17f95ed75R162-R162) ```diff const winner = Number(event.data[0]); - const pillagedResources: Resource[] = []; ``` Suggestion importance[1-10]: 5 Why: Removing unnecessary empty lines is a minor improvement that enhances code cleanliness and maintainability, but it does not significantly impact functionality or performance.	5

[aymericdelab]

fixes #964