Combat targets also defenseless planet if targeting planet fails

[agrrr3]

Addresses #2488 which was not really fixed for outposts.

This thread
forum thread

Current state: should be complete, but only smoketested. I.e. somebody should playtest the effect

[Oberlus]

This doesn't work. Tested as follows:

Turn 20:

• Empire 1 (Oberlus) has colony in system X (it should not be relevant).
• Empire B (Oberlus2) has outpost in system X, with 1 shield, 0 defense (no infrastructure).

Turn 21:

• Empire A's frigate arrive at system X.
• Combat ensues and Empire B's outpost is attacked. Its shields are now 0.

Turn 22

• Empire A's troop ship arrives at system X.
• Combat ensues but the frigate shoots no one (as per combat log).
• Empire B's outpost shields are now 1.
• Empire A's troop ship can't invade.

Oberlus_vs_Oberlus2_turns_20&22.zip

[agrrr3]

Cant load the savefiles. I see you some state of the master plus my changes (according the entry in the savefile 2020-04-18.c4807de).

I think i branched of 49d3259. I am bit surprised. mmetz only did GUI stuff changes AFAICS.

Can you tell me the last commit of non-PR changes in master, i would rebase my branch there.
master+PR is a moving target.

Also I spotted the usual mistake of not making OrderedAlternativesOf toplevel. This could lead to targeting untargetable targets in the first tier in the OrderedAlternativesOf (e.g. your own ship), so not trying the second tier (so not including the defenseless planets). COMBAT_TARGETS_VISIBLE_ENEMY would then remove all that targets which are not enemy (e.g. your ship) or not visible.

Also swaqs use of LastTurnColonized breaks the species/happiness parsing for me.

Edit: Added a commit with OrderedAlternativesOf toplevel for mass driver - so we can check if that works

[Oberlus]

I lack git experience for this.
Can we test this PR within the latest weekly test build? I just created a new branch off that one and pulled there this PR without conflicts.

[Oberlus]

I did the following:

• Checkout weekly test build branch.
• Create new branch and pull there this PR.
• Compile from scratch.
• Start the two clients and load the save game attached in first comment (turn 22). No problem loading that game.
• Next turn (23): combat ensues, outpost is invadable.
• Next turn (24): combat ensues, outpost is invadable.

Seems it is fixed now.

[agrrr3]

I did the following:

* Checkout weekly test build branch.

weekly test branch is quite sensible/stable enough it would be just good if you could mention the revision of that weekly test branch (because that is going to stay if we want to test the next week)

* Create new branch and pull there this PR.

That actually does pull the latest master with my changes. Checking out the weekly test branch before does not have any influence.
If you lack the git-foo, you could add my repo as remote (https://github.com/agrrr3/freeorion) and pull the branch from there instead of the PR. (If you wanted to base my changes on the weekly test branch you would need to checkout my branch first and then do a git rebase origin/weekly-test-builds)

..
Seems it is fixed now.

Great, I will move all the OrderedAlternativeTo conditions to toplevel. I should really add a "filterFirst"/"checkOnly"/"common" condition attribute to OrderedAlternativeTo if I get around to it.

[agrrr3]

I rebased to 4da892b and with that I can load your savegame.

Also actually "System X" is not helpful, I guess you meant Centauri alpha.

I am actually not sure that the savegame does help much, I see the behaviour i would expect without my fix (own planet gets attacked by ship). Probably the combatTarget conditions get serialized into the savegame(?). In that case I would need to build a new ship for testing.

[agrrr3]

So I did a small playtest c4f9839 which contains "Make OrderedAlternativesOf toplevel for all combatTargets."

This seems to do what we want for invasion.

So far so great. But it does also something more: it attacks also neutral defenseless planets (which might be what we want if that is a planet with regenerating defenses/natives). The main downside is that practically your ships do everywhere combat.

[Oberlus]

I guess the alternative fix in #2879 (merged) is better than this one, which can be closed.