Commit graph

5084 commits

Author SHA1 Message Date
9a4f1ce9ef bomb: Fix to reject attempts to bomb unused planes
Report "not spotted", like we do for unused ships and land units.
Missed in commit 23d52a4, v4.3,16.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:49 +01:00
e2b264f02c retreat lretreat: Be less loquacious when changing orders
Instead of listing all the ships or land units ordered, just print how
many got ordered, and describe the order, like this:

    [0:640] Command : retr 2/3 bg itb
    2 ships ordered to retreat along path bg when injured, torpedoed, bombed
    [0:640] Command : retr 2 h
    1 ship ordered not to retreat

fleetadd doesn't list the ships it reassigns, either.  On the other
hand, stop lists the sectors it stops.  Perhaps it should be gagged,
too.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:49 +01:00
c702dc9e94 tests: Wean off deprecated retreat syntax
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:49 +01:00
148af51ab3 retreat lretreat: Deprecate pseudo-condition 'c'
It's redundant; retreat path 'h' cancels orders just fine.  Document
that instead.  'c' still works, and I don't plan to break it as long
as it doesn't get in the way, which seems unlikely.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:49 +01:00
482d54c953 retreat lretreat: Change query syntax to match mission
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>
2015-03-02 08:20:49 +01:00
4de4da259a retreat lretreat: Strip trailing 'h' from retreat path
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>
2015-03-02 08:20:49 +01:00
d5757a4735 info torpedo: Say "torpedo", not "torp"
Documentation and player output should use proper words.  "torp" ain't
one.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:49 +01:00
d5d92caf94 retreat lretreat edit xdump: Change "torped" to "torpedoed"
"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>
2015-03-02 08:20:49 +01:00
e92a522731 symbol: New symbol_set_fmt() parameter sep, use for show
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:49 +01:00
9fc0719e54 symbol: Make actofgod.c's fmtflags() public as symbol_set_fmt()
Move to nstreval.c, rename, external linkage.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:49 +01:00
67fe316c0c bomb: Fix ship list header for ASW planes
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>
2015-03-02 08:20:49 +01:00
e4654866cf subs: Drop a few superfluous variables
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:49 +01:00
4ec6df865b launch name trade: Check for getstarg() failure immediately
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>
2015-03-02 08:20:48 +01:00
57fd96a7cf tests: Fix crash when getstarg() fails
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>
2015-03-02 08:20:48 +01:00
b72f727b9b retreat lretreat: Fix crash when getstarg() fails
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>
2015-03-02 08:20:48 +01:00
beedf8dced retreat: Rewrite automatic retreat code to fix its many bugs
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>
2015-03-02 08:20:48 +01:00
5bc934fe25 tests/retreat: Test mission clearing more thoroughly
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:48 +01:00
c820b984c7 tests/retreat: Cover spies
This exposes yet another bug.  It's marked "BUG:" in the test input.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:48 +01:00
891039d384 tests/retreat: Cover trains
This exposes yet another bug.  It's marked "BUG:" in the test input.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:48 +01:00
7da0c919f1 tests/retreat: Cover canals
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:48 +01:00
5d346f4af1 tests/retreat: Cover ships and land units on sale
This exposes more bugs.  They're marked "BUG:" in the test input.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:48 +01:00
40d347ed85 retreat: Don't report a destroyed ship/land unit couldn't retreat
Commit df8a1ff redone without breaking stuff.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:48 +01:00
cc8b92d555 Revert "retreat: Don't report a destroyed ship/land unit couldn't retreat"
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
2015-03-02 08:20:48 +01:00
32a6504a25 tests/retreat: Improve group retreat coverage
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>
2015-03-02 08:20:48 +01:00
36df559f33 tests/retreat: Put some targets in separate locations
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>
2015-03-02 08:20:48 +01:00
49de769470 tests/retreat: Drop unused minefields
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:48 +01:00
104bf6f777 tests/retreat: Clean up setup, improve comments
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>
2015-03-02 08:20:48 +01:00
617b194cef tests: Normalize setup.journal.log and setup.server.log
These logs are saved in the sandbox to help debugging setup.
Normalize them to make them easier to read.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:47 +01:00
0a012a3ed5 edit: Fix tech and range adjustment for edit p key 'T'
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>
2015-03-02 08:20:47 +01:00
efec4417d3 tests/actofgod: Test plane type change's effect on range
Demonstrate bugs: doesn't always preserve unlimited range, and doesn't
always cut range back to maximum.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:47 +01:00
e8e6faaa65 subs: Inline shp_stays(), lnd_stays()
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>
2015-03-02 08:20:47 +01:00
c5c6fc8a66 navigate march: Stop on non-fatal mine hits, too
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>
2015-03-02 08:20:47 +01:00
0538f95cd6 subs: Rename unit_path() to unit_move_route() and move it
Move it next to its only caller unit_move().

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:47 +01:00
74a71aa007 subs: Rename shp_nav(), lnd_mar()
... to shp_nav_stay_behind(), lnd_mar_stay_behind(), to better reflect
their purpose.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:47 +01:00
24000b4855 navigate march: Fix use-after-free and other bugs
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>
2015-03-02 08:20:47 +01:00
198e2dd076 subs: Move shared code from navi.c to subs
Rename do_unit_move() to unit_move() to blend into unitsub.c.  Give
its helpers static linkage.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-03-02 08:20:47 +01:00
520cfea05d navigate march: Fix "same mob type" and "not on sale" checks
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>
2015-03-02 08:20:47 +01:00
797abf4c8f navigate march: Wipe mission and retreat orders less eagerly
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>
2015-03-02 08:20:47 +01:00
351bb852a3 march: Give up fortification only on actual move or sweep
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>
2015-03-02 08:20:04 +01:00
5c185a1507 subs: Factor lnd_may_mar() out of lnd_mar()
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>
2015-02-28 16:21:35 +01:00
2a23660e68 subs: Factor shp_may_nav() out of shp_nav()
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>
2015-02-28 16:21:35 +01:00
bbacc28606 subs: Clean up direction code range checks
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>
2015-02-28 16:21:35 +01:00
2294785412 bomb fire launch torpedo: Don't disclose ship sinking in retreat
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>
2015-02-28 16:21:35 +01:00
f9316f71c4 torpedo: Fix mobility cost of retreat after hit
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>
2015-02-28 16:21:35 +01:00
b14f5276ab Update copyright notice
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-02-28 16:21:34 +01:00
0b40a88a37 Update known contributors comments
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-02-28 16:21:34 +01:00
ccf6504d1b empmod: Fix usage string of retreat and lretreat
Has been wrong since the commands were added in Chainsaw.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-02-28 16:21:34 +01:00
4e0d02b359 info/retreat info/lretreat: Fix and clean up
.SY claims all arguments are individually optional.  Fix that, and
clarify that omitting the optional arguments shows current orders.

Don't complicate syntax with <SECTS>, <SHIP/FLEET> covers sectors.

The two pages are identical apart from header and footer.  They
mention land units briefly, then explain ship retreat.  Lazy.  Drop
the land unit mention from "info retreat", and rewrite "info lretreat"
for land units.

Update example output for current code.

Don't shout retreat conditions.  We've always accepted both lower and
upper case conditions.  Help has been advertising lower case since
commit bb5dfd8, v4.3.16.

While there, fix a few misspellings and such.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-02-28 16:21:34 +01:00
75a049515b info/lretreat: Resync with retreat.t
Commit 18ee9a2f (v4.2.21) and commit 18ee9a2f (v4.3.16) updated only
info/retreat.t, and missed info/lretreat.t.

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-02-28 16:21:34 +01:00
b4bdd658bd info/retreat: De-document retreat condition help
Documented in commit dc41544, v4.3.16.  It actually worked only at the
condition prompt then.  No longer recognized elsewhere since commit
c699949.  The documentation is now misleading.  Simply drop it; the
prompt points out how to get help,

Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
2015-02-28 16:21:34 +01:00