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

AI code refactoring #8986

Merged
merged 2 commits into from
Jul 24, 2024
Merged

Conversation

oleg-derevenetz
Copy link
Collaborator

@oleg-derevenetz oleg-derevenetz commented Jul 23, 2024

This PR contains no functional changes, just AI code simplification and refactoring.

In the master branch, for historical reasons, the AI code is organized in such a way that there is a rudimentary support for different AI implementations. I believe it looks redundant at the moment. All we have for now is a single AI implementation called "normal" and it does absolutely everything, therefore, in my opinion, there is no need for an extra layer, which just contains a bunch of methods that do literally nothing.

With this PR:

  • There is no longer a bunch of code that was made "for the future", but now just doesn't do anything useful;
  • There are no more virtual methods (AI::Base -> AI::Normal) which do nothing useful but prevent the compiler from performing optimizations (e.g. code inlining during LTO);
  • The local code has been moved to an anonymous namespace as much as possible;
  • The AI battle planner code is now independent from the general AI code and vice versa, so they can be used (and are used) independently of each other.
  • Code structure is improved in general. The AI code is more clearly divided into separate independent modules, no more (well, almost) "god headers", which include a lot of unrelated things that are then defined in different CPP files.

If in the future we suddenly still need to support various AI engines, it will be possible to make corresponding changes. But for now, the current AI code architecture looks like a bit of overengineering, hence this PR.

@oleg-derevenetz oleg-derevenetz marked this pull request as draft July 23, 2024 16:27
@oleg-derevenetz oleg-derevenetz added improvement New feature, request or improvement AI Artificial intelligence behaviour labels Jul 23, 2024
@oleg-derevenetz oleg-derevenetz added this to the 1.1.2 milestone Jul 23, 2024
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/ai_common.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_common.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_battle.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_battle.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_battle.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_battle.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_battle.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_battle.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_planner_kingdom.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_battle_spell.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 (2/2)

src/fheroes2/ai/ai_battle_spell.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_battle_spell.cpp Show resolved Hide resolved
src/fheroes2/ai/ai_battle_spell.cpp Show resolved Hide resolved
@oleg-derevenetz oleg-derevenetz marked this pull request as ready for review July 23, 2024 21:02
@ihhub ihhub requested a review from idshibanov July 24, 2024 01:17
Copy link
Collaborator

@idshibanov idshibanov left a comment

Choose a reason for hiding this comment

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

I have no objections about this. 4 years ago I hoped for a competing AI to pop up but looks like it's never going to happen.

@oleg-derevenetz
Copy link
Collaborator Author

looks like it's never going to happen

Never say never :) However, I think that if such a need arises, some general AI interface will be created once again according to real needs at that moment.

@ihhub ihhub merged commit 0c44fd0 into ihhub:master Jul 24, 2024
20 checks passed
@ihhub
Copy link
Owner

ihhub commented Jul 24, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Artificial intelligence behaviour improvement New feature, request or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants