Failing a command with code RET_SYN prints help and doesn't charge
BTUs. Failing with code RET_FAIL doesn't print help and charges BTUs.
A couple of command failures were changed or added recently to fail
with RET_SYN, because they're due to invalid player input. Some of
them, however, can happen after the command already did something, so
BTUs must be charged, or else players can deliberately fail the
command to save BTUs:
* Commit 9eda5f87 adds RET_SYN failures when getting player input
fails for:
- arm third argument
- deliver fourth argument
- fire third argument
- lmine second argument
- order d fourth argument
- range second argument
- sail second argument
- tend third argument
- designate second argument
- morale second argument
- set third argument
- tend fourth argument
* Commit d000bf92 likewise (with a bogus commit message) for bdes
second argument.
* Commit 9f4ce71a likewise for ltend third and fourth argument.
* Commit 9031b03b changes failure code from RET_FAIL when getting
player input fails for threshold third argument. It adds RET_SYN
failure when the argument is bad. Some bad arguments already failed
that way before.
* Commit a7cf69af changes it from RET_FAIL when designate second
argument is bad.
Change them all to fail with RET_FAIL.
Many other places have the same bug, but those are left for another
day.
Improve work()'s message when sector doesn't need construction
The cheesy test for repeated messages broke down when working on more
than one sector. Reword the message so that repetition is fine, and
drop the test.
Don't permit convert to spend more money than available
Complicated by the fact that conv() ran the conversion code twice,
first for adding up the cost for chkmoney(), then for actually
converting. chkmoney() asks the player to confirm when he's about to
spend more than half his cash. Get rid of that, not worth the
complexity. This merges do_conv() back into conv().
Don't permit demobilize to spend more money than available
Complicated by the fact that demo() ran the demobilization code twice,
first for adding up the cost for chkmoney(), then for actually
demobilizing. chkmoney() asks the player to confirm when he's about
to spend more than half his cash. Get rid of that, not worth the
complexity. This merges do_demo() back into demo().
It also removes the command's virtually undocumented fourth argument.
Update player_coms[] accordingly. While there, make it require money;
it won't do anything useful without money anyway.
RET_SYS was used for commands failing due to internal or environmental
errors, but not really systematically. The difference to RET_FAIL is
how dispatch() treats them: RET_SYS got logged, and cost no BTUs.
More specific logging is possible at the point of failure than in
dispatch(). Make sure that's done for all failures that used to
return RET_SYS.
The change in BTU charging affects commands consider, offer, repay,
trade failing due to internal errors. It also affects deity commands
reload and turn (irrelevant because deities get unlimited BTUs), and
commands apropos, info and motd (irrelevant because they cost no
BTUs).
Behave like plane_bomb() and land_bomb(): deal with leading whitespace
and signs in the input, print a message when asked to bomb a ship that
is not there.
Disable collateral damage for automatically launched missiles
4.0.17 made missiles that miss their target do collateral damage to
the target sector. The code didn't work: it did collateral damage
even when the launch failed or the missile got intercepted. 4.0.18
disabled it again for the launch command (see also commit fa7e3aa9).
Disable it for automatically launched missiles, too, in
msl_launch_mindam().
The initial parts of struct loststr and struct empobj must match.
Commit 49780e2c screwed that up for members lost_uid/uid, which also
broke the equivalence of lost_owner/own. Since lost_uid is not used,
the former had no effect. But the latter broke xdvisible(). Could
make xdump lost leak information.
Commit a680c811 reorderd struct loststr members to make lost_timestamp
equivalent to new struct empobj member timestamp, but failed due to
the bug in commit 49780e2c. Commit f33b96b1 then set the timestamp
through empobj, which screwed up timestamps in lostitems, i.e. it
broke incremental xdump lost.
All of the above is in v4.3.12.
Commit 536ef0b0 (v4.3.15) added lost_seqno / seqno. No effect,
because only seqno is used.
This is the human-readable buddy of xdump product, which dumps pchr[].
It duplicates much of the information in show sect c, but in more
accessible form. It's in show_product().
Remove product information from info Quick-ref. show reflects the
actual game, is more complete, and should be just as readable.
Show packing type instead of selected bonuses in show se s
show_sect_stats() used to show packing bonuses for military,
uncompensated workers, civilians, bars and lcms. The bonus for lcms
was labelled "other", which was okay only as long as all other
commodities receive the same packing bonuses.
Show packing bonus type instead. The actual bonus for each commodity
and packing type is already shown by show item. But change its header
for the NPKG column from "rg" to "no", because packing type NPKG is
shown by show se s as "normal".
show_sect_build() showed only sectors with unusual build
characteristics. Which begged the question what the usual build
characteristics are. Add a line to show them.
Fix extra prompt after abort due to misuse of snxtitem()
The old code used getstarg() to get an argument with a different
prompt than snxtitem() uses, then passed the value to snxtitem()
unchecked. If the player aborts, getstarg() returns a null pointer,
and snxtitem() prompts again. Affected:
* load/lload plane/land third argument; load_plane_ship(),
load_land_ship(), load_plane_land(), load_land_land()
* bomb, drop, fly, paradrop, recon and sweep second argument;
get_planes()
* tend and ltend second and fourth argument; ltend(), tend(),
tend_land()
* mission second argument; mission()
Fix by making snxtitem() taking a prompt argument, null pointer
requests the old prompt.
Use that to simplify multifire() and torp(). Change the other callers
to pass NULL.
Broken since commit 35db407d, v.4.3.12. v4.3.14 and v4.3.15 were
distributed with a stale sources.mk in the tarball. The latter does
not build out of the box because of that.
There were two checks meant to enforce positive numbers, both dating
back to the earliest known versions of the code, and both wrong.
The first one applied only to thresholds given with the command, not
ones prompted for, and it incorrectly rejected numbers starting with +
or prefixed by whitespace. Remove it.
The second one failed to reject negative numbers when prefixed by
whitespace. Fix.
Fortunately, the update doesn't conjure up stuff to satisfy negative
thresholds.
Clean up how desi() handles bad designation arguments
Fail the command when the designation isn't allowed for mortals, or
when the player can't afford it.
Treat '=' and '@' like the other designations not allowed for mortals,
not like invalid designations. Change failure for invalid designation
from RET_FAIL to RET_SYN.
Fix distribute aborting at prompt for second argument
Before failing the command, the old code attempted to change the
current sector's distribution center to the last one used, which might
have been uninitialized coordinates. If lucky, the coordinates were
invalid, and the attempt oopsed and did nothing.
Commit 79407e68 (v4.3.11) changed recvclient() to keep failing after
receiving EOF from player. This was bad, because some places getting
input check player->aborted instead of recvclient() failure, and
player->aborted wasn't set on EOF. Bugs caused by this:
* comm_bomb(), ship_bomb(), plane_bomb(), land_bomb() went into an
infinite loop that eventually ate all memory.
* deli(), desi(), dist(), fly(), morale(), zdon(), att_prompt(),
ask_move_in() interpreted EOF as empty input instead of no more
input.
Oops when player thread keeps reading input unsuccessfully
Reading input fails after EOF and while the current command is
aborted. Commands should detect that and fail. If a command neglects
to do that in a loop, the loop can become infinite. This is
especially bad after EOF, because then the client might not read
output anymore. Output gets buffered until memory runs out.
Mitigate such bugs by counting how many calls have failed in a row,
oopsing on the 256th, and sleeping one minute from the 256th on.
fly() reads the carrier, then passes it to pln_dropoff(), which writes
it back. fly() also calls pln_oneway_to_carrier_ok(), which updates
the carrier when its plane summary information is incorrect.
The old code called it between reading the carrier and passing it to
pln_dropoff(). This made pln_dropoff() wipe out the plane summary
update, and triggered a seqno mismatch oops. Broken by introduction
of pln_oneway_to_carrier_ok() in commit 1127762c, v4.2.17.
Fix by reading the carrier right before passing it to pln_dropoff().
Commit db1ac2ed (v4.3.6) introduced a separate territory for deities,
and made the territory command use it by default for deities. Deities
can still access the old default territory as territory number 0.
Selectors terr, terr0 (alias for terr), terr1, terr2, terr3 remained
unchanged, and a new selector dterr was created to let deities access
the deity territory.
Make selector terr virtual, so that it can do "the right thing", just
like the territory command: select territory 0 for mortals, and the
deity territory for deities.
Unfortunately, this requires us to switch xdump to use terr0 instead
of terr, because otherwise the deity xdump would contain the deity
territory twice, and territory 0 not at all.
Missile interdiction leaves behind used up missiles with the
PLN_LAUNCHED flag set. This can lead to a bogus warning from
pln_zap_transient_flags() on server restart.
Change pln_zap_transient_flags() to ignore dead planes.
The old code recognized group retreat only when the first argument was
on the command line. Didn't make a difference, because it was only
used when there were at least two arguments on the command line.
The old code relied on rflags being represented as two's complement.
When given an empty retreat path, the old code deleted the retreat
path and set the retreat flags normally. The new code deletes both.
Neither is nice; it should perhaps keep the retreat path and only set
the flags.
Fix flying commands not to let planes do double duty as escorts
Commit 7ca4f412 (v4.3.12) marked planes flying a sortie with
PLN_LAUNCHED, and made pln_arm() reject planes with that flag set.
This was designed to reject escorts that were already flying as
bombers. It didn't work, because the test for PLN_LAUNCHED used a
stale copy of the plane created by pln_sel(). Fix by getting a fresh
copy.
The bug always existed, but the botched fix in commit 7ca4f412 made it
worse. Before, ac_encounter() dropped escorts that were also bombers,
so the bug merely wasted plane fuel. After, such planes were
effectively duplicated, and damage to one of them, usually the bomber,
was wiped out. Abusable.
Fix seqno mismatch and use-after-free in shp_sweep()
The code wrote the swept sector after calling shp_check_one_mines().
This failed to use up the mine that hit the minesweeper, and triggered
a seqno mismatch oops.
The code wrote the minesweeper after calling shp_check_one_mines().
This used freed memory when the minesweeper got sunk there.
Broken in 4.0.17. Fix by moving both calls before
shp_check_one_mines().
Don't let non-light units board ships that can't carry them
ask_olist() let non-light land units board ships that can carry only
light units. If the board succeeds, the non-light unit move onto the
ship and then are stuck there.
Make spy() not skip sectors without civilians, military and land
units. There could be other interesting things to report there:
efficiency, gold bars, planes...
Remove columns lnd, pln from spy report to fix spy unit leak
The values in these columns were computed by count_sect_units() and
count_sect_planes(), which included land units and planes in the count
that aren't shown by prunits() and prplanes(), namely own and embarked
units. Confusing. Moreover, count_sect_planes() and prunits() rolled
dice separately for spy units. This could leak the presence of spies
even when prunits() didn't show them.
All fixable, but not worth the trouble; just remove the counts.
projects / empserver / log