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>
When a player moves more than 1023 sectors in a single navigate
command, we overrun the buffer holding the path taken. Remote hole,
but it requires a ship that can go that far, and even a ship with
speed 1000 would need a tech level well in excess of 1000 for that.
Thus, the hole is purely theoretical for even remotely sane game
configurations.
First known version with the flaw is 4.0.0.
Fix by going back the older behavior: don't print the total path
taken, but do print what the path finder does. Context diff of an
example:
[0:634] Command : nav 3 6,0
Flagship is od oil derrick (#3)
+Using path 'n'
h =
k . .
j d
<67.2:67.2: 6,0> h
od oil derrick (#3) stopped at 6,0
-Path taken: n
This is how march works.
Removes the only use of shp_nav_one_sector()'s unusual return value 2.
Return 1 instead.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Does not cover scattered navigate and march, RAILWAYS 0, enemy action
while sitting at the prompt, and interdiction.
The test exposes bugs. They're marked "BUG:" in the test input.
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>
The code needs it first so it can store field values into the object
right away. Save the values instead, and store them when the row is
complete.
The improvement is hardly worth the trouble by itself, but it'll
simplify supporting keys consisting of multiple fields, like sector
xloc, yloc or realm cnum, realm, so we can omit rows in those tables,
too.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
We currently require all rows to be present for tables item, sect-chr,
infrastructure, sect, realm.
The first three make sense: the code hard-codes indexes for them, and
malfunctions when entries are blank, so we want to make it hard to
leave any blank by accident.
The last two don't: blank sectors and realms work fine. There, the
restriction is arbitrary. Drop it.
Sectors and realms still can't be omitted "in the middle" (can do that
only with an ID selector), but that's coming soon.
See also commit 4a4ec91.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Each part of a split table needs to supply rows for the same objects.
We currently require each part to name its objects explicitly, with an
object ID field, and don't support splitting tables that don't have
such IDs. These restrictions became arbitrary when commit 4e23c45
implemented checking each partial table supplies the same rows. Relax
them.
Affects tables sect, news, lost, realm, game, infrastructure.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
New helper run_and_cmp to automate the job: run a program capturing
its standard output, standard error and exit status, then compare the
actual with the expected results.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When cmp_out detects a test failure, it returns non-zero status, which
terminates the test script unless caught.
Accumulate failures in a global variable checked at exit instead, so
running it multiple times in a test script just works.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Feeding other test output files (right now tests/files/files.out
tests/fairland/fairland.out fairland/newcap_script) works, but it's
odd, and might not work as well for all future output files. Only
strip trailing white space there.
There used to be other output files that required normalize.pl: client
output. Gone since commit 9ca3fa9.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Its value has always been "", unless the deity recompiled with
RESOLVE_IPADDRESS defined, or imported another value with empdump.
I'm going to drop the RESOLVE_IPADDRESS code, including struct natstr
member nat_hostname. To prepare for that, fix the selector's value to
"", so it doesn't need nat_hostname, and deprecate it.
This changes its "len" in xdump meta nat from 512 to 0. Unlikely to
upset clients.
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>
"diff -q" isn't blessed by POSIX anyway. Neither is -u, but it should
be widely available. -c is blessed, but I find its output hard to
read.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Now covered in tests/bridgefall. Damage perturbed, because deleting
the bridges from the setup makes defend() roll fewer dice.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Sector 0,2 takes a lot of damage. When it gets shot to deity, the
rest of the test can be upset. Avoid by putting more civilians there.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Copying the ship copies the ship to follow. When the source ship
doesn't follow a ship, the target ship is made to follow the source.
Screwed up since Chainsaw added the means to copy a ship. Fix it.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Copying the sector copies its distribution center. When the source
sector has none, the target sector is made to distribute to the
source. Unexpected. Zap the distribution center then.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Setup uses edit to build units. Stupid idea, because that misses unit
initialization normally done by build, directly or via
unit_wipe_orders(). Use build instead.
Changes test output harmlessly: ship xbuilt, ybuilt go from 0,0 to the
building sector, ship#0's builder goes from 98 to 0, all ships'
cargostart and cargoend go from 0 to -1, jhb range from 0 to 35, and
land unit retreat percentage from 0 to 42.
Setup no longer needs country 98. Drop it.
Setup no longer copies 2,0 to 0,0 messing up its distribution center.
Harmless.
Setup doesn't need POGO's tech level anymore, so don't set it to 400.
Nukes are now built at their required tech level. Also harmless.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
final.xdump changes, because setup no longer copies 3,-1 to 1,-1,
messing up its distribution center. Harmless.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
