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

AI Strategic Bombs Already Max Damage Factories #6718

Closed
drockken opened this issue Jun 18, 2020 · 28 comments
Closed

AI Strategic Bombs Already Max Damage Factories #6718

drockken opened this issue Jun 18, 2020 · 28 comments
Assignees
Labels
AI Relates to AI Problem A problem, bug, defect - something to fix

Comments

@drockken
Copy link

How can the problem be recreated?

Set up a factory with maximum damage, and an opportunity for the AI to strategic bomb the factory.

What is an expected fix?

AI enhancement to not strategic bomb factories that cannot be damaged any further. An algorithm could be added to deny bombing when expected damage is less than the expected cost of losing the plane.

I think I have seen this issue with tactical bombers and harbors/airfields as well, but I don't have an example.

Which Engine Version are you using?

2.0.19993 - I have seen this issue in a number of different versions of TripleA. I attached a save-game, look at Italy's most recent move to see. The max factory damage in London is 20, but Italy strat-bombs the factory anyway.

Let me know if the Zip doesn't upload.

sb_max_dmg.zip

(Optional) Additional information

@DanVanAtta DanVanAtta added the Problem A problem, bug, defect - something to fix label Jun 22, 2020
@beelee1
Copy link
Contributor

beelee1 commented Jun 22, 2020

Maybe AI wants to make sure those factories are obliterated. Might want to ask @ron-murhammer he's the AI guy :)

@panther2
Copy link
Contributor

FWIW I just want to add that "overbombing" is not violating the rules, however useless it appears to be.

@DanVanAtta
Copy link
Member

DanVanAtta commented Jun 22, 2020 via email

@DanVanAtta DanVanAtta added the AI Relates to AI label Jun 22, 2020
@DanVanAtta DanVanAtta added this to Triage in Problem Tracker via automation Mar 25, 2022
@DanVanAtta DanVanAtta moved this from Triage to Back Log: Standard in Problem Tracker Mar 25, 2022
@frigoref
Copy link
Member

@panther2 @DanVanAtta @beelee1
I have no clue what the issue here is. Can anyone summarize this nicely and provide an example that can be used?

AI enhancement to not strategic bomb factories that cannot be damaged any further. An algorithm could be added to deny bombing when expected damage is less than the expected cost of losing the plane.

I think I have seen this issue with tactical bombers and harbors/airfields as well, but I don't have an example.
My questions would be:

  • Is the rule with the "execpted cost of losing the plane" making any sense to you guys? The damage can be repaired, no? Shouldn't the rule reflect the likelihood of killing the factory unit?
  • How about examples for tactical bombers and harbors/airfields? Can anyone provide them as well?

@beelee1
Copy link
Contributor

beelee1 commented Mar 31, 2022

@frigoref The Factories can only be damaged not destroyed. So the AI is risking losing a Bomber for no gain once maximum damage has been attained. Harbors and Airfields work the same as factories in that they can't be destroyed. Tac Bmbrs may only attack Harbors and Airfields, not Factories.

I assume this refers to the Global 40 game, I didn't look at the save, but it is possible and other games may have it, where the "infrastructure" (factories, airfields, harbours, etc...) can be destroyed.

Someone else was working on the AI for a while after Ron, but I can't remember who it was and if they ended up finishing anything with it.

@DanVanAtta
Copy link
Member

Is this fixed now? I recall reading in forums that someone mentioned this was no longer a problem.

@beelee1
Copy link
Contributor

beelee1 commented Apr 1, 2022

Is this fixed now? I recall reading in forums that someone mentioned this was no longer a problem.

I hadn't heard that. I don't play with the AI. @RogerCooper have you seen this with the AI recently ?

@panther2
Copy link
Contributor

panther2 commented Apr 1, 2022

@frigoref @DanVanAtta @beelee1

The reason to 'overbomb' might be to attract enemy fighters to act as interceptors - so those cannot have an active part in the follow up battle. So this really is an AI-issue only.

@beelee1
Copy link
Contributor

beelee1 commented Apr 1, 2022

@panther2 yea AI only and not against the rules but it'd perform better if it didn't do that though. I would think most humans would just let the AA take a crack at the Bmbrs and let their Ftrs sortie elsewhere. Would depend on there Air Battle value I guess.

@panther2
Copy link
Contributor

panther2 commented Apr 2, 2022

@beelee1 Just wanted to point that out again. I remember discussions on A&A .org where players made heavy use of overbombing and others erroneously thought it was against the rules.

@frigoref
Copy link
Member

frigoref commented Apr 2, 2022

@beelee1 @panther2 At should be the new rule to be implemented for our AI?
I would suggest something like
'strategic bombing only if no AA guns and hit potential >= min damage to kill at least one unit'.
What are your suggestions?

@panther2
Copy link
Contributor

panther2 commented Apr 3, 2022

@frigoref

As in some games facilities have their own 'built-in' AAA "no AA guns" would never qualify.

I'd suggest an AI rule as simple as: 'strategic bombing only if current damage < max. damage'.

@frigoref
Copy link
Member

frigoref commented Apr 3, 2022

@frigoref

As in some games facilities have their own 'built-in' AAA "no AA guns" would never qualify.

I'd suggest an AI rule as simple as: 'strategic bombing only if current damage < max. damage'.

Current damage and max. damage of what, @panther2 ?

@panther2
Copy link
Contributor

panther2 commented Apr 4, 2022

@frigoref

Current damage and max. damage of facilities subject to be bombed.

Or have I misunderstood something?

@frigoref
Copy link
Member

frigoref commented Apr 6, 2022

