Merged
Conversation
…value for whether they succeeded. And add a loop in PlayerAI so if a unit fails at its task, it gets a new one... but not infinitely. Currently, once an AI player has explored its entire landmass, it throws the warning about failing twice. This is because the AI gets the Explore task again by default, and is expected for now. Now that it's verified that is working correctly, I'll add logic to ensure units don't get Explorer tasks if there isn't anything left to explore. Also, the change in SettlerAI to improve logging is because the "could not get part of the path" showed up in the logs for me. Unfortunately I was in Run instead of Debug mode at the time. Looking forward to that proper logging framework!
Wrote a lot of comments about how I foresee things evolving in the other case. Should help in writing the actual code later.
…nits are done exploring. Could definitely be better, but I'm not trying to make the AI perfect in this commit, just slightly better.
…dy there. Also fixed the explorer AIs trying to go to coast tiles if they were on land, and thus not switching over to defender AIs once they ran out of tiles to explore.
pcen
approved these changes
Apr 9, 2022
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #138 .
Previously, the AI would assign units on a city that had no defenders to defend that city (usually, these were units the city built). But all other units would explore.
Now that exploration is not random, and tile visibility is tracked, the AI would eventually run out of tiles to explore, and rather quickly if it started on an island. Before this change, the units would just stay where they were when they stopped exploring.
Now, when these units become surplus explorers, they are assigned to defend the nearest city which is tied for having the fewest defenders of any city in the empire, strengthening defenses against barbarians. Newly spawned units in the post-exploration age also receive that task, which can result in new cities quickly getting their defenses up.
Structurally, a key change here is that the unitAI classes now always return whether they succeeded at their task. The new loop in PlayerAI allows units whose task cannot be fulfilled to be assigned a new AI goal, possibly of a different type.
One of my next restructuring goals will be the SetAIForUnit method on line 32 of PlayerAI.cs; I've added comments to that effect. Right now it's a set of conditionals, but as the possible uses of units increase, we'll need a better way to balance priorities. My current thinking is having some sort of arbiter system - each potential use of the unit can be scored, and the one with the highest score will be chosen, or be most likely to be chosen. But as there's already one refactored item here, that seems like a bit too much all at once.