When attempting to enter a sector with a ship that can't go there
while the navigating ships are all in the same sector, navigate stops
and prompts without removing the incapable ship from the group. If
another ship has already entered the sector, the group becomes
scattered.
This can happen only when navigating a mixed group of ships with and
without canal capability into a canal. Broken in commit 74e4e281,
v4.3.0.
Remove the incapable ship from the group when another ship can enter
the sector. This avoids scattering ships.
Don't remove incapable ships when no ship can enter the sector.
Without this, navigate would remove everyone and end then.
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>
SAIL has issues:
* Sail orders are executed at the update. Crafty players can use them
to get around the update window.
* The route is fixed at command time. You can't let the update find
the best route, like it does for distribution.
* The info pages documenting it amount to almost 100 non-blank lines
formatted. They claim you can follow friendly ships. This is
wrong. They also show incorrect follow syntax. Unlikely to be the
only errors.
* Few players use it. Makes it a nice hidey-hole for bugs. Here are
two nice ones:
- If follow's second argument is negative, the code attempts to
follow an uninitialized ship. Could well be a remote hole.
- If ship #1 follows #2 follows #3 follows #2, the update goes into
an infinite loop.
* It's more than 500 lines of rather crufty code nobody wants to
touch. Thanks to a big effort in Empire 2, it shares some code with
the navigation command. It still duplicates other navigation code.
The sharing complicates fixing the bugs demonstrated by
navi-march-test.
Reviewing, fixing and testing this mess isn't worth the opportunity
cost. Remove it instead. Drop commands follow, mquota, sail and
unsail. Drop ship selectors mquota, path, follow.
struct shpstr shrinks some more, on my system from 160 to 120 bytes.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
When a player moves more than 1023 sectors in a single navigate
command, we overrun the buffer holding the path taken. Remote hole,
but it requires a ship that can go that far, and even a ship with
speed 1000 would need a tech level well in excess of 1000 for that.
Thus, the hole is purely theoretical for even remotely sane game
configurations.
First known version with the flaw is 4.0.0.
Fix by going back the older behavior: don't print the total path
taken, but do print what the path finder does. Context diff of an
example:
[0:634] Command : nav 3 6,0
Flagship is od oil derrick (#3)
+Using path 'n'
h =
k . .
j d
<67.2:67.2: 6,0> h
od oil derrick (#3) stopped at 6,0
-Path taken: n
This is how march works.
Removes the only use of shp_nav_one_sector()'s unusual return value 2.
Return 1 instead.
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>
Some callers have to second-guess shp_check_nav() to figure out
whether CN_LANDLOCKED means "too big to fit into the canal" or "can't
go there at all".
Fix that by returning d_navigation. CN_LANDLOCKED becomes either
NAV_CANAL or NAV_NONE, CN_CONSTRUCTION becomes either NAV_02 or
NAV_60, and CN_NAVIGABLE becomes NAVOK.
The CN_NAVIGABLE, ... codes are now unused. Drop them.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Due to a typo, shp_missile_interdiction() picks the admissible target
with highest efficiency instead of the one with highest efficiency *
build cost.
Broken in commit cd8d7423, v4.3.8.
Navigation code reads the ships into a ship list, and writes them back
when it changes them, e.g. when a ship stops. If a ship changes in
the ship file while it is in such a ship list, the copy in the ship
list becomes stale, and must not be used.
To that end, do_unit_move() calls shp_nav() after prompting for path
or destination. shp_nav() re-reads all the ships. Unfortunately, it
still writes back stale copies in certain circumstances. Known ways
to trigger such writes:
* Deity sets a sail path
* Ship's crew gone, e.g. killed by shell fire
* Sector no longer navigable, e.g. harbor shelled down, or bridge
built
Writing a stale copy wipes out the updates that made the copy stale,
and triggers a seqno mismatch oops. For instance, ship damage that
kills all crew while the ship is being navigated gets wiped out.
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 change affects only
notified[victim], which isn't used in the loop around
notify_coastguard(), and gets overwritten before the interdiction fire
loop.
Switching from getrel() to relations_with() can change the value from
NEUTRAL to ALLIED. The change doesn't matter when the value's only
compared to HOSTILE, as both old and new value are greater than
HOSTILE. Likewise for >= NEUTRAL.
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)...".
shp_nav() and shp_nav_one_sector() printed both to their actor
argument and to ship owner. shp_nav_one_sector()'s use of xyas()
looked particularly suspicious: it passed actor, then printed the
result to the ship owner. Fortunately, actor and ship owner are the
same, since even deities can't navigate foreign ships. Normalize to
actor for consistency.
While there, rename shp_mess() to shp_stays().
Much of the code assumes that only the ship's owner can navigate it.
The assumption is correct, because shp_nav() leaves foreign ships
behind with a bogus "was sunk at" message (suppressed for country #0).
It would be nice to let deities navigate foreign ships, but the
assumption is not trivial to remove. For now, just avoid the bogus
message.
Historical note: it looks like deities used to be able to navigate
foreign ships just fine until Empire 2 factored common code out of
navigate, sail and autonav.
Don't use multiple calls of mpr() to print a single line, because that
creates a separate bulletin for each part. The read command normally
merges the bulletins, but if the bulletins are more than five seconds
apart (clock jumped somehow), we get a bulletin header in the middle
of a line.
The mpr() misuse was introduced in Empire 2.
Before commit a269cdd7, pln_damage() returned zero when the damage was
nuclear, and callers used that to bypass conventional damage code.
Zero value can't happen anymore.
Ships can expend shells to defend against missiles, in
shp_missile_defense(). Any shell use by the target ship got wiped out
when shp_missile_interdiction() wrote back the target ship, triggering
a seqno mismatch oops.
Ships get updated when they launch planes to intercept interdicting
planes, in mission_pln_equip(). Any petrol use by the target ship got
wiped out when shp_mission_interdiction() wrote back the target ship,
triggering a seqno mismatch oops.
Fix by re-reading the target ship in shp_damage_one(). This also
affects shp_fort_interdiction(), where it is not necessary. A bit
inefficient, but let's keep this fix simple.
Movement stops when shp_interdict() or lnd_interdict() report
interdiction. However, they reported it only when there was
interdiction damage.
Zero interdiction damage commonly happens when interdicting missiles
miss, or all bombers abort. Stopping regardless of damage makes more
sense there.
Moreover, not stopping is buggy: do_unit_move() needs to take care not
to wipe out updates made by interdiction to the moving ships or land
units. It does so only when it stops. Updates made by interdiction
without interdiction damage could get wiped out, triggering a seqno
mismatch oops.
Known ways moving ships and land units can get updated by interdiction
despite there is no interdiction damage:
* Interdicting bombers get intercepted by planes based on a navigating
carrier, carrier gets charged petrol. The bug wipes out the petrol
use.
* Marching land units get interdicted by planes, but all planes miss.
Sufficiently large collateral damage to the sector can still damage
the land units. The bug wipes out the damage to land units.
To make shp_interdict() and lnd_interdict() report interdiction
regardless of damage, change lnd_missile_interdiction(),
lnd_fort_interdiction(), lnd_mission_interdiction(),
shp_missile_interdiction(), shp_fort_interdiction(),
shp_mission_interdiction() to return whether there was interdiction.
Before, they returned whether there was damage.
Change unit_interdict(), perform_mission(), perform_mission_land(),
perform_mission_ship(), perform_mission_msl(), and
perform_mission_bomb() to return -1 for no interdiction, so that
callers can distinguish no interdiction from interdiction with no
damage.
shp_missile_interdiction() does nothing when there are no suitable
targets. Its users avoid to call it when there are none, probably to
save a few cycles. Optimize shp_missile_interdiction() for that case,
and simplify the callers.
Collateral damage was disabled, because after msl_hit() reported a
miss, the missile may or may not have reached the target.
Fix by splitting msl_launch() off msl_hit().
Drop the disabled collateral damage code for sector targets, because
sectors can't be missed. Enable it for ships and land units.
Since msl_launch() returns whether the missile is sub-launched, drop
launch_missile() parameter sublaunch, and simplify its caller.
Ship interdiction works sector by sector. Interdicting missiles
targeted all ships not yet interdicted, not just the ones in the
current sector. This could lead to interdiction outside missile range
or op area.
Before Empire 2, nukes could be delivered only with bomb (special
mission 'n', airburst only) and launch (targeting sectors or
satellites only).
Empire 2 made nukes available for any kind of bombing, and for any
missile strike on sectors or ships. This included interdiction and
support missions. Nuclear-tipped anti-sats and bomb mission n were
removed.
Unfortunately, this was done in a messy way, which led to
inconsistencies and bugs. The problem is that ordinary damage affects
just the target, while nuke damage affects an area. Code dealing with
plane damage was designed for the former. Instead of rewriting it to
cope with area damage cleanly, nuke damage got shoehorned into
pln_damage(), the function to compute conventional plane damage, as a
side effect: computing damage blasted sectors in the area.
If the plane carried a nuke, pln_damage() returned zero (conventional)
damage. Without further logic, this simply bypassed the code to apply
damage to the target. This worked out okay when the target already
got damaged correctly by the side effect.
However, some targets are immune to the side effect: when interdicting
a move or explore command, the commodities being moved are not in any
sector.
For other targets, damage has effects other than damaging the target:
offensive and defensive support don't apply the (conventional) damage
to the target sector. Instead, they turn it into a combat bonus.
Without further logic, nuclear damage doesn't contribute to that.
To make all that work, pln_damage() returned the nuclear damage for
ground zero as well. Because a plane does either conventional or
nuclear damage, one of them is always zero.
Most callers simply ignored the nuclear damage, and applied only the
conventional damage.
Bug: land units and ships failed to retreat when pin-bombed or
missiled with a nuke. That's because they received zero conventional
damage.
The mission code flies planes and missiles and tallies their damage.
This mission damage included nuclear damage at ground zero (except for
missiles sometimes, see below), to make support and commodity
interdiction work. Unfortunately, this broke other things.
Bug: when bombers interdicted ships or land units, nukes first damaged
the ships or land units by the side effect, then again through mission
damage. Interdicting missiles had a special case to avoid this.
Bug: when interdicting move, explore or transport, nukes first damaged
the sector by the side effect, then again through mission damage's
collateral damage.
There may well be more bugs hiding in this mess.
The mess is not worth fixing. While the idea of interdicting and
supporting with nukes sounds kind of cool, I believe it's pretty
irrelevant in actual play.
Instead, go back to a variation of the original rules: nukes can be
delivered only through bomb mission 's' and launch at sectors.
Make arm reject marine missiles in addition to satellites, ABMs and
SAMs, and clear the mission. Make mission reject planes armed with
nukes. Oops when they show up in mission_pln_equip() anyway.
Make pln_equip() allow planes with nukes only for missions 's' and
't'.
Clean up pln_damage() to just compute damage, without side effect.
Change strat_bomb() and launch_missile() to detonate nukes. Simplify
the other callers. Parameter mission of msl_launch_mindam() is now
unused, remove it.
Missiles exploding on launch no longer set off their nukes. That was
pretty ridiculous anyway.
The automatic supply interface has design flaws that make it hard to
use correctly. Its current uses are in fact all wrong (until commit
0179fd86, the update had a few uses that might have been correct).
Some of the bugs can only bite with land unit capability combinations
that don't exist in the stock game, though.
Automatic supply draws supplies from supply sources in range. Since
that can update any supply source in range, all copies of potential
supply sources a caller may keep can get invalidated. Writing back
such an invalid copy wipes out the deduction of supplies and mobility
from a source, triggering a seqno mismatch oops.
This commit redesigns the interface so that callers can safely keep a
copy of the object drawing the supplies (the "supply sink"). The idea
is to pass the sink to the supply code, so it can avoid using it as
source. The actual avoiding will be implemented in a later commit.
Copies other than the supply sink still need to be eliminated. See
commit 65410d16 for an example.
Other improvements to help avoid common errors:
* Supply functions are commonly used to ensure the sink has a certain
amount of supplies. A common error is to fetch that amount
regardless of how many the sink already has. It's more convenient
for such users to pass how many they need to have instead of how
many to get.
* A common use of supply functions is to get supplies for immediate
use. If that use turns out not to be possible after all, the
supplies need to be added somewhere, which is all too easy to
forget. Many bugs of this kind have been fixed over time, and there
are still some left. This class of bugs can be avoided by adding
the supplies to the sink automatically.
In fact, this commit fixes precisely such bugs in mission_pln_equip()
and shp_missile_defense(): plane interception and support missions,
missile interception (abms), launch of ballistic missiles and
anti-sats could all lose shells, or supply more than needed.
Replace supply_commod() by new sct_supply(), shp_supply(),
lnd_supply(), and resupply_all() by new lnd_supply_all(). Simplify
users accordingly.
There's just one use of resupply_commod() left, in landmine(). Use
lnd_supply_all() there, and remove resupply_commod().
