Optional arguments can save typing. Mandatory arguments are more
easily discoverable: just run the command and answer its prompts.
Empire traditionally uses optional arguments only for expert features.
Consider mission:
[0:640] Command : mission
Ship, plane or land unit (p,sh,la)? s
ship(s)? 0
Mission (int, sup, osup, dsup, esc, res, air, query, clear)? int
operations point? .
frg frigate Early Bird(#0) on an interdiction mission, centered on 21,-3, radius 0
1 ship
Compare retreat:
[0:638] Command : retreat
ship(s)? 0
shp# ship type x,y fl path as flt? flags
0 frg frigate 21,-3
1 ship
Arguments are not discoverable this way.
Change retreat to work like mission: make the second argument
mandatory, and if it's 'q', show retreat orders, else treat it as path
and ask for conditions:
[0:637] Command : retreat
ship(s)? 0
Retreat path, or q to query? jj
Retreat conditions ('?' to list available ones)? i
shp# ship type x,y fl path as flt? flags
0 frg frigate 21,-3 jj i
1 ship
To reduce smart client and script breakage, keep retreat with one
argument working as before, but print a deprecation warning.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Has no effect now. Before the recent rewrite of automatic retreat, it
could be used to trigger group retreat while staying put.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
"torped" comes from symbol table retreat_flags. Visible in output of
edit, retreat, lretreat and xdump. Tolerable in edit, but player
commands like retreat should really use proper words.
Fixing it in retreat_flags changes xdump output, thus risks breaking
clients. Do it anyway, since no known client recognizes this
particular symbol value.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The code to list ships got triplicated in Chainsaw. Empire 2 screwed
up one copy in its global replace of owner by player->owner.
Fix it by de-duplicating.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Rather than after post-yield sanity checking. Just to make it obvious
that we are handling getstarg() failure.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Screwed up when test-suite-only command __cmd was added in commit
e852d45. Should never happen with the intended use. Fix it anyway.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Broken in commit 40ec33b. Mitigating factor: can only happen when the
player gives an empty argument, e.g. retreat 0 "".
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
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
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>
Most of the remaining callers need to fool around with snprintf() to
use them. Not worthwhile anymore.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
We stop on mine hits only when they're fatal. Has always been that
way. When interdiction was added in Chainsaw, it worked the same.
Empire 2 changed the commands to stop on any interdiction damage. Now
stop on any mine damage, too.
Interdiction can fail to do damage (all bombs miss), and mines can be
detected without damage (by sweeping). Perhaps we should stop then as
well. Left for another day.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
unit_move() is too big and has too many paths through its loop.
Maintenance of the (unspoken) loop invariant isn't obvious. In fact,
it isn't maintained on some paths. I found several bugs:
* We check prerequisite conditions for moving before the first move
and around prompts. When a condition becomes wrong on the move,
movement continues all the same until the next prompt. I believe
the only way this can happen is loss of crew due to hitting a mine.
* We cache ships and land units in a list of struct ulist. When a
ship or land unit gets left behind, its node is removed from the
list and freed.
We keep pointer flg pointing to the flagship in that list for
convenience. However, the pointer isn't updated until the next
prompt. It's referenced for automatic radar and all sub-commands
other than the six directions and 'h'. Use after free when such a
sub-command gets processed after a flagship change without a prompt.
Same for land units. For instance, navigating a pair of ships "jh"
where the flagship has no mobility leaves the flagship behind, then
attempts to radar automatically using the ship in the freed list
node. Likewise, marching a similar pair of land units "jl" examines
the land unit in the freed list node to figure out how to look.
* We cache mobility in the same list to support fractional mobility
during movement. Movement deducts from cached mobility and writes
the result back to the ship or land unit.
If something else charges it mobility while it's in this list, the
cache becomes stale. shp_nav() and lnd_nav() reload stale caches,
but don't run often enough. For instance, when a ship hits mines,
the mine damage makes the cache stale. If a direction or 'h'
follows directly, the stale mobility is written back, clobbering the
mine hit's mobility loss.
This mess dates back to Empire 2, where it replaced a different mess.
There may be more bugs.
unit_move()'s complex control flow makes reasoning about its loop
invariant too error-prone. Rewrite the mess instead, splitting off
sensible subroutines.
Also fixes a couple of minor annoyances:
* White-space can confuse the parser. For instance, "jg l" is
interpreted like "jgll". Fix to reject the space. Broken in commit
0c12d83, v4.3.7.
* The flagship uses radar automatically before any sub-command (since
Chainsaw), and all ships use it automatically after a move (since
4.2.2). Make them all use it before and after each sub-command,
whether it's a move or not.
* Land units don't use radar automatically. Make them use it just
like ships.
* Always report a flagship / leader change right when it happens, not
only before and after a prompt.
Left for another day, marked FIXME: BTU charging is unclean.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Unlike the other "may move" conditions, "same mobility type
(MOB_MARCH, MOB_RAIL) as leader" and "not on sale" are only checked
when collecting ships and land units.
The former breaks when a deity unwisely edits a land unit's type while
it is being marched. Messed up when the check was added in commit
36e41e5 (v4.3.7). However, editing has become possible only recently,
in commit 6b0b6f1.
The latter would break if a ship could be put on the market while it
is being navigated, but that's not possible, because only the owner
can navigate (see also commit 8c502d4), and only the owner can sell.
Same for land units. Messed up in 4.0.9. Clean it up anyway. Bonus:
nicer error message, even spelled correctly.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
navi() uses shp_sel() to collect ships, then shp_nav() to drop
ineligible ships. shp_sel() wipes mission and retreat orders. Stupid
when shp_nav() will drop them right away.
Avoid that by having shp_sel() check shp_nav()'s conditions, too.
navi()'s shp_nav() call won't find anything to drop now. The call
will be removed shortly.
This drops "& stays in" from some failure reports, since shp_nav()'s
reject messages end with "& stays in X,Y", and shp_sel()'s don't.
Likewise for marc(), lnd_sel(), lnd_mar().
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Don't wipe it in lnd_sel(), rely on lnd_mar_one_sector() and
lnd_sweep() to wipe it when the land unit actually moves.
Closes FRE#43.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
lnd_may_mar() uses lp->lnd_own rather than actor, but that's okay,
lnd_mar() ensures they're the same.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
shp_may_nav() uses sp->shp_own rather than actor, but that's okay,
shp_nav() ensures they're the same.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Compare with DIR_LAST instead of DIR_VIEW, to avoid assuming DIR_VIEW
is the first non-direction code.
While there, oops on unexpected code.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
These commands report "sunk!" even when the ship survives the attack
but sinks during retreat. bomb even reports where on the retreat the
ship sinks. Has been that way since retreat was added in Chainsaw.
Report "sunk!" only when the attack sinks the ship directly.
Similar code exists for land units, but it doesn't report killings.
Change it anyway, to keep it consistent with the ship code.
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>
shp_sweep() and lnd_sweep() print only a couple of "Sweep...".
Sometimes, the sector isn't obvious, e.g. when you march multiple
sectors in one go, sweeping along the way.
Print "Approaching minefield at X,Y..." right before the first sweep
in a sector.
Note that retreat.c duplicates the sweeping code. Retreating ships
report sweeping with coordinates since commit dcd0794, v4.2.21.
Retreating land units still sweep silently. Left for another day.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Don't report every incapable ship or land unit. Complain only when
there are no capable ships or land units available.
The ships are all in the same sector, so complain about the sector
type just once instead of once per capable ship or land unit.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When ships enter a sector with sea mines, any minesweepers sweep, then
hit mines, and finally all ships (including the minesweepers) hit
mines. Sweeping in a sector (navigate sub-command 'm') works the same
without the final step.
When land units enter a sector with land mines, any engineers sweep,
and then all land units (including the engineers) hit mines. Sweeping
in a sector (march sub-command 'm') works the same, which means
non-engineers can hit mines then. Broken in Empire 2.
Actually broken for ships too then. 4.0.17 fixed ships, but neglected
to fix land units.
Change the land unit code to work like the ship code. Fixes march
sub-command 'm' not to expose non-engineers to mines. Changes march,
attack and assault with option INTERDICT_ATT enabled to expose
engineers twice.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When lnd_sweep() rejects an engineer for want of mobility, it removes
it from the list of units.
Can happen only when sweeping for march sub-command 'm'. Any engineer
without mobility is dropped from the march immediately. Broken in
Empire 2.
Fix lnd_sweep() to handle this case just like the others, and like
shp_sweep(): report and continue with the next list member.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Require positive mobility for sweeping mines, just like ships do.
Screwed up when land units were added in Chainsaw 3.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Commit cd8d742 mechanically combined struct mlist's mcp and struct
llist's llp into struct ulist's chrp, adding type casts to every use.
Not necessary, simply use mchr[] and lchr[] directly.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Handle "no movement" before the movement loop instead of relying on
the first iteration of the loop.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Handle "no movement" before the movement loop instead of relying on
the first iteration of the loop.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When you try to navigate a ship without canal capability into an
unfriendly canal, you get "can't go" when it's below 2%, else "too
large to fit". Always report "can't go" for unfriendly sectors.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The capability to navigate ships spread over several sectors is
obscure and rarely useful. Accidental use is probably more frequent
than intentional use. Issues:
* Interactive prompts show only the flagship's position, and give no
clue that some ships are actually elsewhere.
* Path finding is supported only when all navigating ships are in
the same sector.
* Interdiction becomes rather complex. For each movement, every
sector entered is interdicted independently. This means the same
fort, ship, land unit or plane can interdict multiple times.
Interdiction order depends on the order the code examines
ships. which the player can control. This is all pretty much
undocumented.
* Complicates the code and its maintenance. Multiplies the number of
test cases needed to cover navigate.
I feel we're better off without this feature.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The capability to march land units spread over several sectors is
obscure and rarely useful. Accidental use is probably more frequent
than intentional use. Issues:
* Interactive prompts show only the leader's position, and give no
clue that some land units are actually elsewhere.
* Path finding is supported only when all marching land units are in
the same sector.
* In each step, the bmap is updated for the leader's radar. The bmap
is not updated around other marching land units. Already odd when
all units are in the leader's sector, and odder still when some are
elsewhere.
* Interdiction becomes rather complex. For each movement, every
sector entered is interdicted independently. This means the same
ship, land unit or plane can interdict multiple times. Interdiction
order depends on the order the code examines land units. which the
player can control. This is all pretty much undocumented.
* Complicates the code and its maintenance. Multiplies the number of
test cases needed to cover march.
I feel we're better off without this feature.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Unlike the move command, march checks sector abandonment before every
step.
If the player declines, the last land unit stays put and is removed
from the march.
Except when sectors or land units change while we're waiting for the
player's reply. Then the last unit is not removed from the march.
This can scatter land units. Screwed up when checking for abandoning
the sector was added in 4.2.2.
Change march to work like move, and to avoid scattering land units: if
the player declines to abandon the sector, the command simply fails.
Put the check into new lnd_abandon_askyn().
Extend would_abandon() and want_to_abandon() from a single land unit
to many. Rename the latter to abandon_askyn() for consistency.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
