LoneGazebo / Community-Patch-DLL

Community Patch for Civilization V - Brave New World
Other
286 stars 158 forks source link

Potential Bugs/Issues in CvDiplomacyAI Code #5245

Closed RecursiveStar closed 5 years ago

RecursiveStar commented 5 years ago

_1. Mod version (i.e Date - 4/23): 3-14 Beta

_2. Mod list (if using Vox Populi only, leave blank):

_3. Error description: While reading through CvDiplomacyAI as part of a forum post I plan on making soon, I discovered what appear to be bugs or unintended issues in the code.

_4. Steps to reproduce (optional): N/A


Supporting information: Please note that you can attach .zip files by dragging-and-dropping them. If possible, zip up all supporting data and post that way.

Issue No. 1 When factoring in the result of GetVictoryBlockLevel (see lines 3744 and 3747), there are two instances when the FRIENDLY personality weight is added to the NEUTRAL approach instead of the (presumably intended) NEUTRAL personality weight.

viApproachWeights[MAJOR_CIV_APPROACH_NEUTRAL] += viApproachWeightsPersonality[MAJOR_CIV_APPROACH_FRIENDLY]

This issue also occurs in the exact same places for GetWonderDisputeLevel, GetVictoryDisputeLevel, and GetLandDisputeLevel, which are checked immediately below.

Issue No. 2 Beginning at line 4136, the part of the function involving vassal treatment is still using hardcoded values. As everything else in the function (including the AI's factoring in of its opinion towards a master's vassals, immediately above) is using personality-weighted values and your opinion of hardcoded diplomacy values has been previously expressed as "steaming garbage", I find this a little odd. :b

Probably not a big issue, but I thought I may as well point it out.

Relevant section starts with: switch(GetVassalTreatmentLevel(ePlayer))

Issue No. 3 Line 4281, the function deciding whether to add a bonus for dogpiling, seems to have some issues:

As I understand the function, it works as follows:

  1. Look at the other major civ and see if their projection of the current war against the player we're looking at is GOOD or better. If so, bThinkingAboutDogpiling = True;

  2. Get our victory dispute level against the player we're looking at. If it's STRONG or higher, bThinkingAboutDogpiling = True;

  3. Get our victory block level against the player we're looking at. If it's STRONG or higher, bThinkingAboutDogpiling = True;

  4. If the target value of the other major civ (not the player we're evaluating), is BAD or worse, bThinkingAboutDogpiling = False;

If bThinkingAboutDogpiling is currently set to true, iBonus += (int)eBlockLevel + (int)eDisputeLevel + (int)eWarProjection + GetPlayerTargetValue(eLoopPlayer)

Potential issues with this function:

  1. Why is the target value of the loop player factored in, rather than the one the AI is thinking of dogpiling on (the victim)? As the function is written, the fourth check seems to nullify the effects of a strong victory dispute and/or a strong victory block and/or the loop player's good war projection if the loop player (not the victim) is a bad war target. And why should the loop player's target value be added to the bonus value?

  2. Why is the other warring player's war projection used rather than this AI's war projection towards the victim? Let's say Greece is thinking of dogpiling on Indonesia, which is also at war with the devastating Brazilian empire. Just because Brazil's war is going well doesn't mean that Greece wouldn't be destroyed if they went to war with Indonesia.

  3. The AI accessing the other player's war projection is an information cheat that isn't really necessary - and would it work in the case of a human? Why not simply replace it with the war projection towards the victim?

  4. GetPlayerTargetValue(eLoopPlayer) doesn't have (int) written before it.

  5. Suggestion: Perhaps disincentivize dogpiling if the war projection of this AI against the loop player is a DEFEAT or worse (especially if the loop player doesn't like this AI)? The logic being, the loop player is at war with the dogpiling victim, and probably want some spoils from that war. If we dogpile on this player and capture some cities, the other player will probably hate us next, and if they're powerful enough to defeat us we wouldn't want that, especially after just going to war.

Issue No. 4 Line 4386 reads: if(GET_TEAM(GET_PLAYER(ePlayer).getTeam()).IsHasDefensivePact(GET_PLAYER(ePlayer).getTeam()))

And if it's True, it adds the FRIENDLY bias. It looks like the intention is to make the AI more friendly if they have a Defensive Pact with the other player's team, but what it seems to do in the code is check if the other player's team has a DP with themselves.

Issue No. 5 The check on line 4430 seems to be useless, as all possibilities result in setting the WAR approach score to 0. Minor performance improvement opportunity?

Issue No. 6 The check at line 4600 seems to destroy all weight to go to war or be hostile if Domination Victory is not enabled? I suspect I'm misinterpreting this and it actually means "are declarations of war enabled", but if I'm not, the problems with this should be obvious...Domination Victory isn't the only reason to go to war.

Issue No. 7 Lines 32251 and 32276 have inaccurate comments (victory dispute and victory block aren't minor civ dispute issues!).

Issue No. 8 On lines 32551 and 32566, there is a typo affecting the ideology opinion modifier calculations.

(GC.getGame().getCurrentEra())->getDiplpEmphasisLatePolicies()

"Diplp" rather than "Diplo".


Hopefully this is of use to you!

LoneGazebo commented 5 years ago
  1. Def typo, thanks. Minor impact, but still useful to fix.
  2. I was hoping putmalk might come back. He probably won't. I guess I'll fix.
  3. It's not technically incorrect, but I can see swapping out GetPlayerTargetValue for GetWarProjection in the 4th slot
  4. Typo
  5. I left it that way in case we ever change the code. Not a performance concern.
  6. Typo, should be /=2
  7. Lol
  8. Vanilla, doesn't affect anything
LoneGazebo commented 5 years ago

Thanks!

RecursiveStar commented 5 years ago

You're welcome! I see the issues are fixed in the latest beta.

The bonus value for dogpiling appears to still use the target value; intentional?

886094b5311ee8097b30b68149a7f7a1

I also have one additional question about this. Are these checks, occurring immediately after the vassal treatment level check, in use?

514c5cfac31fbf69bc535b360e3ff6b8

If so, they seem to use hardcoded values as well, but it's unclear as to whether they are used or not.

Glad I was of service :)

LoneGazebo commented 5 years ago

Yes it's used, but it's also marginal. As I said I try to respect the decisions putmalk et. al. make with their projects.

RecursiveStar commented 5 years ago

Ah, gotcha. Was just wondering as I plan on writing it up in a forum post like I did last year. :P

Is the bonus still using the target value intentional?