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

Fix bug that AI production pause when there are too many units in UnitDelays #20892

Merged
merged 1 commit into from Sep 9, 2023

Conversation

dnqbob
Copy link
Contributor

@dnqbob dnqbob commented May 28, 2023

Problem To Solve

From line 119 to line 131 in UnitBuilderBotModule.cs, the AI check the validation of the unit after pick up a unit, and drop the unit when unit is invalid. This will cause a problem: when there is too many restrictions set in UnitDelays and UnitLimits, the production will pause because AI cannot find a proper unit to match the restrictions by randomly picking up an unit in a production queue.

For example, in TS I want Nod produce more "e1", because we need to prevent nearby human player winning easily by using early-game infantry rush, in order to do that we delay the trainning of other infantry (I think that is the most common usage of UnitDelays). However, due to the bug here, AI will only have 1/4 possibility to produce the correct unit and doing nothing at most of the dangrous early-game time, which is against our intial design on using UnitDelays.

The similar problem can happen when you using UnitLimits, even UnitToBuild when there is too many units in production queue is not assigned in UnitToBuild.

SideEffect

This PR will in no doubt wasting some perfs on checking the validation of the unit until find a correct unit to build. But it should be worthy.

@dnqbob dnqbob force-pushed the ai-pause branch 2 times, most recently from 7fc9f1f to 95e25bc Compare May 28, 2023 14:26
var desiredError = int.MaxValue;
foreach (var unit in buildableThings)
{
if (!Info.UnitsToBuild.ContainsKey(unit.Name) || (Info.UnitDelays != null && Info.UnitDelays.ContainsKey(unit.Name) && Info.UnitDelays[unit.Name] > world.WorldTick))
Copy link
Contributor

Choose a reason for hiding this comment

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

Do not first check to see if something is in the dictionary to after retrieve it. Use TryGetValue to do it in one pass.

Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't the first always true? buildableThings is Info.UnitsToBuild so it will always contain the unit name?

Copy link
Contributor

Choose a reason for hiding this comment

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

So if a unit delay is set to have a value greater than world tick, then skip that unit. Why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't the first always true? buildableThings is Info.UnitsToBuild so it will always contain the unit name?

buildableThings is from queue.BuildableItems()

So if a unit delay is set to have a value greater than world tick, then skip that unit. Why?

This is the original design.

if (error < 0)
return HasAdequateAirUnitReloadBuildings(unit) ? unit : null;

if (error < desiredError)
Copy link
Contributor

Choose a reason for hiding this comment

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

First you shuffle all units. Then to here make sure you first build the units farest off from the desired count?

Is there a need to shuffle then still? I think you loose your randomness here?

Copy link
Contributor Author

@dnqbob dnqbob Jun 4, 2023

Choose a reason for hiding this comment

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

I think we should better combine both random and unit fraction control.

Consider this: we need medic forming 40% of the infantry squad, if AI build it fully randomly, then it is possible that we will get a suqad almost all medics, which is against our design.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You can consider the new ChooseRandomUnitToBuild is a combination of old ChooseRandomUnitToBuild and ChooseUnitToBuild with delay check and unit limit check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The random is, when units in Info.UnitToBuild less than the allowed fraction, AI will build unit randomly. If unit in the list is all more than allowed fraction, AI will choose unit to build to keep fraction.

@dnqbob dnqbob force-pushed the ai-pause branch 2 times, most recently from 6a4a2e2 to d52a081 Compare June 4, 2023 01:34
@PunkPun
Copy link
Member

PunkPun commented Jul 17, 2023

needs a rebase

@dnqbob
Copy link
Contributor Author

dnqbob commented Jul 17, 2023 via email

@PunkPun
Copy link
Member

PunkPun commented Jul 29, 2023

An RA AI game crashed just almost as soon AI started to produce units.

desiredUnit needs a fallback, we can't just let it be null. I'd also assume that as it crashed very early into the game there's an error in the logic

@dnqbob
Copy link
Contributor Author

dnqbob commented Jul 29, 2023

shall I have a look at the log?

@PunkPun
Copy link
Member

PunkPun commented Jul 29, 2023

@dnqbob
Copy link
Contributor Author

dnqbob commented Jul 29, 2023

An oversight, due to the base version of this PR have already drop the HasAdequateAirUnitReloadBuildings.

it crashed very early into the game there's an error in the logic

If this crash happends before AI build airfield or helipad, then there is an error in other place.

@dnqbob
Copy link
Contributor Author

dnqbob commented Jul 29, 2023

Also rebased to bleed.

@PunkPun
Copy link
Member

PunkPun commented Jul 29, 2023

With this PR AI's barracks idle way more than usual

@dnqbob
Copy link
Contributor Author

dnqbob commented Jul 29, 2023

I think it is due to I change IdleBaseUnitsMaximum as a hard limit, while we haven't adjust it in AI.yaml.

I have set it in ai.yaml.

@PunkPun
Copy link
Member

PunkPun commented Jul 29, 2023

It doesn't build even when it has no units

@dnqbob
Copy link
Contributor Author

dnqbob commented Jul 29, 2023

It doesn't build even when it has no units

In my test on map "Behind The Veil" it is not like that. 2 AI players (normal and rush) fills their squad immediatly, unless AI only build a kennel (because dogs has build limit).

Make sure you have this commit during tests, I have set the IdleBaseUnitsMaximum properly in this commit so production won't stall like before.

@PunkPun
Copy link
Member

PunkPun commented Sep 7, 2023

needs a rebase

@dnqbob
Copy link
Contributor Author

dnqbob commented Sep 7, 2023

Rebased, but untested.

mods/ra/rules/ai.yaml Outdated Show resolved Hide resolved
public readonly int IdleBaseUnitsMaximum = 12;
[Desc("If > 0, only produce units as long as there are less than this amount of units idling inside the base.",
"Beware: if it is less than squad size, e.g. the `SquadSize` from `SquadManagerBotModule`, the bot might get stuck as there aren't enough idle units to create squad.")]
public readonly int IdleBaseUnitsMaximum = -1;
Copy link
Member

Choose a reason for hiding this comment

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

We do want to have a specific value here. Imagine a scenario that an AI is stuck on an island. We don't want it to continue to produce units it can't use to attack

Copy link
Member

Choose a reason for hiding this comment

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

Though that usecase doesn't really even work. Units just get assigned to a squad and despite idling are no longer considered as idling

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Well, I think developers would prefer limit is disabled by default or it will be confusing when squad is stuck on not producing units.

Though that usecase doesn't really even work. Units just get assigned to a squad and despite idling are no longer considered as idling

This can be solved with add a limit of the max number of each type squad in bot hand.

Copy link
Member

Choose a reason for hiding this comment

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

I kinda think that this variable should either be removed in its entirety, or the calculation of idle units changed to include idle squad units in base

Copy link
Contributor Author

@dnqbob dnqbob Sep 8, 2023

Choose a reason for hiding this comment

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

Or the best idea is, change the design: SquadManager request units to build, UnitBuilder take the order to build.

If SquadManager find only hover units can reach the target, then it will build mostly hover unit on offensive. When SquadManager find it impossible to reach the target, it will just keep a protection squad and no order to UnitBuilder

Copy link
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

While this PR does not resolve IdleBaseUnitsMaximum properly, it does not leave bleed more broken in any way. I think the benefits are strong enough to justify a merge

@PunkPun PunkPun merged commit eab0bf8 into OpenRA:bleed Sep 9, 2023
3 checks passed
@PunkPun
Copy link
Member

PunkPun commented Sep 9, 2023

Changelog

@dnqbob dnqbob deleted the ai-pause branch September 9, 2023 17:44
@Mailaender Mailaender changed the title Fix bug that AI producion pause when there is too many unit in UnitDelays Fix bug that AI production pause when there are too many units in UnitDelays Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants