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

Incorrect handling of Facility bombing #2322

Closed
panther2 opened this issue Sep 4, 2017 · 24 comments
Closed

Incorrect handling of Facility bombing #2322

panther2 opened this issue Sep 4, 2017 · 24 comments
Labels
Problem A problem, bug, defect - something to fix

Comments

@panther2
Copy link
Contributor

panther2 commented Sep 4, 2017

My Operating System:

Windows 7.64

TripleA version:

1.9.0.0.6454 (and earlier)

Map:

WWII_Global_1940

Can you describe how to trigger the error? (eg: what sequence of actions will recreate it?)

TripleA does not handle bombing facilitites correctly, in case facilities are already at their maximum damage.

In the starting scenario, 6/6 damage is already assigned to as well the air base as the naval base. The factory has no damage.
See savegame SBR_StartPosition.tsvg
SBR_StartPosition.zip

When in combat move all planes from Holland Belgium are brought to UK at the same time with the intention to bomb the facilitites, the engine incorrectly sends them to a territory fight. TripleA does not ask for bombing, does not initiate a fight against interceptors, does not trigger facilities' AAA-fire.
Instead it executes the territory fight.
See savegame SBR_all_chosen.tsvg (history mode)
SBR_all_chosen.zip

What should happen is

  • the German player should be asked to bomb the facilities or fight in the territory
  • in case the German player decides for bombing, the British player should be asked to intercept
  • after the fight against the interceptors the Germany player should be asked to assign the bombers to whatever facility and the tactical bombers to the bases, regardless to whatever extent those are already damaged
  • facilities AA-fire should be fired (damage does not affect the self-defense antiaircraft ability of the facility)
  • bombing should be executed
  • damage should be assigned where possible

There is nothing in the rules that prevents bombers from bombing damaged (to whatever extent) facilities ... but damage exceeding the limits is not applied.

Rule discussion starting from here:
http://www.axisandallies.org/forums/index.php?topic=28562.msg1690237#msg1690237

When - from the same starting position - first the bombers are combat moved to UK, the prompt whether to bomb does appear. But when the tactical bombers are combat moved there, the prompt to bomb is missing - and they are automatically subject to territory fight, what is incorrect. Instead they should be able to bomb the bases (regardless of the amount of their damage).
See savegame SBR-chosen_separately.tsvg (history mode)
SBR_chosen_separately.zip

@prastle
Copy link
Contributor

prastle commented Sep 4, 2017

Excellent description @panther2 I can confirm this as it just occurred testing trains in @hepps new tww yesterday. we were using .6454 as well. It does function fine in the original tww using the current stable.

@ssoloff
Copy link
Member

ssoloff commented Sep 4, 2017

@panther2 The errors you attached are unrelated to the primary bug you reported above and are possibly a regression caused by #2313. I'll investigate and open a separate issue, if necessary.

@ssoloff
Copy link
Member

ssoloff commented Sep 4, 2017

@panther2 Confirmed. Those error logs are a regression caused by #2313. I opened #2323 and should have a fix prepared by the end of the day. Thanks for always being complete in your error reporting, even when things seem unrelated. 😄

@prastle
Copy link
Contributor

prastle commented Sep 4, 2017

i would like to reinforce the pt that escorts no longer seem to be supported as well. he did mention that above not sure if that is same problem. On a bombing run escorts sent are not included and act as combat units.

@panther2
Copy link
Contributor Author

panther2 commented Sep 4, 2017

Indeed, @prastle. As said, the complete SBR-sequence is suppressed, when the bug occurs.
Send fighters (as escorts) with the bombers only and it will work as expected.
But if you send them together with bombers and tac bombers, the bug will occur.
You can't send them as escorts together with the tac bombers (for bombing), either.

@ssoloff
Copy link
Member

ssoloff commented Sep 5, 2017

I did some debugging and think I found some useful information, although I'm not sure where the fault lies for causing this bug.

When sending the bombers and tactical bombers from Holland Belgium to attack the UK, no offer is made to choose SBR because the UnitAttachment#getAllowedBombingTargetsIntersection() method is returning the following collection of allowed unit types to bomb:

[UnitType{name=harbour}, UnitType{name=airfield}]

