triplea-game / triplea

TripleA is a turn based strategy game and board game engine, similar to Axis & Allies or Risk.
https://triplea-game.org/
GNU General Public License v3.0
1.29k stars 386 forks source link

AI Strategic Bombs Already Max Damage Factories #6718

Closed drockken closed 2 years ago

drockken commented 4 years ago

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

beelee1 commented 4 years ago

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

panther2 commented 4 years ago

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

DanVanAtta commented 4 years ago

This is the right place to report. Please do not PM or ask specific people to look at items.

If only one person handles an issue they are unlikely to create documentation, build code that is easily understood by others, and they themselves are stuck as they cannot go on vacation. If they do go manage to go on vacation, then it leaves the rest of us in a bad place

On Mon, Jun 22, 2020, 1:59 AM P@nther notifications@github.com wrote:

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

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/triplea-game/triplea/issues/6718#issuecomment-647383754, or unsubscribe https://github.com/notifications/unsubscribe-auth/AC6SZOMDSMAXF4QK7GOXX6LRX4MOVANCNFSM4OB72KZQ .

frigoref commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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

beelee1 commented 2 years ago

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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@frigoref

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

Or have I misunderstood something?

frigoref commented 2 years ago

@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 commented 2 years ago

Pretty please :innocent: (@panther2 , @beelee1 , @DanVanAtta )

beelee1 commented 2 years ago

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

frigoref commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

@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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

I think this is fixed by https://github.com/triplea-game/triplea/pull/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.

frigoref commented 2 years ago

@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 commented 2 years ago

I think it doesn't take into account AA?

drockken commented 2 years ago

As the submitter of this problem report to begin with, I think the context of the original issue has been lost, or at least I can't see that it has been directly addressed.

Take a major factory that can have a maximum of 20 damage and has been damaged to the maximum extent (20). The AI should not send bombers to bomb that same factory, because the built in AA could shoot down the bomber but no additional damage could be inflicted to the factory. If this no longer happens, then as far as I am concerned, the issue has been fixed.

Surely, the AI could be improved to make decisions in circumstances when the factory is only at 16/20 damage, whether there are intercept (or escort fighters), or other better uses of the bomber. But that is beyond the scope of the submitted problem and could go in as a separate tracking item.

~drock


From: asvitkine @.> Sent: Wednesday, June 29, 2022 12:51 PM To: triplea-game/triplea @.> Cc: drockken @.>; Author @.> Subject: Re: [triplea-game/triplea] AI Strategic Bombs Already Max Damage Factories (#6718)

I think it doesn't take into account AA?

— Reply to this email directly, view it on GitHubhttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftriplea-game%2Ftriplea%2Fissues%2F6718%23issuecomment-1170301807&data=05%7C01%7C%7Cc581d6a76a2b4c251bee08da59f80657%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637921219035452651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=fmtiy9ejq%2FOxOY9zyM6ieZdnWl1Lu5JtEMLY50cCK8I%3D&reserved=0, or unsubscribehttps://nam12.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fnotifications%2Funsubscribe-auth%2FAKBA4OLEMFMOTXBVN2JHECLVRSEK3ANCNFSM4OB72KZQ&data=05%7C01%7C%7Cc581d6a76a2b4c251bee08da59f80657%7C84df9e7fe9f640afb435aaaaaaaaaaaa%7C1%7C0%7C637921219035452651%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=BVMwTDZGOOmhFD438jG9kRx2Ml0I76YuoqqkcaOcNmw%3D&reserved=0. You are receiving this because you authored the thread.Message ID: @.***>