Move clear_telegram_is_new() call to beginning of update
The call was added in 4.2.5 "so that the next telegram is flagged as
new and not part of the update". Since the update sends only
TEL_UPDATE telegrams (the previous commit restored that property), and
nothing else does, the next telegram is flagged as new automatically,
except when it's from the next update. Document that, and move the
call to a more natural place.
Ensure the update's production report isn't split by bulletins
During the update, wu() sends TEL_UPDATE telegrams ("Production
Report") instead of TEL_BULLETIN telegrams, but typed_wu() has no such
logic. It's used by tele(), which doesn't run during the update, and
mpr(), which may, e.g. called from ship or mission code used by
autonav or sail. This inserts bulletins in the middle of the
production report, splitting it apart.
Happens since mpr() was added in Empire 2. Before, only tele() used
typed_wu() directly, and everything else wu().
Leave nat_ann, nat_tgm alone and return -1 on all errors. Before,
only failed open was handled that way. Failed write and close were
logged and ignored. While there, improve the log messages a bit.
Note: the return value fix has little effect. It makes tele() log the
failure, which is redundant. Everything else goes through wu() and
ignores the value.
Make add require confirmation for unadvisable actions
Deleting a country in state STAT_SANCT, STAT_ACTIVE or STAT_GOD is
risky, because any references to this country become dangling, which
makes ef_verify() unhappy. For a reason: we may well have code that
isn't prepared for dangling references, and breaks.
Replacing a country that is being used is risky, because it can get us
into weird states. For instance, replacing a player by a visitor can
result in a visitor that owns stuff.
The add command sets nation flags NF_FLASH, NF_BEEP, NF_COASTWATCH,
NF_SONAR, and since 4.2.6 NF_TECHLISTS. POGO is created by the files
utility, which sets only NF_FLASH (since 4.2.2). Change files to
match add.
Before, add reset the country only when adding a player or a visitor.
When adding a deity or deleting a country, it set just nat_cnam,
nat_pnam and nat_state. Has always been that way.
Because of that, a newly minted deity country could inherit all kinds
of crap from a previous user of its country number: origin, realms,
relations, telegrams, ... Harmless if the country number has never
been used before, which is how add is generally used.
When adding a deity country, initial levels (start_education, ...) now
apply, relations start NEUTRAL instead of AT_WAR, and the usual
initial nation flags are set.
Reset on delete as well, just to get rid of the special case.
Argument "active" is obscure. It creates a country in STAT_ACTIVE
that doesn't have a capital, and has its origin at the true origin.
If you really want such a country, create it in STAT_NEW normally,
then use edit to go to STAT_ACTIVE.
0 <= fd < FD_SETSIZE must hold, or else undefined behavior in FD_SET()
and buffer overrun in LwpFdwait[fd]. Check of upper bound off by one,
check of lower bound missing.
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.
projects / empserver / log