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.
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.
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_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.
Spy units are now enabled when a land unit type with capability spy
exists. To disable them, deities have to customize table land-chr.
Before, spy units types were ignored when option LANDSPIES was
disabled. Except for xdump land-chr, which happily dumped unusable
spy unit types.
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.
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.
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.
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.
Replaces commit 5750107b, v4.3.15.
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().
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.
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.
getstarg(), snxtitem() and snxtsct() can yield the processor, because
they call getstring(). But only for null or empty arguments. For
other arguments, we should call ef_make_stale(), to catch errors.
Problem: if a caller never passes null or empty arguments, it may rely
on these functions not yielding. We'd get false positives. In
general, we can't know whether that's the case. But we do know in the
common special case of player arguments. Call ef_make_stale() for
those.
Split with parse() and pass first two arguments instead of the raw
tail to the map() callback. Advantages:
* Consistent with do_unit_move().
* Does the right thing when the tail is just spaces. Before, the
spaces got passed to the map() callback, which complained about
syntax. Now, they are ignored. This is what the commit I just
reverted tried to fix.
* Works better when the tail splits into more than two arguments.
Except for explore_map(), which ignores the argument(s), the map()
callbacks use display_region_map(), which split the tail at the
first space, and complained about any spaces in the second part.
Now, display_region_map() takes two argument strings instead of a
single, unsplit argument string, and extra arguments get silently
ignored, as usual.
It misuses snxtsct() and snxtitem() to find out whether the first
argument looks like sectors or like ships, which doesn't work with a
bad conditional argument.
Not worth fixing now; it's been disabled since 4.0.1, and broken at
least since commit 2fc1e74a (v4.3.0) broke its sector/ship
disambiguation via third argument.
snxtsct() and snxtitem() fail when the condition argument is bad.
satmap() didn't check for failure. Due to the way snxtsct() and
snxtitem() work, bad condition arguments were reported and otherwise
ignored.
Redundant information, but incredibly useful when you want to figure
out what happened without a (still nonexistent) journal replay tool.
The redundancy could help making a journal replay tool more robust.
To enable, set econfig key keep_journal to at least 2. Output events
are *not* flushed to disk immediately.
A deity can easily break BRIDGETOWERS by reducing etu_per_update
without compensating customization of buil_tower_bh,
rollover_avail_max or bridge span maxpop.
Document the issue in output of pconfig (which is installed as
$prefix/etc/empire/econfig).
To increase the chance deities actually read the documentation,
disable BRIDGETOWERS.
bestownedpath() is a rather simple-minded breadth-first search. It's
slower than the new path finder, and maintaining it in addition to the
new path finder makes no sense.
This gets rid of the memory leak mentioned in the previous commit.
To get rid of the buffer overruns for long paths mentioned in the
previous commit, make BestLandPath() fail when path length exceeds
1023 characters.
assemble_dist_paths() and move_ground() pass buffers with a different
size. Eliminate assemble_dist_paths()'s buffer. Update now works
regardless of distribution distance (the distribute command still
limits to 1023, to be fixed in a later commit). Enlarge
move_ground()'s buffers. Doubles the length of paths accepted by
explore, move, and transport.
I use two test cases to benchmark the path finders: "continental" (Hvy
Metal 2 updates) and "island" (Hvy Plastic 2 updates).
The new path finder runs my tests around 3-4 times faster than the old
A* without its caches. That's enough to meet its cached performance
for "island", but it's only half as fast for "continental". Not for
long; big speedups are coming.
We've been using Phil Lapsley's A* library to find land paths since
Chainsaw 3. It's reasonably general, and uses relatively complex data
structures to conserve memory. Unfortunately, it occasionally leaks a
bit of memory (see commit 86a187c0), and is unsafe for long paths (see
commit e30dc417).
To speed it up, v4.2.2 added two caches: the neighbor cache and the
path cache.
The neighbor cache attempts to speed up lookup of adjacent sectors.
It allocates 6 pointers per sector for that. In my tests, this is
more, sometimes much more memory than the A* library uses. See commit
7edcd3ea on branch old-astar for its (modest) performance impact.
The path cache attempts to speed up the update's computation of
distribution path costs. There, A* runs many times. Each run finds
many shortest paths, of which only the one asked for is returned. The
path cache saves all of them, so that when one of them is needed
later, we can get it from the path cache instead of running A* again.
The cache is quite effective, but a bit of a memory hog (see commit
a02d3e9f on branch old-astar).
I'm pretty sure I could speed up the path cache even more by reducing
its excessive memory consumption --- why store paths when we're only
interested in cost? But that's a bad idea, because the path cache
itself is a bad idea.
Finding many shortest paths from the same source has a well-known
efficient and simple solution: Dijkstra's algorithm[*].
A* is an extension of Dijkstra's algorithm. It computes a *single*
path faster than Dijkstra's. But it can't compute *many* shortest
paths from the same source as efficiently as Dijkstra's.
I could try to modify Phil's code to make it compute many shortest
paths from the same source efficiently: turn A* into its special case
Dijkstra's algorithm (at least for distribution path assembly), then
generalize it to the many paths problem. Of course, I'd also have to
track down its memory allocation bugs, and make it safe for long
paths.
Instead, I'm replacing it. This commit is the first step: a rather
unsophisticated implementation of Dijkstra's algorithm specialized to
hex maps. It works with simple data structures: an array for the hex
map (16 bytes per sector), and a binary heap for the priority queue
(16 bytes per sector, most of it never touched). This is more memory
than Phil's A* uses, but much less than Phil's A* with v4.2.2's
caches.
[*] To fully exploit Dijkstra's "many paths" capability, we need to
compute distribution paths in distribution center order.
Why upgrade? I'm not a lawyer, but here's my take on the differences
to version 2:
* Software patents: better protection against abuse of patents to
prevent users from exercising the rights under the GPL. I doubt
we'll get hit with a patent suit, but it's a good move just on
general principles.
* License compatibility: compatible with more free licenses, i.e. can
"steal" more free software for use in Empire. I don't expect to steal
much, but it's nice to have the option.
* Definition of "source code": modernization of some details for today's
networked world, to make it easier to distribute the software. Not
really relevant to us now, as we normally distribute full source code.
* Tivoization: this is about putting GPL-licensed software in hardware,
then make the hardware refuse to run modified software. "Neat" trick
to effectively deny its users their rights under the GPL. Abuse was
"pioneered" by TiVo (popular digital video recorders). GPLv3 forbids
it. Unlikely to become a problem for us.
* Internationalization: more careful wording, to harden the license
outside the US. The lawyers tell us it better be done that way.
* License violations: friendlier way to deal with license violations.
This has come out of past experience enforcing the GPL.
* Additional permissions: Probably not relevant to us.
Also include myself in the list of principal authors.
