Skip to content
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 the random generator usage by the non-Arena code #7967

Merged
merged 37 commits into from
Oct 21, 2023

Conversation

oleg-derevenetz
Copy link
Collaborator

@oleg-derevenetz oleg-derevenetz commented Oct 20, 2023

fix #7964

This PR prohibits the use of the Battle::Arena's random generator by external code (mostly by AI decision-making code), which was the main reason for the non-reproducibility by a human of results of battles performed by AI.

It also redesignes the Battle::Commands in order to be able to perform additional checks at compile time and introduces a few minor improvements.

@oleg-derevenetz oleg-derevenetz marked this pull request as draft October 20, 2023 01:45
@oleg-derevenetz oleg-derevenetz added bug Something doesn't work logic Things related to game logic labels Oct 20, 2023
@oleg-derevenetz oleg-derevenetz added this to the 1.0.10 milestone Oct 20, 2023
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ clang-tidy found issue(s) with the introduced code (1/2)

src/fheroes2/ai/normal/ai_normal_battle.cpp Outdated Show resolved Hide resolved
src/fheroes2/battle/battle_command.h Outdated Show resolved Hide resolved
src/fheroes2/battle/battle_catapult.cpp Outdated Show resolved Hide resolved
src/fheroes2/battle/battle_catapult.h Outdated Show resolved Hide resolved
src/fheroes2/battle/battle_catapult.h Outdated Show resolved Hide resolved
src/fheroes2/battle/battle_catapult.cpp Show resolved Hide resolved
src/fheroes2/battle/battle_main.cpp Outdated Show resolved Hide resolved
src/fheroes2/battle/battle_main.cpp Outdated Show resolved Hide resolved
src/fheroes2/battle/battle_main.cpp Outdated Show resolved Hide resolved
src/fheroes2/battle/battle_main.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ clang-tidy found issue(s) with the introduced code (2/2)

src/fheroes2/battle/battle_troop.cpp Show resolved Hide resolved
src/fheroes2/battle/battle_command.h Outdated Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

src/fheroes2/battle/battle_arena.cpp Show resolved Hide resolved
Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

⚠️ clang-tidy found issue(s) with the introduced code (1/1)

src/fheroes2/ai/normal/ai_normal_battle.cpp Show resolved Hide resolved
@oleg-derevenetz
Copy link
Collaborator Author

oleg-derevenetz commented Oct 20, 2023

Hi @fheroes2bugs

Regarding #7964: I believe that in this PR I have eliminated the main reasons for the discrepancies that led to a different outcome of the battle when performing the same actions by the AI and the human player. According to my tests, now the outcome of the battle should be the same if the AI and the human player perform exactly the same actions (by "exactly" I mean maximum accuracy - for example, if you cast a spell on a wide unit, that spell must be cast on exactly the same cell of that unit). I also checked the pseudo-random number chains during the AI-controlled battle and human-controlled battle (by performing the same actions) and I didn't see any differences with this PR. I hope that I have fixed all the causes of the former discrepancies.

Regarding #7965: according to my tests with this PR, the battle outcome is the same when performing absolutely the same actions in Debug and Release builds, if both Release and Debug builds were created from the same source code using the same compiler. Just in case, let me remind once again that the same battle outcome in case of using different compilers is not currently guaranteed. And of course, this is not guaranteed in case of using different versions of the game (built from different versions of the source code), because changes are regularly made to the logic of the combat AI.

@oleg-derevenetz oleg-derevenetz marked this pull request as ready for review October 20, 2023 23:44
Copy link
Owner

@ihhub ihhub left a comment

Choose a reason for hiding this comment

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

Hi @oleg-derevenetz , I left a set of comments and questions. Could you please take a look at them once you have free time?

src/fheroes2/battle/battle_action.cpp Show resolved Hide resolved
src/fheroes2/battle/battle_arena.h Outdated Show resolved Hide resolved
src/fheroes2/battle/battle_action.cpp Show resolved Hide resolved
src/fheroes2/battle/battle_command.h Show resolved Hide resolved
src/fheroes2/battle/battle_command.h Show resolved Hide resolved
@ihhub ihhub merged commit a147e60 into ihhub:master Oct 21, 2023
20 checks passed
@ihhub
Copy link
Owner

ihhub commented Oct 21, 2023

@oleg-derevenetz , thank you so much for these useful changes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something doesn't work logic Things related to game logic
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Battle: morale in autobattle and player battle don't work the same way
2 participants