sell used to search multiple sectors for sellable commodities, keeping
tally in totalcom. It failed with message "No eligible" when none
could be found.
sell's second argument got changed to a single sector in Empire 3. If
the sector can't sell, we return early. Else, totalcom is positive.
Thus, the "No eligible" code is unreachable. Remove it.
POGO can navigate dead ships, and march dead land units. The ghosts
even get sighted and interdicted, and can hit mines (landmines only
until commit fe372539, v4.3.27). Noted for ships in commit 9100af0b.
Has always been broken. Fix by making shp_sel() and lnd_sel()
explicitly reject ghosts.
Same code pattern also exists in pln_sel, but dead plains fail the
efficiency test, so it's harmless there. Apply the same fix anyway.
Fix "sector can board" test to ignore land units on land units
boar() lets a sector board if it has mobility or usable land units.
Embarked land units are not usable. But it tests only "on ship", not
"on land unit". Broken in 4.0.17.
Fix transport to reject planes loaded on land units
The transported plane remains on its carrier. When the land unit
moves, the plane is teleported right back to it. Broken since
Chainsaw 3 added land units.
While there, improve the message for planes on ships.
Fix prewrite callbacks' cargo list update for in-place update
When updating in-place (old==new), we must not write through new
before we're done reading the same memory through old.
Bug: we write the carrier uids too early. Cargo lists aren't updated
when a carrier dies in an in-place update. No such updates are known.
Broken in commit 64a53c90, v4.3.17.
They didn't since commit 93d033cf, v4.3.26. Drawback: micromanagement
incentive to unload them for the update. Similar incentive has always
existed for military on ships.
Since the previous commit, land units loaded on land units get
unloaded when the carrier dies fighting che. Such land units get
stuck in the sector if the take over, and can be boarded. Doesn't
feel right, and increases the micromanagement incentive. Avoid by
letting them fight.
Don't leave cargo stuck on land unit killed by che
When che destroy a land unit, any embarked units remain stuck on their
now dead carrier. Closely related to and same impact as the bug fixed
in commit 8ccad0d7. Broken since Chainsaw 3 added land units.
The obvious fix would be to match what normally happens when a carrier
gets destroyed: destroy the cargo. Requires recursion. To keep
things as simple as possible, destroy plane cargo, but unload land
unit cargo. That way, the only cargo of cargo to visit are nukes on
planes.
Unloading the land units creates another problem, which will be
addressed in the next commit.
Damage due to lack of maintenance is now limited by the unit's minimum
efficiency.
Before, units could die. Unfortunately, the update left any embarked
units on their dead carrier. Should have seen this when I fixed a
related bug in commit c2c0d1ff, v4.3.22. Broken for ships and land
units when Empire 2 added their maintenance cost, and for planes when
commit 2e40a4bb (v4.3.4) replaced nuclear stockpiles by nuke units.
The common root cause of these bugs is the update bypassing pre-write
functions (bug#1010856).
If another unit with the same number got built, it picked up the stuck
cargo, triggering the oops from commit 6fb5caf6, which see.
In "stuck on dead carrier" state, units pretty much behave as if their
carrier was still alive, with additional protection from the fact that
a dead carrier can't be damaged or boarded.
The server detects this state on startup since commit 7da9aab5, and
refuses to start.
Oops when stuck cargo snaps to new ship, plane or land unit
When units somehow get stuck on a dead carrier, a new build reusing
the dead carrier's UID picks up its cargo. The cargo gets teleported
to its new carrier when the carrier moves.
Oops when a ship, plane or land unit is created with cargo. To
recover, destroy the cargo.
Avoid false positive generation oops in navigate and march
Commit e3cf1e32 (v4.3.27) created make_stale_if_command_arg() to
permit catching more potential yields on input. Unfortunately, the
implementation of navigate and march sub-commands 'r', 'l' and 's'
breaks it.
do_unit_move() reads units into a unit list at the beginning and at
each stop. It writes them back when they move or sweep. If a unit
changed in the file in between, the changes would get wiped out.
Therefore, do_unit_move() must not yield between stops.
do_unit_move() parses sub-commands into player->argp[], then supplies
defaults for missing arguments, so that code using them (radar(),
do_look(), sona(), mine(), landmine()) won't prompt for missing
arguments. Unclean and brittle. See also commit 28cc236e and commit 45106ab9.
Unfortunately, make_stale_if_command_arg() doesn't recognize the
difference between these defaulted arguments and parsed arguments, so
it makes objects stale, even though the defaulted arguments can't be
missing. If a move or sweep follows, it triggers a false positive
generation oops.
To fix, test "points into argument buffer" (only true for parsed
arguments) instead of "is in player->argp[]". Requires making the
argument buffer accessible: new struct player member argbuf[]. Use it
for parsing commands, in command(), execute(), do_unit_move(). Don't
use it in emp_config(), player_login(), move_ground(), because these
parse something else.
Fix client redirection and execute for non-ASCII characters
The client rejects unauthorized redirection and execute. Its
authorized check always fails for arguments with non-ASCII characters.
The culprit is ring_search(): It compares plain char to unsigned char,
which breaks when char is signed. Believed to be broken in commit 8b7d0b91, v4.3.11.
Note that non-ASCII characters only work in UTF-8 sessions. In ASCII
sessions, the server replaces them, and the authorized check fails.
Works as designed.
Fix execute not to mangle the argument when it prompts for it
The argument is UTF-8. If it's missing, execute() gets it with
getstring(), which mangles non-ASCII characters. Clients reject
mangled file names for security reasons.
Fix by switching to ugetstring(). Missed in commit 69832255, v4.3.11.
Non-ASCII characters are still mangled in ASCII sessions, but that's
not expected to work.
Fix execute to filter input just like the main command loop
When we added input filtering in v4.2.21, we missed the execute
command. Because of that, funny characters can be treated differently
in batch files.
The main command loop uses getcommand(), which filters input.
execute() uses recvclient() directly, and doesn't filter input. Both
feed the command to parse(), which drops unwanted control and
non-ASCII characters.
Input filtering drops unwanted control characters and, for ASCII
sessions, replaces non-ASCII characters by '?'.
Because of that, execute in ASCII sessions drops non-ASCII command
characters in batch files rather than replacing them.
Except where parts of the command are used raw: execute's command
echo, flash and wall message argument. There, unwanted control
characters aren't dropped in UTF-8 sessions, and non-ASCII characters
are interpreted as UTF-8 in ASCII sessions. Output filtering replaces
any resulting non-ASCII characters.
players column "last command" also uses the command raw, but commands
executed from batch files are not visible there, so it isn't affected.
To get rid of the differences, move the prompting from getcommand() to
command(), then switch execute() over to getcommand().
Side effect: the batch file's commands are now recorded in
player_commands[]. That's desirable.
copy_utf8_to_ascii_no_funny() eats the character following a replaced
non-ASCII character. Buffer overrun possible when the terminating
zero gets eaten. Broken in commit b5ff7e3b, v4.2.21.
Affected commands:
* players column "last command" in ASCII sessions: struct player
member combuf is UTF-8, uprnf() filters to ASCII.
* read in ASCII sessions: telegram chunks are UTF-8, uprnf() filters
to ASCII.
* flash and wall with message argument in ASCII sessions: argument is
used raw, i.e. UTF-8, pr_flash() filters to ASCII. Safe as long as
we have input filtering sanitizing the raw argument. command() does
that, but execute() doesn't (bug, to be fixed in a later commit).
* execute prompting for its argument in UTF-8 sessions: prmptrd()
receives user text, and filters to ASCII.
Unaffected:
* dispatch() argument redir is UTF-8, uprnf() can filter to ASCII.
Safe as long as we have input filtering sanitizing the raw argument.
command() does that. execute() doesn't, but rejects redirections
before calling dispatch().
* getele() buffer is UTF-8, uprnf() can filter to ASCII. Safe,
because its contents comes from uprmptrd(), which filters input.
Fix xundump's "value must match" check for split tables
The check applies to selectors with flag NSC_CONST set. It permits
initializing them in new objects, but prevents changing them in
existing objects. For split tables, initialization worked only in the
first part, because new objects were considered old in later parts.
For instance, in a custom config sect-chr with mnem in the second
part's field 2, new sector types were rejected with `Value for field 2
must be ""'.
"version" is a normal table since commit da8a1dae, v4.3.12. xdump.pl
wasn't updated for that, and queried the version table twice. When
the deprecated special "xdump ver" was removed in commit 78b3af20
(v4.3.27), the extra query broke. Remove it.
Fix empdump not to touch plane file when import fails
pln_zap_transient_flags() fixes up planes stuck in the air (commit 7ca4f412, v4.3.12). Since commit 4e9e58bf (v4.3.14), it writes back
the fixed planes. This is wrong for empdump.
empdump should touch data only on successful import. When it fails
because ef_verify() fails, and any planes are found stuck in the air,
the plane file gets rewritten.
Make parameter ef_verify() take parameter may_put to let empdump
suppress the plane write-back. The plane file still get written out
on successful import, along with the other imported game state.
Verify game state and configuration reference sanity
Table elements reference other table elements. Bad things happen when
references dangle. ef_verify() already checks whether the referenced
table elements exist. This commit makes it check whether the elements
are "in use". This catches stuff like living planes on dead carriers.
verify_row() refrains from rejecting zero uids, because some tables
may contain blank entries, with zero uid.
Change it to check only header sanity for entries that are not in use.
This filters out all legitimately blank entries. Tighten up the uid
check.
For computing "in use", factor empobj_in_use() out of xdvisible().
Note that xdvisible()'s case EF_COUNTRY doesn't bother to check
nat_stat, because that's implied by what it does check. It's not
implied in empobj_in_use(), so add it there.
Trade ships are now enabled when a ship type with capability trade
exists. No such type exists by default; to enable trade ships,
deities have to customize table ship-chr.
Before, trade ship types were ignored when option TRADESHIPS was
disabled. Except for xdump ship-chr, which happily dumped unusable
trade ship types.
Only trade ships can be auto-scuttled. orde() rejects scuttle orders
for other ships. scuttle_it() double-checks, but gets the test wrong:
it rejects only when opt_TRADESHIPS is enabled. Fix that. While
there, make it oops on inadmissible ships.
Fix how configuration tables and empdump treat omitted entries
Change xundump to blank out omitted rows. Before, they were left
alone. Impact:
* No change for reading builtin tables.
* Reading custom tables now replaces the builtin tables instead of
sort-of-merging them. Wasn't a real merge, because it dropped
builtin entries after the last custom entry, except for
non-truncatable tables item, sect-chr and infrastructure.
* empdump -i now replaces the old state instead of sort-of-merging the
dump into the old state. Wasn't a real merge, because it dropped
old state entries after the last entry in the dump, except for the
fixed-size tables sect, nat, realm and game.
Don't misinterpret blank configuration entries as sentinels
Configuration table entries not defined by builtin and custom
configuration files remain blank. They get misinterpreted as sentinel
in tables that use one. Affected are tables product, ship-chr,
plane-chr, land-chr and nuke-chr. Tables item, sect-chr and
infrastructure are immune despite using a sentinel, because omitting
entries is not permitted there.
Code relying on the sentinel fails to pick up entries after the first
blank one. They don't get set up correctly, they're invisible to
build and show, and not recognized as symbolic selector values (the
frg in ship ?type=frg). xdump is fine, because it doesn't rely on
sentinels. It dumps blank entries normally.
The bugs don't bite in the stock game, because the builtin
configuration files are all dense.
The sentinels are all null strings. Set them to "" in the affected
tables' oninit callback. Fix up code iterating over the tables to
ignore such entries. This is precisely the code relying on sentinels,
plus xdump's xdvisible().
Permit omitting rows at the end of tables nat and game
When not enough rows are supplied for a table with fixed size, treat
the rows missing at the end just like rows omitted elsewhere: make
them blank if the omission is permitted (tables nat and game), else
fail (tables sect and realm; no change).
Forbid omitting rows for tables with const fields: item and sect-chr.
This is consistent with the rule for truncation.
The server expects certain entries in these two tables, and
malfunctions when they're blank. Omitting them in the builtin tables
has always left them blank, but deities are not supposed to edit them,
and maintainers are supposed to know what they're doing, so the issue
was deemed unimportant and ignored. However, I plan to blank out
omitted rows in custom tables as well, and then the issue isn't
unimportant anymore.
Truncating is actually fine for table product. It's forbidden because
selector sname has flag NSC_CONST set. I don't remember why sname was
made const in commit 445dfec9 along with item selector mnem, sect-chr
selector mnem and infrastructure selector name. Unlike the other
tables, no code depends on product's builtin values. Clear the flag.
Overhaul how xundump keeps track of current object
Factor tracking of cur_type, cur_obj, cur_id and cur_is_blank into a
set of functions. They replace getobj(). While there, improve some
error messages.
Move pln_oninit(), lnd_oninit(), nuk_oninit() to filetable.c
They set up invariants, and thus should be always active, not just in
the server. Since ef_blank() isn't used for these files outside the
server right now, this isn't a bug fix, just cleanup.
Move file initialization from files.c to empfile oninit()
files.c writes initial contents to game state files with fixed size.
Necessary for setting up invariants, such as struct sctstr members
sct_x, sct_y matching sct_uid.
Do that from the oninit() callback, so ef_blank() sets up invariants
correctly. Since ef_blank() isn't used for these files right now,
this isn't a bug fix, just cleanup.
Instead of stuffing NULL initializers into the cache initializer
macros UNMAPPED_CACHE(), ARRAY_CACHE(), PTR_CACHE(), ARRAY_TABLE().
In preparation of having some non-NULL initializers.
Fix empdump not to grow game state files with fixed size
empdump -i now complains about extra rows instead of silently growing
the file to a size the server will reject. Affects tables sector,
nation, realms, game.
Bonus fix: better error message on I/O error or insufficient memory.
Fix empdump not to truncate game state files with fixed size
empdump -i now complains about missing rows instead of silently
truncating the file to a size the server will reject. Affects tables
sector, nation, realms, game.
New struct empfile member nent replaces ef_open() parameter nelt.
Cleaner, because the expected size is a property of the file, not of
how it's used. Also fixes empdump to check file sizes.
Complication: with EFF_CREATE, ef_open() creates an empty file, to be
extended to the correct size. Callers passed nelt argument -1 along
with EFF_CREATE, to make ef_open() accept the empty file. Can't do
the same for empfile member nent. Instead, make ef_open() not check
the (zero) size then.
ef_verify() assumes views are open. Bug is harmless, because
ef_nelem() returns zero for closed views, and ef_verify() accesses
only immutable parts of struct empfile then.
New struct empfile member base replaces ef_open_view() parameter base.
Cleaner, because the base table is a property of the view, not of how
it's used.
Use it to clean up verify_fail()'s base table access, and for extra
sanity checks in ef_open() and ef_open_view().
Change struct empfile callback onresize() to return void
ef_open() handles onresize() failing incorrectly. Instead of fixing
that, drop the failure mode. It's not really used: unit_onresize()
fails only when used incorrectly. It isn't. If it ever is, ignoring
the failure is safe.
Make ef_close() clear them always, even for views. Harmless, as
ef_open_view() always sets the same flags.
Drop redundant assignment to ep->flags in ef_open(). ef_close()
clears mutable flags, no need to clear them some more right before
calling it. Missed in commit 3eb3607f, v4.3.0.
ef_open() and ef_close() clear EFF_SENTINEL because of that. Broken
since commit 1492845c (v4.3.17) added EFF_SENTINEL. Harmless, as no
file-backed table has a sentinel.
Fix ef_open()'s error message for file larger than static cache
The message claims the file is larger than it actually is. Broken
since commit 71908018 (v4.3.0) implemented static cache in ef_open().
Harmless, because no file-backed table has a statically allocated
cache.
Fix how empdump rejects attempt to split table sect
Split tables require the record index in the leftmost column.
defellipsis() correctly rejects "..." when the table doesn't have one.
It fails to reject it when it has one that is NSC_EXTRA, and thus not
permitted in a dump. This is the case for table sect. defellipsis()
happily succeeds, then chkflds() demands column "uid" if it's missing,
and rejects it if its present.
It missed those with more flags than just NSC_EXTRA set: table sect
name uid, table nat names passwd, xorg, yorg, contacts, rejects.
Since xundump() doesn't provide space for these, the bug could lead to
buffer overruns. Fixes flawed commit 726a8e3d, v4.3.12.
xubody() neglected to check ef_truncate()'s return value. Two failure
modes: invalid arguments, and ftruncate() failure. The former
shouldn't happen, and the latter can happen only for file-backed
tables, hence only in empdump -i.
Tables with a file name are: any game state, and any table that's
initialized from a .config file.
Tables that are no longer customizable: "updates" (customization had
no effect, because update_get_schedule() overwrote it), "table",
"meta" and the symbol tables (customization couldn't change them
anyway), and news-chr (customizing r_newsstory[] was kind of neat, but
unsafe because they are format strings for sprintf()).
"xdump updates" believes there are always 15 (UPDATE_TIME_LEN - 1)
scheduled updates. When fewer than 15 updates are scheduled, it shows
whatever crap update time happens to be in the unused part of
update_time[]: the initial zero or a previously scheduled update.
Root cause is that table EF_UPDATES has always UPDATE_TIME_LEN - 1
entries, which is incorrect when fewer updates are scheduled. Only
xdump is affected, as the other users ignore the length and stop at
the sentinel.
Fix update_get_schedule() to resize table EF_UPDATES.
projects / empserver / log