@panther2 , @beelee1 , @DanVanAtta
Can someone have a look for my commit 667701f?
It is my first in the AI area and I am not sure the logic makes sense in all cases.

My test was positive and I derived it from the initially provided save game. Here for your own retest sb_max_dmg_IT_combat.zip

@frigoref
Copy link
Member

Pretty please 😇 (@panther2 , @beelee1 , @DanVanAtta )

@beelee1
Copy link
Contributor

beelee1 commented Apr 20, 2022

@frigoref heh heh I can test the save but @asvitkine and @RoiEXLab would be better suited to reviewing your code changes :)

@frigoref
Copy link
Member

@beelee1 A functional test on your end would be great to confirm the issue is solved from a user perspective.
Please provide feedback here.

@asvitkine
Copy link
Contributor

@frigoref Thanks for looking at this.

I think when you're looping through for (final Unit targetUnit : t.getUnitCollection()) {, you need to check if the units are factories that can be bombed, and not just any unit. So you need something like Matches.unitCanProduceUnitsAndCanBeDamaged().

But also, there's a check above for .and(Matches.unitIsLegalBombingTargetBy(unit)), so I'm not sure a map that's independent of the units makes sense in this case. Perhaps start with just doing the computation each time so we can get the logic right, and then we can see if there's an opportunity to optimize? Bombers are fairly rare so it may not be necessary to do more optimization...

@frigoref
Copy link
Member

@asvitkine Thanks for the feedback!
I have added another commit 8e48922 and was able to reuse a match for this.
Please check the new version and let me know if you see anything else.

@asvitkine
Copy link
Contributor

asvitkine commented Apr 21, 2022

@frigoref Perhaps you can open a PR for easier review? You can mark it as a work in progress so that it doesn't get merged until it's ready and we're confident in it.

I think your update is along the right lines, but I'm not sure using the bombedTerritoryDataMap makes sense. I don't know all the details of the possible map options, but if there is a unit A that can bomb factory B and a unit X that can bomb factory Z (where A != X and B and Z are different factory types), then they should be tracked separately - because the data inside bombedTerritoryDataMap.computeIfAbsent() that was computed for A bombing B isn't valid for X bombing Z.

So I suggest getting rid of the map and just doing the computation directly. At least, as a start. We can then see if there is actually a performance issue there or not. If there is, we could have a map by territory and unit type, for example.

@frigoref
Copy link
Member

@asvitkine
Thanks for your feedback! I have create PR 10367 AI Strategic Bombs (Issue 6718)

Concerning your example A bombs B and X bombs Z (B and Z are different factory types) I have to say I am not that familiar with this. If I understand you correctly, than what is missing is the maxDamage check against the individual target unit.
If this is the case, then that is not the topic of this issue, but a separate one, right?
Before there was no such check and with the PR there is still none, so the PR could go through nevertheless.

My understanding would be that for your described case the damage would have to be assigned only to the potential target units. Then the territory is only a potential target for bombing if for any units minDamageNeeded <= maxDamage.
Is that also your understanding? Could it be that a factory unit has a minimal damage for each bombing unit, meaning is it correct to sum up the potential damage of all bombing units?
I already have something in the making, but I can commit this after the questions are clarified and potentially after PR 10367.

@asvitkine
Copy link
Contributor

Concerning your example A bombs B and X bombs Z (B and Z are different factory types) I have to say I am not that familiar with this. If I understand you correctly, than what is missing is the maxDamage check against the individual target unit. If this is the case, then that is not the topic of this issue, but a separate one, right? Before there was no such check and with the PR there is still none, so the PR could go through nevertheless.

I was going by the actual APIs and code that exist in the game right now, which seem to imply the above scenario is possible. Notably:

  public static Predicate<Unit> unitIsLegalBombingTargetBy(final Unit bomberOrRocket) {
    return unit -> {
      final UnitAttachment ua = UnitAttachment.get(bomberOrRocket.getType());
      final Set<UnitType> allowedTargets =
          ua.getBombingTargets(bomberOrRocket.getData().getUnitTypeList());
      return allowedTargets == null || allowedTargets.contains(unit.getType());
    };
  }

This implies that there can be different types of bombers that are allowed to bomb different buildings. Since the code above checks the attachment on the bomber/rocket and checks the allowed targets on that attachment. Let's continue discussion on the PR.

@panther2
Copy link
Contributor

panther2 commented Apr 27, 2022

FWIW, indeed, different bombers can attack different targets. E.G. Tactical Bombers may bomb Harbors and/or Airfields, while Strategic Bombers may bomb Industrial Complexes and/or Harbors and/or Airfields (example taken from 1940_global).

@asvitkine
Copy link
Contributor

I think this is fixed by #10367.

Closing.

Note: It covers logic to not send excess bombers, but doesn't cover the separate enhancement request of "An algorithm could be added to deny bombing when expected damage is less than the expected cost of losing the plane.". If you want, you can file a separate feature request for that.

Problem Tracker automation moved this from Back Log: Standard to Done Jun 29, 2022
@frigoref
Copy link
Member

@asvitkine As the coding avoids any air battles, wouldn't the additional feature be:
Also when there is a potential air battle, bombers should be considered bombing when expected damage is less than the expected cost of losing the plane.
Or how can a bomber be damaged during bombing at the moment?

@asvitkine
Copy link
Contributor

I think it doesn't take into account AA?

@drockken
Copy link
Author

drockken commented Jun 29, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
AI Relates to AI Problem A problem, bug, defect - something to fix
Projects
Status: Done
Development

No branches or pull requests

6 participants