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.
Destination arguments can be a path or sector coordinates.
do_unit_move() passes the argument buffer to unit_path() to convert
coordinates to a path. If unit_path() fails, do_unit_move() still
interprets the argument as path.
This is correct when unit_path() fails because the argument is not
coordinates. But it can also fail when it is coordinates, namely when
the destination isn't reachable, when the path to it is too long, or
when the ships or land units aren't together. Then do_unit_move()
interprets coordinates as path, and rejects them with "Legal
directions are:".
Except when a land unit's destination read from a march prompt isn't
reachable, because then unit_path() empties the argument buffer that
do_unit_move() uses.
Change unit_path() to succeed when the argument is not coordinates.
Make do_unit_move() discard the argument when unit_path() fails,
i.e. when it is bad coordinates. unit_path() emptying the argument no
longer has an effect, drop it.
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.
do_unit_move() reads the ships into a list. It re-reads them when it
prompts for sub-commands. shp_nav_one_sector() writes them back when
it moves ships.
Mine-laying (sub-command 'd') updates the minelayer, invalidating the
copy in the list. Any movement sub-command before the next prompt for
sub-commands wiped out this update, triggering a seno mismatch oops.
Happens only if 'd' is used without arguments, because remaining
sub-commands are discarded when there are arguments.
Broken when mine-laying was added in commits 2438fe7c, v4.3.7.
Same for march, commit 274c8e42, v4.3.7.
Fix by stopping after 'd' regardless of arguments.
When sub-command 'd' was used without arguments, do_unit_move() failed
to supply the second argument to mine(), which duly prompted for it.
This contracticted info, and could trigger a generation oops.
do_unit_move() reads the ships into a list. It re-reads them when it
prompts for sub-commands. shp_nav_one_sector() writes them back when
it moves ships.
The mine prompt made the list stale. Movement sub-commands before the
next prompt for sub-commands wrote back stale ships, triggering a
generation oops. Example: "nav 15 dg".
Broken when mine-laying was added in commits 2438fe7c, v4.3.7.
Same for march, commit 274c8e42, v4.3.7.
For an argument consisting of a valid path plus whitespace,
do_unit_move() eventually passed the whitespace suffix to parse(),
then dereferenced player->argp[0]. But that was null.
Broken in commit 0c12d837, v4.3.7. Trivial for players to trigger.
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.
land_unit as well.
(do_unit_move): Add view option for land units using unit_view().
Combine ship and land viewing using the unit_view().
(shp_view): Remove, not used any more, replaced by unit_view().
beginning of function instead using the leader. The leader
becomes unknown when stopping or unit list becomes
empty. This broke printing the path for ships. Broken
in rev 1.43.
