Don't report every incapable ship or land unit. Complain only when
there are no capable ships or land units available.
The ships are all in the same sector, so complain about the sector
type just once instead of once per capable ship or land unit.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When ships enter a sector with sea mines, any minesweepers sweep, then
hit mines, and finally all ships (including the minesweepers) hit
mines. Sweeping in a sector (navigate sub-command 'm') works the same
without the final step.
When land units enter a sector with land mines, any engineers sweep,
and then all land units (including the engineers) hit mines. Sweeping
in a sector (march sub-command 'm') works the same, which means
non-engineers can hit mines then. Broken in Empire 2.
Actually broken for ships too then. 4.0.17 fixed ships, but neglected
to fix land units.
Change the land unit code to work like the ship code. Fixes march
sub-command 'm' not to expose non-engineers to mines. Changes march,
attack and assault with option INTERDICT_ATT enabled to expose
engineers twice.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When lnd_sweep() rejects an engineer for want of mobility, it removes
it from the list of units.
Can happen only when sweeping for march sub-command 'm'. Any engineer
without mobility is dropped from the march immediately. Broken in
Empire 2.
Fix lnd_sweep() to handle this case just like the others, and like
shp_sweep(): report and continue with the next list member.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Require positive mobility for sweeping mines, just like ships do.
Screwed up when land units were added in Chainsaw 3.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Commit cd8d742 mechanically combined struct mlist's mcp and struct
llist's llp into struct ulist's chrp, adding type casts to every use.
Not necessary, simply use mchr[] and lchr[] directly.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Handle "no movement" before the movement loop instead of relying on
the first iteration of the loop.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
The capability to march land units spread over several sectors is
obscure and rarely useful. Accidental use is probably more frequent
than intentional use. Issues:
* Interactive prompts show only the leader's position, and give no
clue that some land units are actually elsewhere.
* Path finding is supported only when all marching land units are in
the same sector.
* In each step, the bmap is updated for the leader's radar. The bmap
is not updated around other marching land units. Already odd when
all units are in the leader's sector, and odder still when some are
elsewhere.
* Interdiction becomes rather complex. For each movement, every
sector entered is interdicted independently. This means the same
ship, land unit or plane can interdict multiple times. Interdiction
order depends on the order the code examines land units. which the
player can control. This is all pretty much undocumented.
* Complicates the code and its maintenance. Multiplies the number of
test cases needed to cover march.
I feel we're better off without this feature.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Unlike the move command, march checks sector abandonment before every
step.
If the player declines, the last land unit stays put and is removed
from the march.
Except when sectors or land units change while we're waiting for the
player's reply. Then the last unit is not removed from the march.
This can scatter land units. Screwed up when checking for abandoning
the sector was added in 4.2.2.
Change march to work like move, and to avoid scattering land units: if
the player declines to abandon the sector, the command simply fails.
Put the check into new lnd_abandon_askyn().
Extend would_abandon() and want_to_abandon() from a single land unit
to many. Rename the latter to abandon_askyn() for consistency.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When attempting to enter a sector with a land unit that can't go there
while the marching land units are all in the same sector, march stops
and prompts without removing the incapable land unit from the group.
If another land unit has already entered the sector, the group becomes
scattered.
This can happen when marching a mixed group of spies and non-spies
into a non-allied sector. Same for marching a mixed group of trains
and non-trains into a sector without rail, except such groups have
been disallowed since commit 36e41e5 (v4.3.7). Both screwed up when
spies and trains were added in 4.0.0
Remove the incapable land unit from the group when another land unit
can enter the sector. This avoids scattering land units.
Don't remove incapable land units when no land unit can enter the
sector. Without this, march would remove everyone and end then.
It can also happen when sectors or land units change while we're
sitting at the "Do you really want to abandon X,Y" prompt. I'm going
to fix that differently.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
With autonav and SAIL gone, shp_nav_put() isn't used externally
anymore. lnd_mar_put() never was; it got external linkage just for
symmetry.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
lnd_put() serves two masters: march, which wants it to report
"stopped" and write back struct ulist member mobility to
unit.land.lnd_mobil, and ground combat, which doesn't.
lnd_put() assumes march when actor is non-zero. Correct (but see
commit 8c502d4). Dates back to Empire 2.
Too ugly for my taste. Specialize for each master instead:
lnd_mar_put() for march, and lnd_put() for ground combat.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Commit d94d269 combined them into unit_put(), but that has turned out
not to be useful. Split them again.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
... get_dlist(), att_reacting_units().
This loses malloc() error checking in ask_olist() and get_dlist(). No
great loss, because we don't check in so many other places, including
att_reacting_units(). We should use a wrapper that terminates on
error, though. Left for another day.
ask_olist(), get_dlist() and att_reacting_units() zero the struct
ulist with memset(). lnd_insque() doesn't, so these functions need to
zero any members not otherwise initialized explicitly now.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Change chance in percent lnd_retreat - lnd_effic - 1 to lnd_retreat -
lnd_effic. It's been that way since Empire 2, but I can't bring
myself to document the silly -1.
"info morale" wasn't updated when the retreat chance was changed in
Empire 2. Fix that.
March code reads the land units into a land unit list, and writes them
back when it changes them, e.g. when a land unit stops. If a land
unit changes in the land unit file while it is in such a land unit
list, the copy in the land unit list becomes stale, and must not be
used.
To that end, do_unit_move() calls lnd_mar() after prompting for path
or destination. lnd_mar() re-reads all the land units.
Unfortunately, it still writes back stale copies in certain
circumstances. Known ways to trigger such writes:
* Deity loads land unit onto a ship or land unit
* Land unit's crew killed just right, e.g. by collateral damage from
interdiction, followed by additional updates, such as shell fire
damage
* Sector no longer owned or allied, e.g. allied sector captured by an
enemy (own sector would kill or retreat the land unit)
Writing a stale copy wipes out the updates that made the copy stale,
and triggers a seqno mismatch oops. For instance, damage that follows
killing of all crew by collateral damage from interdiction is wiped
out. If no damage follows, we still get a generation oops.
lnd_take_casualty() uses uninitialized rsect to compute the mobility
cost of retreating a defending land unit. This can charge incorrect
mobility, prevent retreat, or, if the stars align just right, crash
the server when sector_mcost() subscripts dchr[] with it.
Broken in commit 4e7c993a, v4.3.6. Reported by Scott C. Zielinski.
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.
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.
No functional change, because the value of rel only matters when
sect.sct_own != actor, and then it's the same as before.
The new value of rel permits simplifying sect.sct_own != actor && rel
!= ALLIED to just rel != ALLIED.
Replacing getrel(getnatp(US), THEM) by relations_with(US, THEM) makes
a difference only when US equals THEM.
Replace patterns like "us == them || getrel(getnatp(us), them)..." by
"relations_with(us, them)...".
feels_like_helping() case cn == foe is missing in the code it
replaces. No difference in behavior, because:
* cn == foe && cn == friend can't happen. Because you can't get into
ground combat against yourself (assault, attack and paradrop don't
let you), friend != foe for support.
* cn == foe && cn != friend behaves the same: no support.
feels_like_helping() returns 0 because of the explicit case. The
replaced code doesn't support because cn can't be at war with
itself.
lnd_mar() and lnd_mar_one_sector() take an actor argument.
Nevertheless, they sometimes used player->cnum. Fortunately, they are
the same: all callers pass current player for actor. Normalize to
actor for consistency.
SLOW_WAR has issues:
* The check whether the attacker old-owns the attacked sector is
broken, because att_abort() uses sect.sct_oldown uninitialized.
Spotted by the Clang Static Analyzer.
* Its implementation in setrel() is somewhat scary. It's actually
okay, because that part of setrel() only runs within decl(). Other
callers don't reach it: update_main() because player->god != 0
there, and the rest because they never pass a rel < HOSTILE.
* Documentation is a bit vague.
SLOW_WAR hasn't been used in a public game in years. Fixing it is not
worth it, so remove it instead.
lnd_mar(), lnd_sweep() and lnd_mar_one_sector() printed to the current
player, their actor argument, and to land unit owner.
lnd_mar_one_sector()'s use of xyas() looked particularly suspicious:
it passed actor, then printed the result to the current player or land
unit owner. Fortunately, all three are the same: all callers pass
current player for actor, and land unit owner is the same, since even
deities can't march foreign land units. Normalize to actor for
consistency.
While there, rename lnd_mess() to lnd_stays().
take_def() and ask_move_in() printed both to the current player and to
land unit owner. Their use of prcom() and xyas() looked particularly
suspicious: they used the current player, then printed the result to
the land unit owner. Fortunately, current player and land unit owner
are the same, since even even deities can't attack with foreign land
units. Normalize to current player for consistency.
Switch get_ototal(), get_oland(), kill_land() and move_in_land() to
current player as well.
