Much of the retreat code duplicates navigate and march code. Worse,
retreat's version is full of bugs:
* Land units can sometimes retreat when they couldn't march: while on
the trading block (forbidden with march since 4.0.9), crewless
(likewise since 4.0.0), kidnapped in a foreign sector (inconsistent
since land units were added in Chainsaw 3), loaded on a ship
(likewise) or a land unit (inconsistent since trains were added in
4.0.0).
* Ships can retreat while on the trading block (forbidden with
navigate since 4.0.9)
* Land units can't retreat into foreign sectors even though they could
march there, namely when sector is allied or the land unit is a spy.
They can march there since 4.0.0.
* Land units keep their fortification on retreat. Has been that way
since retreat was added in Chainsaw.
Then there's group retreat. It's basically crazy:
* It triggers retreat for everyone in the same fleet or army, one
after the other, regardless of retreat path, conditions (including
group retreat), or even location. The latter is quite abusable
since retreats aren't interdicted. Has been that way since retreat
was added in Chainsaw.
* Group retreat fails to trigger when the originally retreating ship
or land unit has no retreat path left when it's done. Broken in
commit b860123.
Finally, the reporting to the owner is sub-par:
* When a retreat is cut short by insufficient mobility or
obstructions, its end sector isn't reported, leaving the player
guessing.
* Non-retreats can be confusingly reported as retreat to the same
sector. Can happen when the retreat path starts with 'h' (obscure
feature to suppress a single retreat), or when a group retreat
includes a ship or land unit without retreat orders.
* Interaction with mines during retreat is reported before the retreat
itself, which can be quite confusing.
* Sweeping landmines isn't reported at all.
* Much code and much bulletin text is dedicated to reporting what
caused the retreat, even though it should be perfectly obvious.
Rewrite this on top of common navigate and march code. Reuse of
common code fixes the "can retreat when it couldn't navigate/march"
and the "can't retreat into sectors it could navigate or march into"
bugs, and improves the reporting.
One special case isn't a bug fix but a rule change: mountains. The
old code forbids that explicitly, and it's clearly intentional, if
undocumented. The new code allows it by not doing anything special.
Turn group retreat into an actual group retreat: everyone in the same
fleet and sector with the the same retreat path and group retreat
condition joins the group. The group retreats together, just like in
navigate and march.
Take care to always report the end sector. When retreat is
impossible, report "can't retreat". When retreat is partial, report
"and stays in X,Y". When it's complete, report "stopped at X,Y".
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
This reverts commit df8a1ffc1b.
Because it breaks group retreat. Trivial conflicts due to the removal
of option SAIL.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Conflicts:
src/lib/subs/retreat.c
tests/retreat/journal.log
This exposes more bugs. They're marked "BUG:" in the test input. A
few bugs get masked, but I'll unmask them again in the next commit.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
To reduce coupling between test cases.
Lucky dice expose another bug. It's marked "BUG:" in the test input.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
setup-POGO unintentionally gives dead ships, planes and land units
mobility, which makes them show up in final.xdump. Rearrange to avoid
that, and for clarity. While there, improve comments.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
It sets the new type, then falls through to setting tech if the new
type requires more than the plane currently has. Two problems with
that:
* If we fall through, the plane is invalid: it has less tech than
required. Its only use before it gets overwritten is pln_set_tech()
calling pln_range_max() to find out whether the range is limited.
Passes a negative number to log(). Not fatal, but pln_set_tech()'s
range adjustment is unlikely to work.
* If we don't fall through, the range may still need adjustment,
either up (to keep it unlimited if the new type has more range), or
down (to keep it within the new type's shorter range).
Screwed up when the key was added in commit 6b0b6f17. Fix by
adjusting tech first, then setting the type, then adjusting the range.
The latter relies on pln_set_tech() coping with ranges exceeding the
type's maximum, which it does.
Change the other type edits similarly for consistency.
When a type edit triggers a tech change, the tech change is now
silent.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
torp() applies torpedo damage after retreat. Wrong, because mobility
cost increases with damage. Broken since retreat was added in
Chainsaw.
Fix by applying damage before retreat. Bonus: bulletins make more
sense.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
SAIL has issues:
* Sail orders are executed at the update. Crafty players can use them
to get around the update window.
* The route is fixed at command time. You can't let the update find
the best route, like it does for distribution.
* The info pages documenting it amount to almost 100 non-blank lines
formatted. They claim you can follow friendly ships. This is
wrong. They also show incorrect follow syntax. Unlikely to be the
only errors.
* Few players use it. Makes it a nice hidey-hole for bugs. Here are
two nice ones:
- If follow's second argument is negative, the code attempts to
follow an uninitialized ship. Could well be a remote hole.
- If ship #1 follows #2 follows #3 follows #2, the update goes into
an infinite loop.
* It's more than 500 lines of rather crufty code nobody wants to
touch. Thanks to a big effort in Empire 2, it shares some code with
the navigation command. It still duplicates other navigation code.
The sharing complicates fixing the bugs demonstrated by
navi-march-test.
Reviewing, fixing and testing this mess isn't worth the opportunity
cost. Remove it instead. Drop commands follow, mquota, sail and
unsail. Drop ship selectors mquota, path, follow.
struct shpstr shrinks some more, on my system from 160 to 120 bytes.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The autonavigation feature has issues:
* Autonavigation orders are executed at the update. Crafty players
can use them to get around the update window.
* Usability is poor:
- The order command is overly complex, not least because it can do
five different things: clear, suspend, resume, declare route, set
cargo levels.
- Unlike every other command involving movement, order does not let
you specify routes, only destination sectors.
- Setting cargo levels can silently swap start and end point of a
circular route, because "this keeps the load_it() procedure
happy". Maybe it does, but it surely keeps players confused.
- Setting "start" cargo levels actually sets the "end" levels, and
vice versa. Has always been broken that way.
- Predicting what exactly autonavigation will do at the update isn't
easy.
* The info pages documenting it amount to almost 400 non-blank lines
formatted. They claim only merchant ships can be given orders.
This is wrong. Unlikely to be the only error.
* Few players use it, and its workings at the update a fairly opaque.
Makes it a nice hidey-hole for bugs. Here are two:
- Unlike the scuttle command, autonavigation happily scuttles trade
ships while they're on the trading block.
- Unlike the load command, autonavigation can load in friendly and
allied sectors.
* It's more than 700 lines of rather crufty code nobody wants to
touch. Thanks to a big effort in Empire 2, it shares code with the
navigation command. It still duplicates load code. The sharing
complicates fixing the bugs demonstrated by navi-march-test.
Reviewing, fixing and testing this mess isn't worth the opportunity
cost. Remove it instead. Drop commands order, qorder and sorder.
Drop ship selectors xstart, xend, ystart, yend, cargostart, cargoend,
amtstart, amtend, autonav.
xdump ship sheds almost half its columns. struct shpstr shrinks, on
my system from 200 to 160 bytes.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Cuts size of export files in test suite by a factor of four. Not a
big deal for disk usage, as export files compress very well, and disk
space is cheap anyway. Export files are simply easier to work with
when they aren't full of redundant crap.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When a retreating ship or land unit runs into a sector it can't enter,
it stops. The direction character that led it there is consumed, even
though it could not be followed. The next retreat will then attempt
to follow the rest of the path. Don't do that.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Undocumented misfeature: retreat and lretreat accept anything as
retreat path. The paths' actual consumers retreat_ship1() and
retread_land1() silently ignore invalid direction characters.
The retreat paths are in xdump, and invalid ones could conceivably
confuse smart clients.
Change the commands to reject invalid paths, and the consumers to oops
on invalid direction characters.
Note that invalid paths get rejected even when they're not actually
used because the conditions argument contains a "c" for "cancel".
Requiring the user give a new path so he can cancel the old one is
comically bad design.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Obscure feature: 'h' in a retreat path stops the current retreat. The
code treats that as entering the current sector again, thus charges
mobility for staying put. It also reports "could not retreat to" for
a ship or land unit that can retreat out of, but could not retreat
into its current sector, e.g. a ship in an unfriendly harbor.
Fix by cleaning up the tortuous control flow.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The retreat code happily retreats anything, without considering who
owns it. It reports retreat to the owner by bulletin, even when the
owner is the current player.
Commands shouldn't report to the current player by bulletin, they
should print directly. Fixable. However, your ships and land units
retreating from your own actions makes little sense. Suppress it.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
A group retreat is executed in increasing UID order. The resulting
bulletin can be confusing.
Instead, retreat the ship that had its retreat conditions satisfied
first, and only then its group, if any.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The mission gets cleared whenever a retreat is triggered, even for
ships and land units that are unable to retreat.
Clear it only when the ship or land unit actually retreats.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
retreat_land() reads ships instead of land units, overrunning local
variable land. On lucky systems such as mine, this clobbers ni, and
triggers an oops. On unlucky systems, it crashes. On really unlucky
systems, it corrupts the land units file.
Broken since land unit retreat was added in Chainsaw 3.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Does not cover land unit retreat after a failed morale check.
The test exposes bugs. They're marked "BUG:" in the test input.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
