Fix supply update handling of new fleets

[Dilvish-fo]

Needed since there is now a pre-combat supply update being done.

In #1968 I had proposed, and in #2051 implemented, that monsters would not create supply blockades on the turn of their creation.

The particular fleet age check done in that implementation was explicitly dependent on the fact that all our supply updates at that time were done post-combat, relative to turn-update.

Somewhat more recently we wound up adding a post-turn-increment but pre-combat supply update, due to other issues, but at that time no one remembered about the relationship to monster blockades, and so new monsters were again able to create blockades on their first turn of creation and before surviving any combat round.

This PR fixes that by making the age check dependent on whether the supply update is being done precombat or not.

The following test file can be used to assess with and without this fix.
monster_blockade_check.sav.zip
In that file there is a floater (not currently visible) that will arrive at system Sitter Alpha on turn 63. To ensure that it spawns a Dyson Tree to test out this blockade issue, it may be helpful to first make the following change:

diff --git a/default/scripting/ship_hulls/monster/SH_FLOATER.focs.txt b/default/scripting/ship_hulls/monster/SH_FLOATER.focs.txt
index 0edd22b..b3e0c34 100644
--- a/default/scripting/ship_hulls/monster/SH_FLOATER.focs.txt
+++ b/default/scripting/ship_hulls/monster/SH_FLOATER.focs.txt
@@ -34,7 +34,6 @@ Hull
             activation = And [
                 Turn low = Source.LastTurnActiveInBattle + 1
                 InSystem
-                Random probability = 0.1
             ]
             stackinggroup = "TREE_GROWTH"
             effects = [

[Dilvish-fo]

Turns out this issue is a bit more complicated than I had hoped, for a couple of reasons.

One has to do with the intra-turn timing of savegame files. A manually saved file is done during the pre-combat stage of a turn. The supply update that is done when a save file is loaded is currently done with default parameters, which with this PR would wrongly result in a 'post combat' type supply update, and I have a test situation where this leads to different results depending on whether one loads the T-2 file or the T-1 file, which is clearly bad. @Vezzra So perhaps some at-least-minimal fix (for that issue with this PR) should be mandatory for 0.4.8 (or perhaps its better thought of as, this PR could only be optional for 0.4.8 if my expected fix for that complication works, and would otherwise just need to wait for post 0.4.8).

One simple fix for that would appear to be to make the game-load supply update be a pre-combat type check, which would be right for manually saved games, but until I get time to dig into the autosaves more I'm a bit uncertain about them.

It would be intuitive to think that the 'turn-start' autosaves would be pre-combat, and the 'turn-end' autosaves would be post combat, but then they would probably need other differential handling besides just this supply update issue, so I am tentatively suspecting that they all correspond to the pre-combat stage, but then I am left wondering if the difference is really only whether they include non-AI player orders. @dbenage-cx do you recall, could you clarify that for me?

The other complication relates to the paired supply-blockade, fleet-movement blockade aspects. The fleet movement aspect is currently partly, but not entirely, determined by the supply-blockade aspect. While the testing this out & discovering the problem above, I'm starting to think a further small tweak might be in order. I'll post some screenshots and savefiles and add some more explanation when I have some more time later.

[Vezzra]

@Dilvish-fo,

@Vezzra So perhaps some at-least-minimal fix (for that issue with this PR) should be mandatory for 0.4.8 (or perhaps its better thought of as, this PR could only be optional for 0.4.8 if my expected fix for that complication works, and would otherwise just need to wait for post 0.4.8).

So just let me see if I got that right:

When trying to implement this fix for supply update handling of new fleets, you ran into something what maybe can be called an "inconsistency" between manual and automatic saves, which leads to different results when loading a game, depending on the kind of the saved game (manual or automatic).

This "inconsistency" is essentially a bug, which needs to be fixed before your fix for supply update handling of new fleets works. Right so far?

Assuming yes, my question is if you think this bug is serious enough to be considered release blocking. But regardless of it's severity, my suggestion is to create a separate issue for that, and tag it accordingly (that is, assign it to optional or mandatory depending on your assessment of its severity).

When do you think you can get around to give an assessment on this save game inconsistency bug? The last release blocking issues/PRs have been closed/merged, so in theory we're ready for the first release candidate.

If possible, I'd want to provide RC1 on Thursday next week, but I want your assessment on the above first (no point in setting dates when you have to postpone them practically immediately anyway...).

[Dilvish-fo]

;tldr I think the state of the PR is fine now, it is a bug fix and should be cherrypicked into 0.4.8.

@Vezzra the inconsistency stemmed from the initial commit of this PR not fully fixing the problem this PR was addressing, which I determined with manual save files, dependent on whether they were from T-2 vs T-1 relative to the potential blockade. The cause of that inconsistency was that although the savegames correspond to the precombat state, they were getting the default (post-combat) supply update, which could give the wrong result if there was a potential blockade immediately upon load (the T-1 case). The second commit of this PR fixes that now by specifying the game-load supply update to be the precombat type. I have both thoroughly reviewed it, and tested it, and am pretty confident of it.

The issue regarding autosaves was that I was uncertain if they were always post-turn-increment-and-pre-combat like manual saves are. I have now reviewed the relevant code and concluded that they are. The 'turn start' autosave is done by the HumanClientFSM , at GameStart and when the turn-update is received, in both cases just before the player enters their orders (so is post turn-increment and precombat), and the 'turn end' autosave is done in HumanClientApp::StartTurn() immediately prior to submitting the players orders (so also pre combat). So all of the save files correspond to the post-turn-increment-pre-combat state, and so the supply update handling of this PR (as of the second commit) is correct.

[Dilvish-fo]

p.s. I had also mentioned that as a separate matter I was considering proposing a further tweak to the general handling, which would not have been a bug fix and so not suitable for 0.4.8. On considering the matter more I no longer think I even want to propose that tweak at all.

[Vezzra]

Alright, in that case, if this PR is merged in time (which I expect it will), it'll get cherry picked into the release branch.