End game buggy supreme leader calculation

[scient]

Here is another annoying buggy function that I haven't fully figured out the best way to fix. There are a collection of functions used to determine if end game is nearing. This particular one (yes that's the original function name) looks to be around a faction closing in on enough votes to trigger Supreme Leader win state. Here is original code below, minus red which is my addition.

uint32_t __cdecl aah_ooga(int factionID, int allyFactionID) {
   if (!(*GameRules & VICTORY_PEACE_IN_OUR_TIME)) {
      return 0; // Diplomatic Victory not allowed
   }
   uint32_t votesTotal = 0;
   for (uint32_t i = 1; i < MaxPlayerNum; i++) {
      votesTotal += council_votes(i);
   }
   uint32_t factionIDRet = 0;
   for (int i = 1; i < MaxPlayerNum; i++) {
      if (i != allyFactionID && (!allyFactionID || (allyFactionID >= 0 &&
         !(PlayersData.diploStatus[allyFactionID] & DSTATUS_PACT)) || !(*GameRules & VICTORY_ONE_FOR_ALL))) {
         uint32_t proposalPreq = Proposal[1].preqTech; // Unite Behind Me As Supreme Leader
         if (has_tech(proposalPreq, factionID)
            || (proposalPreq >= 0 && (has_tech(Technology[proposalPreq].preqTech1, factionID)
               || has_tech(Technology[proposalPreq].preqTech2, factionID)))) {
            if (council_votes(i) * 2 >= votesTotal && (!factionIDRet || i == factionID)) {
               factionIDRet = i;
            }
         }

      }
   }
   return factionIDRet;
}

There are calls to this from various game locations. Of the functions, 3/8 pass -1 as allyFactionID parameter: climatic_battle, great_satan and enemy_diplomacy. The way the code works is it uses this parameter as offset to access diploStatus. However, when it's -1 it is actually incorrectly checking Pact status against tutorialMoreBases player_data field. So this can throw off return value. To make matters more interesting, climatic_battle() is the one function I've fully decompiled on list. It runs through check if a faction is nearing Corner Global Energy Market as well as Transcendence victory. Then it has this check.

Code: [Select]

if (aah_ooga(0, -1)) {
return true;
}
Even with correcting player_data bounds issue, this does absolutely nothing. It will always return false since native life faction is 0. So it never takes into account if faction is nearing Supreme leader victory. It might be redundant since a lot of functions that call climatic_battle() also call aah_ooga() directly. Most of the time, they pass same factionID for both parameters. But there are instances where tech research/goals and ranking would be affected. Will have to dig into this some more but figured I would open this up to discussion.

[scient]

It could be that this check as part of climatic_battle() was intentionally disabled. Not sure why, but maybe once I have more of the affected functions decompiled a clear picture will be present. The -1 bounds tutorialMoreBases issue is definitely a bug since that is generating faulty logic around the pact check. It might have been oversight about what parameter to pass to disable pact check (0 vs -1).

[bvanevery]

Are you sure fixing this is is a benefit?  Maybe the test was suppressed, because human player would near Supreme Leader threshold, and then all the AIs would turn uppity and refusenik against the human?  Which might defeat the game mechanical purpose of pursuing a peaceful way to win the game.  If you got forced into war and then had to start doing a bunch of Conquest stuff, that wouldn't necessarily be fun.  I'm only speculating, but sometimes magic numbers and dead code branches aren't bugs, they're deliberate design decisions that arise in playtesting.  They're not clean or elegant ways to solve problems, but with complex systems and production realities, one may not have much choice.

I'm quite sure the HSA "factions will beeline for this" stuff, arose on that basis.  It doesn't smell like clean premeditated design at all, but rather a production expedient.

[scient]

Yeah the more I think about it, the more I come to the same conclusion.  This could easily have been a design decision. Definitely something I'll revisit down the road. I'm going to correct the bounds issue that I'm 100% certain is a bug but leave alone climatic_battle() logic that essentially does nothing.