As both the harbor and airfield in this territory are at maximum damage, no option is given for SBR. (@panther2, you mentioned above that there is nothing in the rules that prevents bombing units at maximum damage, but the TripleA code seems to filter such units out from the available target list. I'm not sure when this "feature" was introduced.)

The obvious omission in the above collection is the factory_major unit type. I think this is because of the following configuration in ww2global40_2nd_edition.xml:

  1. The tactical_bomber unit attachment has isStrategicBomber set to true.
  2. The tactical_bomber unit attachment has bombingTargets set to airfield:harbour.

Because of (1), both the bombers and the tactical bombers are included in the bombersOrRockets parameter of getAllowedBombingTargetsIntersection(). Because the tactical bombers are part of this collection, the value of their bombingTargets property is considered when calculating the allowed bombing targets intersection. Thus, because of (2), the intersection of all possible bombing targets shared between bombers and tactical bombers ends up only including airfields and harbors.

The code that calculates the possible SBR targets is in MovePerformer.java:

final Collection<Unit> enemyTargets =
    Match.getMatches(enemyTargetsTotal,
        Matches.unitIsOfTypes(UnitAttachment
            .getAllowedBombingTargetsIntersection(Match.getMatches(arrived, Matches.unitIsStrategicBomber()),
                data)));

It looks like all this code was introduced in dc0cf72 and has pretty much remained unchanged since then (2012). So I'm not sure how a group of bombers and tactical bombers would be permitted to SBR anything other than an airfield or harbor unless

  1. The bombingTargets setting for tactical bombers included the different factory types sometime in the past, or
  2. The isStrategicBomber setting for tactical bombers was false, or
  3. The code somehow previously excluded tactical bombers from the call to getAllowedBombingTargetsIntersection() based on some other heuristic (e.g. it decided to treat the tactical bombers as escorts rather than as participants in the SBR).

@prastle You mentioned that the bombers + tactical bombers SBR scenario works in the "original TWW." Can you point me to a copy of that map? I'm curious if it predates the 2012 "bombingTargets" feature or if it has a different value than what's in the current version.

@panther2
Copy link
Contributor Author

panther2 commented Sep 5, 2017

Interesting, @ssoloff , thanks.

Just let me add that the engine would not allow bombers to further SBR max. damaged factories, either, not only in Global but for example in WWII_v5, too.
What would not bother me because it would be absolutely pointless to send bombers to AA-fire for nothing or to not use them in any territory fight, instead.

But it supports your above statement

... there is nothing in the rules that prevents bombing units at maximum damage, but the TripleA code seems to filter such units out from the available target list. I'm not sure when this "feature" was introduced.

@prastle
Copy link
Contributor

prastle commented Sep 5, 2017

@ssoloff actually I wasn't looking at the bombing. I was observing the escorts not escorting in the new and working fine in the old. The original Total World War in the download maps list works for escorts. The newer one with trains did not.

@ssoloff
Copy link
Member

ssoloff commented Sep 5, 2017

@prastle Thanks for the clarification, and sorry for the confusion. Would it be correct to say then that the escort issue is separate from the facility bombing issue @panther2 originally reported? If so, would you mind opening a separate issue for that so it doesn't get lost in the details of this ticket?

@panther2 I tried your save game in 3635 and experienced the same result as in the latest pre-release. Given all the information we've discovered so far, this bug doesn't sound like it's actually a regression but has been present for some time. Do you have any evidence that would suggest your scenario ever behaved as expected? (Just trying to nail down if this is a regression or not; if we know it's not, we won't waste time hunting through code history looking to find when it was broken. 😄)

@prastle
Copy link
Contributor

prastle commented Sep 5, 2017

@ssoloff actually i think it might be the same issue and engine related. Since while playing today with hepps using .6481 it still occurs but i will test the bomber issue separately. What I can clarify is that if using tww the fighter's escort on a bombing run with .3635 and current tww in download list.

@prastle
Copy link
Contributor

prastle commented Sep 5, 2017

and btw what is this amazing new list of functions im looking at LOL in github (off topic) :)

@panther2
Copy link
Contributor Author

panther2 commented Sep 5, 2017

@ssoloff
I would consider it being a bug rather than a regression. I suppose it always had been there and became only evident because of that special scenario that has been discussed. If the TripleA code seems to filter such units out from the available target list it needed a special scenario to discover this issue (as bombing max. damaged factories only is pointless and thus would not occur).

@ssoloff ssoloff added category: game engine > game rules Problem A problem, bug, defect - something to fix and removed regression labels Sep 5, 2017
@prastle
Copy link
Contributor

prastle commented Sep 6, 2017

after discussing with hepps it is def a separate issue. I will open a new issue.

@Heppisorus
Copy link
Contributor

Yes the escort function is definitely not working in the latest pre-release even though using the same XML in .3685 yields perfect functioning of the escort properties.

@ron-murhammer
Copy link
Member

@panther2 @ssoloff So does this essentially boil down to SBR units should be able to target units that are at max damage but currently aren't allowed to? And the logical reason this needs to be allowed is for maps that have air battles or want to take casualties for other bombers?

@Heppisorus
Copy link
Contributor

Heppisorus commented Sep 7, 2017 via email

