Question on MustFightBattle#checkForUnitsThatCanRollLeft and "Transport Casualties Restricted" property
-
Thanks @LaFayette for clarifying that it depends exclusively on the basic defence value (as defined in the xml), and not at all on the defensive rolls value. I'd say the program should rather look at the current value, after applying all supports (instead of looking if there are any), territory effects and whatever the like.
@Trevan I think this is such a mess of unclear things and problems (outright wrong behaviours) that the only way out I see is that a developer documents exactly how the system works (every case of auto-elimination of units, clarifying exactly and fully when it happens and under what conditions it happens), then we can discuss how it should work, instead. I suggest doing this both for the non-infrastructure and the infrastructure case (infrastructures have problems too, I recall), trying to set or keep them apart.
-
@Cernel teasing out how it works is maybe not feasible. There's probably around 2-5k lines of code around thus, and you've seen how much analysis there was for about 10 of them.
Beyond that, we should be able to state how the sequencing should work ignorant of the actual. Such specifications are actually a really important and necessary thing for developers to have.
Let's be sure all specific questions here are answered. At some point we need a restructuring if rules are applied so that they are sensible and configurable. Assumptions like "in vo2 all units have defensive values and transport restricted means v3" need to be made explicit and bot just special cases throughout.
-
@LaFayette said in Question on MustFightBattle#checkForUnitsThatCanRollLeft and "Transport Casualties Restricted" property:
Generally the rule is, if the defender has no power left, and the attacker would just need to sit there and keep rolling dice, then it's an auto-kill situation.
So, should the "remove units that can't roll anymore" (not including unescorted transports) logic be based on the value of "Transport Casualties Restricted"? Because it is sometimes based on the value and sometimes isn't.
-
In short, yes (kinda).
In the current code the 'transport casualties restricted' tells you if the transports can defend of not. That is used in place of checking for a 'v3' rule set. Probably we should be checking if the transport units have hitpoints instead.
-
@Trevan So why the first post still says "At the end of a round when a defender either has no more units or only infrastructure units"? Didn't you say that is actually in between of the defensive fire step and the retreat step? You said there is nothing related to this issue anywhere after the retreat step (thus nothing "at the end of a round"), right? Just asking to be sure the first post is wrong and being corrected by following ones.
Also, didn't you say that you misread the code, and the first auto-kill is about defenceless and the second about infrastructures? In this case, of course, the first one would be v3 rules only and the second one would be generic.
Also, can you confirm that when you said things like this:
Another question. In this auto-kill location after the standard attack, it will only remove undefended transports IF "Transport Casualties Restricted" is true. But it will always remove units with no rolls. But the auto-kill location before the standard attack, it will only kill undefended transports AND units with no rolls IF "Transport Casualties Restricted". Any ideas why the two locations treat units with no rolls differently? Should they be treated the same? And if so, which way should it be treated?
The "units with no rolls" are actually units with attack and defence values equal to 0 (which I would say is a wrong check, as only one of the other should matter, dependind whether the unit is offending or defending) and with no support attachments, regardless of their offensive or defensive rolls values? If so, can everyone please stop confusingly saying "no rolls" or "units that can't roll anymore" to refer to this, as that literally comprises units that have 0 rolls (and possibly positive attack or defence or both) and units that cannot target any remaining units, which I understand it is not what it is being checked (right?)?
Regarding:
There are two types of auto-destruction: 1) unescorted transports 2) units with no rolls. #1 is always controlled by "Transport Casualties Restricted". #2 is only controlled by "Transport Casualties Restricted" before the standard fire step. After the standard fire step, it doesn't care about that property.
How is the situation after the fire steps ever even happening since you said:
I'm looking at the second place where the auto-kill is happening. And I think I misread the code. I think the auto-kill rarely ever happens there. It first checks that
CollectionUtils.getMatches(defendingUnits, Matches.unitIsNotInfrastructure()).size() == 0. So, it will only do the auto kill IF there are only infrastructure units available. And only those infrastructure units will be auto-killed.And, obviously, when only infrastructures remain for one or both sides, the battle is over regardless of anything else. So how is not everything else useless for the second check.
Furthermore, and here we go back to what I suggested that, before thinking about fine tuning these codes, one should rather fix the known problems related to them, you say:
The auto destruction also never happens after the retreat steps. It is either before the standard attack or after the standard attack but before the retreat step.
Which is contrary to what experienced as reported at this issue:
https://github.com/triplea-game/triplea/issues/4688
TripleA should not offer the attacker's option to retreat, if after a round of combat only a defending AAA-gun is left. In the attached savegames (1.9/1.10) the remaining infantry is incorrectly offered a retreat option when facing only a "defending" AAA gun.
That is saying the problem is actually the behavioural absence of auto-destruction "after the standard attack but before the retreat step".
-
@LaFayette said in Question on MustFightBattle#checkForUnitsThatCanRollLeft and "Transport Casualties Restricted" property:
In short, yes (kinda).
In the current code the 'transport casualties restricted' tells you if the transports can defend of not. That is used in place of checking for a 'v3' rule set. Probably we should be checking if the transport units have hitpoints instead.
V3 transports have hit-points too. Just you can assign to those hit-points only after nothing else eligible remains.
Moreover, in my opinion, infrastructures have hit-points too and should be targetable by other units if these units have targeted fire against the infrastructures (for example, I believe a flying infrastructure should be targeted by AA fire). I said in my opinion, as this never acually happens in the basic games, so I'm extrapolating.
-
@LaFayette said in Question on MustFightBattle#checkForUnitsThatCanRollLeft and "Transport Casualties Restricted" property:
In short, yes (kinda).
In the current code the 'transport casualties restricted' tells you if the transports can defend of not. That is used in place of checking for a 'v3' rule set. Probably we should be checking if the transport units have hitpoints instead.
I'm not talking about transports. My question is about the other "auto-kill" situation; units that can't do anything anymore because they either don't give support or they have no attack/defense.
In one part of the code, those units are auto-killed depending on the value of "transport casualties restricted". In another part, those units are auto-killed regardless of the value of "transport casualties restricted". I feel like that is a bug. Going through the history, it looks like the code to auto-kill these "useless" units and the code to auto-kill the unescorted transports were add before the "transport casualties restricted" check. And then when that check was added, it was either not added in all the correct places or was added in too many places.
-
@Cernel Well, v3 transports do and don't have hitpoints, and that is my point. Instead of modelling it well, we have this hack "v3 transports restricted".
If you add transports to a battle calc, you're not adding hitpoints. A hitpoint is a damage slot you can assign during a battle. If you can only assign such damage at the end, and if you do it's always an auto-kill situation, then those units do not actually have hit points. That is my point, instead of having a "hit points" unit property that is set to zero for transports, we do this very round-a-about pedantic way to have a 1-hitpoint unit to effectively have no hit points. If the code just considered it if 0-hitpoint units remained then things would flow naturally rather than this special casing for a specific property that is only for transports and that also effectively overrides their unit properties.
-
"transport casualties restricted" is often used as a proxy for V3 rules. When other rule sets were launched, instead of updating the data model, the most convenient property was double-used. It's likely not a bug persay, but is a specific rule set for WWII maps and has assumption about which maps exist and that V3+ rules implies 'transport casualties restricted'
-
In other words @Trevan , it's likely not checked in other situations due to assumptions about which units would exist in those scenarios.
-
@Cernel said in Question on MustFightBattle#checkForUnitsThatCanRollLeft and "Transport Casualties Restricted" property:
@Trevan So why the first post still says "At the end of a round when a defender either has no more units or only infrastructure units"? Didn't you say that is actually in between of the defensive fire step and the retreat step? You said there is nothing related to this issue anywhere after the retreat step (thus nothing "at the end of a round"), right? Just asking to be sure the first post is wrong and being corrected by following ones.
When I said "end of round" in the first post, I was talking about after the shooting. I should have been more clear. I didn't think it really mattered.
Also, didn't you say that you misread the code, and the first auto-kill is about defenceless and the second about infrastructures? In this case, of course, the first one would be v3 rules only and the second one would be generic.
No, the first auto-kill and the second auto-kill affect the same types of units. It is just that the second auto-kill only occurs if all of the units are infrastructure units. That makes me think it hardly ever happens because I'm not sure any current maps have those types of units.
Also, can you confirm that when you said things like this:
Another question. In this auto-kill location after the standard attack, it will only remove undefended transports IF "Transport Casualties Restricted" is true. But it will always remove units with no rolls. But the auto-kill location before the standard attack, it will only kill undefended transports AND units with no rolls IF "Transport Casualties Restricted". Any ideas why the two locations treat units with no rolls differently? Should they be treated the same? And if so, which way should it be treated?
The "units with no rolls" are actually units with attack and defence values equal to 0 (which I would say is a wrong check, as only one of the other should matter, dependind whether the unit is offending or defending) and with no support attachments, regardless of their offensive or defensive rolls values? If so, can everyone please stop confusingly saying "no rolls" or "units that can't roll anymore" to refer to this, as that literally comprises units that have 0 rolls (and possibly positive attack or defence or both) and units that cannot target any remaining units, which I understand it is not what it is being checked (right?)?
"units with no rolls" are units that have either attack or defense of 0 depending on whether they are attacking or defending. That's in the code snippet above.
The reason why I was using "no rolls" is because that is how the code describes them. I'm going to start calling them "useless" units or "unprotected" units since they are units that have no attack or defense (depending on the situation) or they have no support capability.
Regarding:
There are two types of auto-destruction: 1) unescorted transports 2) units with no rolls. #1 is always controlled by "Transport Casualties Restricted". #2 is only controlled by "Transport Casualties Restricted" before the standard fire step. After the standard fire step, it doesn't care about that property.
How is the situation after the fire steps ever even happening since you said:
I'm looking at the second place where the auto-kill is happening. And I think I misread the code. I think the auto-kill rarely ever happens there. It first checks that
CollectionUtils.getMatches(defendingUnits, Matches.unitIsNotInfrastructure()).size() == 0. So, it will only do the auto kill IF there are only infrastructure units available. And only those infrastructure units will be auto-killed.And, obviously, when only infrastructures remain for one or both sides, the battle is over regardless of anything else. So how is not everything else useless for the second check.
That is what I'm trying to understand. In the second case, it checks if only infrastructure is around. Then it will auto-kill any of the undefended transports if "transport casualties restricted" is true. And it will auto-kill any of the unprotected units regardless of the "transport casualties restricted". But in the first case, it only auto-killed the unprotected units if "transport casualties restricted". So, why is the difference? And it is looking like the second case might be pointless unless you can have an infrastructure unit that is also a "unprotected unit" that needs to be auto-killed.
Furthermore, and here we go back to what I suggested that, before thinking about fine tuning these codes, one should rather fix the known problems related to them, you say:
The auto destruction also never happens after the retreat steps. It is either before the standard attack or after the standard attack but before the retreat step.
Which is contrary to what experienced as reported at this issue:
https://github.com/triplea-game/triplea/issues/4688
TripleA should not offer the attacker's option to retreat, if after a round of combat only a defending AAA-gun is left. In the attached savegames (1.9/1.10) the remaining infantry is incorrectly offered a retreat option when facing only a "defending" AAA gun.
That is saying the problem is actually the behavioural absence of auto-destruction "after the standard attack but before the retreat step".
No, that bug report is not contrary to what I'm saying. The aaGun is not an infrastructure unit. So the auto-kill that occurs before the retreat doesn't see it. The user is given the retreat option and then the round starts over and the unit is auto-killed before the attack step.
-
@LaFayette said in Question on MustFightBattle#checkForUnitsThatCanRollLeft and "Transport Casualties Restricted" property:
In other words @Trevan , it's likely not checked in other situations due to assumptions about which units would exist in those scenarios.
Except in this case it is both checked and not checked for the same situation. The first time it tries to auto-kill undefended units, it checks the property. The second time, it doesn't check the property.
-
@Trevan said in Question on MustFightBattle#checkForUnitsThatCanRollLeft and "Transport Casualties Restricted" property:
Except in this case it is both checked and not checked for the same situation. The first time it tries to auto-kill undefended units, it checks the property. The second time, it doesn't check the property.
Here's the code for the two places so that you can see what I'm talking about:
https://github.com/trevan/triplea/blob/master/game-core/src/main/java/games/strategy/triplea/delegate/battle/MustFightBattle.java#L1478 - Notice that this
IExecutableis wrapped in an if check for the property.https://github.com/trevan/triplea/blob/master/game-core/src/main/java/games/strategy/triplea/delegate/battle/MustFightBattle.java#L1806 - Notice that this call (
checkForUnitsThatCanRoll) isn't wrapped in a check for the property.In both situations, the call to
checkUndefendedTransportsis wrapped in the check for the property. -
@Trevan From what I understand, the behaviour is going to be wrong either way. So this question sounds to me like "is it more correct to be wrong this way or more correct to be wrong this other way?". I don't think it's possible to give a good answer, here.
If the second check applies only as long as all units are infrastructures, then nothing else matters, since, if only one side has only infrastructures remaining, the battle is over regardless, thus this would be just a major case of pointless coding. Otherwise, it may be interesting to know what are the other cases in which the second check may trigger.
-
@Cernel said in Question on MustFightBattle#checkForUnitsThatCanRollLeft and "Transport Casualties Restricted" property:
@Trevan From what I understand, the behaviour is going to be wrong either way. So this question sounds to me like "is it more correct to be wrong this way or more correct to be wrong this other way?". I don't think it's possible to give a good answer, here.
If the second check applies only as long as all units are infrastructures, then nothing else matters, since, if only one side has only infrastructures remaining, the battle is over regardless, thus this would be just a major case of pointless coding. Otherwise, it may be interesting to know what are the other cases in which the second check may trigger.
So, you are saying that when there is only infrastructure units remaining on the defensive side, then there shouldn't be any "unprotected units" that need to be auto-killed? In other words, when all non-infrastructure units are dead, the defender should have no units that are (isInfrastructure OR isAA OR isFactory) AND NOT (attack OR defense OR hasUnitSupportAttachment)?
I'm going to change the code to check the property of "Transport Casualties Restricted" in both the cases.
-
@Trevan I suggest you not to make changes if you don't know what's the matter about it, especially in the moment that the change is not going to fix any known problems.
The auto-elimination steps should rather be generally revised, maybe from scratch, so to solve the known problems and creating a system actually enforcing the correct rules (that will need to be interpreted, to some extent).
Thinking hard about this, the only situation of only infrastructures remaining needing auto-elimination of non-infrastructure units I can think of is the case of defending units refusing to partake in air battles.
