Remove redundant prompts - attacking defenseless units or retreating to a single territory #1646
Conversation
…a single territory
…porting another defenseless unit to become non-defenseless
|
Resolves #1270. |
| @@ -26,6 +26,8 @@ | |||
| import games.strategy.engine.message.ConnectionLostException; | |||
| import games.strategy.sound.SoundPath; | |||
| import games.strategy.triplea.TripleAUnit; | |||
| import games.strategy.triplea.ai.proAI.ProData; | |||
| import games.strategy.triplea.ai.proAI.util.ProBattleUtils; | |||
There was a problem hiding this comment.
This needs updated to remove AI classes.
simon33-2
force-pushed
the
miniRemovePrompts
branch
from
06e942f
to
1793246
Compare
simon33-2
force-pushed
the
miniRemovePrompts
branch
from
1793246
to
9378c74
Compare
|
@ron-murhammer Is this ok now? |
| @@ -819,6 +819,19 @@ private void pushFightLoopOnStack(final boolean firstRun) { | |||
|
|
|||
| @Override | |||
| public void execute(final ExecutionStack stack, final IDelegateBridge bridge) { | |||
| if (Match.someMatch(m_attackingUnits, Matches.unitHasAttackValueOfAtLeast(1))) { | |||
There was a problem hiding this comment.
Are you sure this would cover all cases? Would it work if I had 2 units that provided negative support to each other therefore making them have a total of 0 attack? Would it work if enemy units provided negative support canceling out my attack value?
I think in general, we can't be checking individual units when it comes to attack/defense values do to how flexible support attachments are (some maps do use negative support and/or supporting enemy units).
There was a problem hiding this comment.
Would that ever happen? I would imagine that the approach for defense units would work so I'll apply that here.
There was a problem hiding this comment.
Potentially. Imagine I create a 'wall' unit that reduces enemy unit attack by 1. Then I attack with a unit that has only 1 attack so my total attack would reduce to 0.
| latch.countDown(); | ||
| if (possible.size() == 1) { | ||
| retreatTo[0] = possible.iterator().next(); | ||
| latch.countDown(); |
There was a problem hiding this comment.
So a potential problem with this is that you may not want to retreat after seeing you only have 1 option. So consider that I attacked with some units and that I conquered a province adjacent to it already this turn. The current battle is going poorly and I'm thinking that I'll retreat to the newly conquered province and forget that I can't. Then the retreat options pop-up and I realize that I can only retreat to a province that I don't want to retreat to.
I think you could probably skip the retreat options for air units that only have 1 option (this is probably the main case you want to avoid prompting the user). Land units on the other hand I think you'd need to add information on possible retreat locations to the first prompt in order to remove selection prompt if there is only 1 option.
There was a problem hiding this comment.
Surely that is the responsibility of the attacker though.
I suppose we could move the adding the battle territory when there are no retreat territories to after the prompt and look for size() == 0 but then I would still ask: Why?
Regarding your suggestion of the newly conquered province, the only way that is possible is if you blitzed through it is it not? In that case, retreating to the newly conquered territory would be completely valid.
There was a problem hiding this comment.
I don't think I was clear. Imagine I have unit A that attacks territory 1 and unit B that attacks territory 2 (adjacent to territory 1). I fight the battle for territory 1 and win. Then I'm fighting the battle in territory 2 and decide I might want to retreat and I forget that I didn't attack from territory 1. So I'm thinking I can retreat there when in reality I can't (many times users forget which territories they attacked from and therefore are allowed to retreat to).
There was a problem hiding this comment.
Sorry, I still don't see the relevance of the battle in territory 1 is relevant. If I own territory 3 which borders territory 2 and didn't attack from there I could just as easily think that I could retreat from there.
Anyway, it was trivial to add the territory to the label on the initial retreat button so all of the above is academic.
There was a problem hiding this comment.
You're correct. My example was overly complicated and not sure why I was thinking of another battle and not just multiple owned adjacent territories.
… territory retreating to
|
Seems like I've been able to address your concerns here @ron-murhammer . Thanks for clearing away the clutter in the PR list! |
|
@simon33-2 We need to re-review these changes based on issues reported in #1809 and quick fix made in #1811 |
|
@simon33-2 Can you provide a save game to test the first part of this PR of adding the defenseless check and removing defenders? |
|
Not 100% sure what you are looking for but it seems to work 100% as it should. |
|
@simon33-2 This change is proving problematic. I've reverted the first portion of this change in #1953. |
Fix up and reinstate #1646 to not remove infrastructure units
DanVanAtta
changed the title
Part of #1632 #1615 #1599 #1621
Only a few lines changed and shouldn't break serialisation from what I can see.