Fix AI team recruitment inconsistency causing underfilled teams

[handama]

In certain situations, the AI incorrectly assumes that some units on the map are recruitable when forming a team, leading to teams that can never reach full strength. For example, when there are 3 GI on the map with recruitable flags (RecruitableA, RecruitableB) set to 1,1; 1,0; and 0,0, and an AI team with Autocreate enabled is created via trigger action 4 (Create Team), only the units with 1,1 should be considered recruitable, and the AI should produce 2 additional GIs to fill the team. However, during production planning (0x4DA230), the AI incorrectly treats units with 1,0 or 0,0 as recruitable and therefore only produces 1 GI. During actual recruitment (0x6EA610), these GIs are skipped due to stricter conditions, leaving the team permanently underfilled. Notably, if the intermediate GI (1,0) is destroyed, the AI will then correctly produce the missing GI and the team can finally form.

testmap.zip

This PR addresses it by modifying 0x4DA230 so that its conditions match those of 0x6EA610 as closely as possible to avoid such inconsistencies.

[phoboscn-bot]

To Chinese users:
This pull request has been mentioned on Phobos CN. There might be relevant details there:

致中文用户：
此拉取请求已在 Phobos CN 上被提及。那里可能有相关详细信息：

https://www.phoboscn.top/t/topic/358/11

[github-actions]

Nightly build for this pull request:

• compiled-dll-1f7d9ef65c8359dd1a88ec51fd8d9907ec3349ff.zip
  These artifacts will expire in 90 days and will not be available for download after that time.

This comment is automatic and is meant to allow guests to get latest nightly builds for this pull request without registering. It is updated on every successful build.

[Coronia]

as far as I know, if Recruitable A and B are both set to 0, the unit will not be considered as occupying a team member. AI will still skip it when recruiting a team and build sufficient units instead

Making it as 1,0 is indeed problematic but maybe a toggle is needed for this fix, since it'll alter the AI behaviors of existing missions, making their AI unintentionally stronger

[handama]

as far as I know, if Recruitable A and B are both set to 0, the unit will not be considered as occupying a team member. AI will still skip it when recruiting a team and build sufficient units instead

Making it as 1,0 is indeed problematic but maybe a toggle is needed for this fix, since it'll alter the AI behaviors of existing missions, making their AI unintentionally stronger

I understand the concern about potentially affecting AI strength, but I don’t think a toggle is necessary here. This change mainly fixes a case which is more of a bug than intended behavior. The situations where this occurs should also be relatively limited, so the overall impact on existing missions is likely small. What do you think?

[TaranDahl]

The code looks good to me.
I have a shallow understanding of the Team system. So I think I am not qualified for judging the design aspect.

[TaranDahl]

@Metadorius @Starkku @ZivDero can anyone help judging the design concept?

[DeathFishAtEase]

Tested by: https://www.phoboscn.top/t/topic/361/9

[TaranDahl]

Given that no one is willing to make further comments, this PR will be merged in 24 hours.

[DeathFishAtEase]

Strange, ef120cd is just a documentation change (resolve merge conflicts), but Nightly failed to build successfully. However, when I pull the branch of that PR locally for testing, it builds successfully.

error C1083: Cannot open include file: 'atlbase.h': No such file or directory [D:\a\Phobos\Phobos\Phobos.vcxproj]

[TaranDahl]

Could it possibly be this one that's causing the problem?
055cdc2

[DeathFishAtEase]

Could it possibly be this one that's causing the problem? 055cdc2

Not that one. I also suspected this, so I tested in another repository. The result was that whether or not Revert that commit, the build succeeded. I even reset the FreeWindor:develop branch of that repository to be exactly the same as Phobos-developers:develop (without Revert or Reapply) for testing, but the build also succeeded.

Additionally, that 055cdc2 had already been merged into the develop branch during the last synchronization (dea4751), and at that time it did not cause any problems.

After further testing, it was found that as long as the synchronization target of the branch where this PR is located is set to commit 0b21108 and later commits, this abnormal behavior will occur, but the cause is not clear.

[TaranDahl]

@Metadorius Help

[DeathFishAtEase]

The branch has been restored to the state before a series of tests were conducted, waiting for others to handle it.

[Starkku]

After review, investigation and some research it appears this PR addresses a real inconsistency and correctly fixes the problem - adjusting the recruitment-stage check to require both recruitment flags (which as an aside could be renamed in YRpp to better match their actual purpose, A is for Autocreate=false teams and B for true ones) to be set to true produces no false-positive results for Autocreate=true teams and has no impact on Autocreate=false teams which didn't have this issue to begin with.

Although I am unsure how the both flags set to false case return true from 0x4DA230 when the only code path in it that can requires RecruitableA to be set to true.

Aside from that I have some nitpicks.

[Starkku]

Single row if statements (the condition is on single row) don't need the curly braces per project code style specs and omitting them is usually cleaner but them being there isn't a problem. LGTM I guess.

Although I am unsure how the both flags set to false case return true from 0x4DA230 when the only code path in it that can requires RecruitableA to be set to true.

If someone is able to enlighten me on how this happened though it would be nice for understanding this part of the game better.

[handama]

Single row if statements (the condition is on single row) don't need the curly braces per project code style specs and omitting them is usually cleaner but them being there isn't a problem. LGTM I guess.

Although I am unsure how the both flags set to false case return true from 0x4DA230 when the only code path in it that can requires RecruitableA to be set to true.

If someone is able to enlighten me on how this happened though it would be nice for understanding this part of the game better.

My mistake. The issue only occurs in the 1,0 case. I didn’t thoroughly double-check during the translation.