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

Move survey_universe to empire module and call it earlier #3762

Merged
merged 2 commits into from Mar 26, 2022

Conversation

Grummel7
Copy link
Contributor

This should fix #3760 for good.

It also makes sense as a refactoring, since survey_universe mostly sets values in the empire module and was completely independent from the rest of ColonisationAI.

It does also set some value in the colonization module (nice mix of spelling ;-) which should probably be protected by locks as well, but this can be done with a later PR, this one is big enough and is needed as a fix.

@geoffthemedio geoffthemedio added category:bug The Issue/PR describes or solves a perceived malfunction within the game. component:AI The Issue/PR deals with the Python AI decision making code or affects it. labels Mar 25, 2022
@@ -343,7 +347,6 @@ def generateOrders(): # pylint: disable=invalid-name
debug("Calling AI Modules")
# call AI modules
action_list = [
ColonisationAI.survey_universe,
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just move aistate.prepare_for_new_turn() into the action list after ColonisationAI.survey_universe?

Most of the code before action_list is timers and debug prints, so I'd prefer survey universe calls after that.

Actually prepare_for_new_turn() is a number of small functions without arguments, so we could just "inline" it here and put all calls to the list directly. This will give us the flexibility to adjust orders.

The survey universe does a single cycle over everything and collects information.
Algorithmically doing this cycle twice does not look good. But this could simplify the logic. If we will have 2 or more cycles we could deal with temporal coupling in much easy way.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

survey_universe must be called before this line because get_capital needs the information.

I wonder whether we can get rid of survey_universe completely by doing the calculation in the cache_for_current_turn-functions, like it did it for _get_pilot_summary (see second commit). We wouldn't even need the lock anymore then, but of course we would have a dozen loops instead of one.

@Cjkjvfnby
Copy link
Contributor

@Grummel7
Copy link
Contributor Author

@Grummel7 , please apply isort to the code.

https://github.com/freeorion/freeorion/runs/5702180393?check_suite_focus=true

Done it. You can merge it now, I cannot do that...

@Cjkjvfnby Cjkjvfnby merged commit 068685a into freeorion:master Mar 26, 2022
@Vezzra Vezzra added this to the v0.5 milestone Mar 27, 2022
@Vezzra Vezzra added the status:merged All relevant commits of this PR were merged into the master development branch. label Mar 27, 2022
@Grummel7 Grummel7 deleted the survey_universe branch May 7, 2022 16:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:bug The Issue/PR describes or solves a perceived malfunction within the game. component:AI The Issue/PR deals with the Python AI decision making code or affects it. status:merged All relevant commits of this PR were merged into the master development branch.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

get_colony_builders, etc.
4 participants