@ron-murhammer
Copy link
Member

@Heppisorus That appears to be a separate problem as what @panther2 describes is the same behavior in the current stable release. Can you open a different issue describing the issue you are seeing around escorts (save game showing the behavior would be helpful as well).

@Heppisorus
Copy link
Contributor

Aye Capt.

@ron-murhammer ron-murhammer self-assigned this Sep 7, 2017
@ssoloff
Copy link
Member

ssoloff commented Sep 7, 2017

@ron-murhammer The problem is that attacking a territory with both bombers and tactical bombers in a single move gives the user different options than when the bombers and tactical bombers are moved separately. In the specific scenario described by @panther2 above, there are two independent engine rules responsible for this difference in behavior:

  1. SBR target list only includes facility unit types that can be SBRed by all air units in the sortie.
  2. SBR target list only includes facilities that are not at maximum damage.

When both bombers and tactical bombers are moved in a single sortie, (1) produces an SBR target list that only contains airfields and harbors. (2) further filters the airfield and harbor from the list because both are at max damage. The result being an empty SBR target list, and thus no option to SBR is offered to the user, and all units in the sortie are forced to air attack.

When the bombers and tactical bombers are moved in two sorties, the above rules are applied to each sortie individually:

  • For the bomber sortie, (1) produces an SBR target list that contains all facility types. (2) filters the airfield and harbor from the list because both are at max damage. The result being an SBR target list that contains the undamaged factory, and the user is offered the option to SBR.
  • For the tactical bomber sortie, (1) produces an SBR target list that only contains airfields and harbors. (2) further filters the airfield and harbor from the list because both are at max damage. The result being an empty SBR target list, and thus no option to SBR is offered to the user, and the tactical bombers are forced to air attack.

IMO, rule (1) is the primary cause of the difference in behavior. Rule (2) may be incorrect according to the tabletop rules, but I don't think changing it would fully solve the difference in behavior between moving using one or two sorties.

For example, let's say we modify rule (2) to not filter out fully-damaged units, so that the airfield and harbor are in the target list. In this case, the user would be prompted to SBR in the single sortie case, because the SBR target list after applying (1) and (2) would be non-empty. However, it would still only contain the airfield and harbor. Thus, (and I'm guessing here because I haven't verified) SBR damage rolls will only be applied to the fully-damaged airfield and harbor; no damage will be applied to the facility. If that's what the desired behavior is, then great, it's simply a matter of removing rule (2). But I would think the user would expect bomber damage to be applied to all targets (specifically the factory), while tactical bomber damage would be applied to the airfield and harbor (even though it has no effect).

Otherwise, if the goal is for the one- and two-sortie moves to produce identical behavior (i.e. bombers have the option to SBR the factory and tactical bombers always air attack), this will probably be non-trivial. I don't see how simply changing the behavior of rule (1) can achieve this. It seems the engine would somehow have to partition the units in the sortie to give the attacker the most bang for their buck.

@ron-murhammer
Copy link
Member

@ssoloff So the reason I believe for (1) is that you'd end up with a complex SBR target selection window. Imagine that instead of ALL it did ANY so you get a full list of any that could be bombed, then it would have to limit which target various SBR units could target (couldn't assign tac to factories for example). It instead makes you move them in chunks based on target so you move strats and choose targets then move tacts and choose targets unless the targets are the same then you can move all at once. You could argue this isn't great and a better UI to allow moving all at once then assigning to targets would potentially be better but that UI would be fairly complex if you have multiple targets and multiple types of bombers.

My thought is to leave (1) alone for now and fix (2) as that is the real bug and would allow you to do the appropriate bombings as long as you make multiple moves.

@ssoloff
Copy link
Member

ssoloff commented Sep 7, 2017

@ron-murhammer I agree that trying to modify (1) to accommodate every possible SBR scenario would just be a mess. You're right that putting the onus on the user to split units across multiple moves to achieve the desired attack behavior is the best compromise.

@panther2
Copy link
Contributor Author

panther2 commented Sep 7, 2017

So does this essentially boil down to SBR units should be able to target units that are at max damage but currently aren't allowed to? And the logical reason this needs to be allowed is for maps that have air battles or want to take casualties for other bombers?

This is my understanding, yes.

@Heppisorus
Copy link
Contributor

Not that I am fluent in what goes on behind the magic curtain... but is the mechanism that governs bombarding of any use in this situation? Seems like when you have lots of bombard options in the same SZ the engine does a very efficient job of separating out the different options available to the player. Just a random suggestion.

@panther2
Copy link
Contributor Author

panther2 commented Sep 8, 2017

@ron-murhammer @ssoloff
I am sorry, this is still not resolved correctly, will open a new issue....

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

No branches or pull requests

6 participants