Without ARG1, display_region_map() formats a rectangular area around
CURX,CURY, so it can use do_map(). Ugly, and duplicates some
unit_map() functionality.
Factor snxtsct_around() out of unit_map(). Inline do_map() into
display_region_map(), so we can use snxtsct_around().
Don't mess with player->condarg. Leftover from when we abused map()
here.
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.
parse_map_flags() silently truncates map flags after the first 't' or
'r'. This makes it accept arguments "true" and "revert". However, it
also breaks the perfectly sensible argument "ts", which should show
ships just like "st", but doesn't.
info bmap & friends document arguments "true" and "revert", and also
suggest flags 't' and 'r'. What a mess.
Make argument "revert" a special case. Deprecate flag 'r', and clean
up truncation there.
Don't truncate after flag 't'. If any bad flags follow, ignore
everything after 't', but deprecate that usage.
player->aborted gets set when we get an interrupt or EOF cookie from
the player, when update or shutdown abort commands, and when we abort
an attack (not relevant here).
The checks are useless: player interrupt and EOF are checked
elsewhere, and update/shutdown can run only when we yield the
processor, which we never do (output doesn't yield because C_MOD is
set).
It assumes snxtsct() fails only when the argument can't be parsed. It
can also fail when the condition argument has errors. `map # ?xxx'
first complains about xxx, then maps around ship#0. Broken since
Chainsaw 2 introduced smap, pmap and lmap.
Use sarg_type() to recognize sectors vs. unit argument. `map # ?xxx'
now fails as it should.
Subtle side effect: do_map() no longer prompts for argument "",
because snxtsct() is now guarded by sarg_type(). Impact on callers:
* display_region_map() is not affected, because it never passes "".
* map() passes on "" arguments. Change it to prompt in that case.
Consistent with how other commands behave. No functional change.
* do_unit_move() passes on "" arguments. Keep it that way. This
changes navigate and march sub-commands 'M' and 'B' not to prompt
for "" arguments, which is consistent with sub-command 'm' of move,
test and transport.
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.
World-sized bitmaps were allocated with size WORLD_SZ() / 8, which
expands to (WORLD_X * WORLD_Y / 2) / 8. The divisions truncate unless
WORLD_X * WORLD_Y is a multiple of 16. The bitmaps were one byte too
small then. Bitmap overruns happen when:
* A lookout looks at one of the last sectors of the sector file.
Besides commands look and llook, this affects navigate and march
sub-command 'l'.
* Command spy spies into one of the last sectors of the sector file.
* A map or nmap (but not a bmap) shows one of the last sectors of the
sector file, or a sector that can see one of the last sectors
(visual range is two sectors at 100% efficiency). Besides commands
lmap, map, nmap, pmap, smap, this affects move and transport
sub-command 'm'.
Diagnosed with valgrind.
Already broken in BSD Empire 1.1 (bitmaps were on the stack then).
Before, it overwrote '?', '.', ' ' in the bmap with the capitalized
country letter, but only for sectors the player owns. Pretty
harmless, just weird. It can't happen currently, because sharebmap
with self fails with "does not have friendly relations towards you".
Sub-command 'm' calls display_region_map() to display a map. The map
is centered on the current sector by default. It extended one sector
farther to the right and down than to the left and up. Odd, and
inconsistent with the map size used by unit_map() for navigate and
march sub-command 'M'. Fix that.
Change struct range from exclusive to inclusive upper bounds, for
consistency with struct realmstr and the area syntax. Also fix many
bugs.
real()'s conversion from struct range's exclusive upper bounds to
struct realmstr's inclusive upper bounds could underflow and store -1
in the realms file. Harmless, because its users didn't mind:
list_realm() and nstr_exec_val() convert back to relative coordinates,
and sarg_getrange() is only used by sarg_area(), which happened to
undo the damage. The change to inclusive upper bounds gets rid of the
broken conversion.
xyinrange() incorrectly treated the upper bound as inclusive, unless
the bounds were equal. Impact:
* nxtitem() and nxtitemp() cases NS_AREA and NS_DIST attempted to hack
around xyinrange()'s lossage(!), but screwed up: sectors on the
lower bound of of a range spanning the the whole world were skipped.
This affected all command arguments that support area or distance
syntax for items. In sufficiently small worlds, it could also make
radar miss satellites and ships, sonar miss ships, satellite miss
ships and land units, nuclear detonations miss ships, planes, land
units and nukes, automatic supply miss ship and land unit supply
sources, ships and land units fail to return fire, ships fail to
fire support.
* draw_map() could draw units sitting just right or just below of the
mapped area. No effect, as these parts of the map weren't actually
shown.
xydist_range() produced an inclusive upper bound when it decided that
the range covers everything in that dimension (which it didn't get
quite right either). This could make snxtsct_dist() and
snxtitem_dist() initialize the iterator with an incorrect upper bound.
Similar impact as the xyinrange() / nxtitem() lossage.
border() could print the hundreds line unnecessarily.
snxtsct() and snxtsct_all() screwed up for odd WORLD_Y: they failed to
include (WORLD_Y - 1) / 2 in the y-range. This affected all command
arguments that support "*" syntax for sectors, plus add ... c, power
n, and break.
snxtsct_all() failed to normalize the bounds (presumed harmless).
There were a few correct, but somewhat unclean uses of struct range
with inclusive upper bounds:
* nat_reset() used one internally.
* pathrange() worked with inclusive upper bounds internally, but
corrected to exclusive upper bounds before passing the range out.
* sarg_getrange() worked with inclusive upper bounds. Its only caller
sarg_area() corrected that to exclusive upper bounds.
The change to inclusive upper bounds cleans this up.
unit_map() and xysize_range() had no issues (isn't that amazing?), but
need to be updated for the changed struct range semantics.
The correct method to compute indexes into a map buffer for a struct
range is deltx(), delty().
path() used deltax(), deltay() instead, which yield correct results
only for indexes up to half the world size. Pathes spanning larger
areas were screwed up.
sona(), radmap2(), satmap() also used deltax(), deltay(), but only
with arguments where those yield correct results.
draw_map() used xnorm(), ynorm() instead, which is correct, but less
clear and less efficient.
Old code didn't check value of sctoff() for success. But it can't
fail, because we already took care of the condition that can make it
fail. Moreover, the arguments are already normalized. Therefore, we
can just call XYOFFSET().
The get_FOOp() macros are generally avoided outside the update,
because direct access to the sector cache needs synchronization to be
safe. unit_map() didn't access the cache directly until it was
converted from get_ship() & friends to get_empobjp() in commit
fec9878c. Switching to get_empobj() reverts the change to direct
access while keeping the simplification.
get_empobj_chr() and emp_obj_chr_name() access struct sctstr member
sct_type through struct empobj member type. This is technically
non-portable, because the two differ in signedness. It was also
undocumented. Fix by making sct_type signed. sct_newtype as well,
for consistency.
map_char() uses unsigned char for a sector type argument. Change that
to int. Matches how this is done elsewhere.
Move stuff to untangle the ugly cyclic dependencies between the
archives built for selected subdirectories of src/lib/:
* Move common/io.c to empthread/ because it requires empthread stuff
* Move parts of subs/nstr.c to common/nstreval.c to satisfy
common/ef_verify.o
* Move getstarg.c getstring.c onearg.c from gen/ to subs/ because they
require stuff from there
* Move bridgefall.c check.c damage.c empobj.c journal.c maps.c
sectdamage.c from common/ to subs/ because they require stuff from
there
* Move cnumb.c from subs/ to common/ to satisfy common/type.o
* Move log.c fsize.c from common/ to gen/ because they really belong
there
* Move emp_config.c mapdist.c from gen/ to common/ because they really
belong there, and require stuff from libglobal.a
Also package as/ as libas.a to satisfy common/path.o.
Remaining dependencies:
lib needs
--------------------------------------------
libas.a libglobal.a
libcommon.a libas.a libglobal.a libgen.a
libgen.a
libglobal.a
liblwp.a libgen.a
libw32.a[*] libgen.a
[*] Except for service.o, which can only be linked into the server
Link order now: liblwp.a libcommon.a libas.a libgen.a libglobal.a
libw32.a. The position of libw32.a is not quite right, but works
anyway.
