do_defdam() iterates over the list firing ships, applies return fire
to each, and frees its list element. When it finds a ship that has
been sunk already, it skips it. This also skips the free, leaking the
list element. The leak goes back to flawed bug fix in Empire 2.
As far as I can see, missile defense is the only way a ship can be
sunk there.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
quiet_bigdef() runs for each attacker. It lets each eligible defender
fire at most once. The first time a defender is eligible, it fires
and is saved in the list of defenders, along with its firing damage.
If it's eligible again for a later attacker, it's found in the list of
defenders, and the damage is reused. The list of defenders searched
with search_flist(). Unfortunately, search_flist() compares only uid,
not type, and therefore can return a previously found defender of
another type.
If there are multiple attackers and multiple defenders with the same
uid, total damage can be off, damage can be spread to attackers out of
range, and defenders may not be charged shells. Abuse is possible,
but complicated to set up, and probably not worth the trouble.
Broken in commit f89edc7, v4.3.12. Fix by comparing the type as well.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
This reverts commit f4d8d64bb3.
Breaks retreat after ship got sunk by bombs or missile.
ship_bomb() and launch_missile() pass .shp_own to retreat_ship().
Wrong after putship(), because putship() resets the owner when the
ship got sunk. retreat_ship() then oopses and fails to retreat the
surviving members of the group.
Other callers save the owner before putting the ship, and pass that.
We could change these two to do the same. But since we're trying to
get a release out, simply revert the broken commit instead.
This partly reverts a change made in Empire 2.3 to tell a submarine's
opponent only that he's dealing with a "sub" instead of the
submarine's UID and type. Hiding submarines is done by prsub().
Uses:
* Command torpedo: defender depth charges or torpedoes an attacking
submarine
If you can attack a submarine reactively, you should be able to
attack it actively, too. But that requires its UID. Reveal it
again, but keep the type hidden.
* Command fire: defender fires back at a submarine using its deck gun
Submarines need to surface to fire deck guns, so they shouldn't be
treated any different than surface ships. Revert Empire 2.3's
change entirely there, i.e. defender learns type as well as UID.
* Command torpedo: attacking submarine hits its target
Keep the submarine hidden.
* Commands torpedo and fire: attacking ship hits a submarine
The attacker passed the UID as command argument, so it doesn't
matter whether we print it or not. Printing it is simpler to code,
so do that.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Repeated for ship, sector and land unit firing. The latter prints
range only when the sanity check succeeds.
Factor out, changing ship and sector to behave like land unit firing.
When the sanity check fails, print "Jammed!" instead of "Klick!",
because "Klick!" suggests no shells. Used to be printed exactly then,
but the condition first became impossible (Chainsaw), then generalized
to "can't fire for whatever reason" (commit 22c6fd8, v4.3.12).
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
This reverts commit 9b0b0dc772.
The fire command drops depth charges when the target is a submarine in
range and firing ship has the capability. Else, it blindly fires
guns. It used to reject ships that can't use guns, even when they
could use depth charges, but commit 9b0b0dc (v4.3.31) lifted that
restruction. No such ships exist in the stock game.
If the firing ship can't fire guns, shp_fire() returns -1, triggering
an oops. Broken in commit 0757042.
Avoiding dependence of depth charge on gun fire capability is
pleasing, but nevertheless a bad idea without test coverage. Creating
the necessary tests isn't worth it, so put back the traditional
restriction instead.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Meaning of targ_sub changes from "target is a submarine" to "attacking
the target with depth charges".
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Mission is cleared only when firing at a target that is out of range.
Screwed up when missions were added in Chainsaw. Always clear it when
firing. Matches torpedo.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
multifire() writes back the firing sector after applying damage. When
an artillery unit on a bridge span commits suicide by shelling down
the supporting bridge heads, this writeback puts the bridge span right
back (less the land units and planes on it), triggering a seqno oops.
On the next update of the bridge head, the bridge span falls again.
Broken in commit fe5b266, v4.3.14.
The problematic write back is superfluous. Remove it along with a few
equally superfluous ones.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
boar() puts before retreating, the other callers afterwards. Subtle
difference, because putting resets the owner of the dead to POGO.
Until the commit before previous, retreat didn't fully work after put.
Now it does. The subtle difference between boar() and the other
callers still exists. It's better to do it the same everywhere, as
subtle differences invite bugs. Since changing boar() is not
practical, change the others.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Group retreat still doesn't work, because when boar() passes a sunk
ship to retreat_ship(), its owner has been reset to POGO already.
This makes it impossible to find the group to retreat. Instead, it
attempts to retreat ships that sank in the same sector with group
retreat orders and with the same fleet letter assigned. If any exist,
shp_may_nav() oopses, and prevents actual retreat of these ghosts.
The other retreat conditions don't have this problem, because they
call putship(), which resets the owner, only after retreat_ship().
Making boar() work the same is not practical. Instead, add an owner
parameter to retreat_ship(), and for symmetry also to retreat_land().
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
These commands report "sunk!" even when the ship survives the attack
but sinks during retreat. bomb even reports where on the retreat the
ship sinks. Has been that way since retreat was added in Chainsaw.
Report "sunk!" only when the attack sinks the ship directly.
Similar code exists for land units, but it doesn't report killings.
Change it anyway, to keep it consistent with the ship code.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
TREATIES has issues:
* Treaties can cover attack, assault, paradrop, board, lboard, fire,
build (s|p|l|n) and enlist, but not bomb, launch, torpedo and
enlistment centers.
* Usability is very poor. While a treaty is in effect, every player
action that violates a treaty condition triggers a prompt like this:
This action is in contravention of treaty #0 (with Curmudgeon)
Do you wish to go ahead anyway? [yn]
If you decline, the action is not executed. If you accept, it is.
In both cases, your decision is reported in the news.
You cannot get rid of these prompts until the treaty expires.
* Virtually nobody uses them.
* Virtually unused code is buggy code. There is at least one race
condition: multifire() reads the firing sector, ship or land unit
before the treaty prompt, and writes it back after, triggering a
generation oops. Any updates made by other threads while trechk()
waits for input are wiped out, triggering a seqno mismatch oops.
* The treaty prompts could confuse smart clients that aren't prepared
for them. WinACE isn't, but is reported to work anyway at least
common usage. Ron Koenderink (the WinACE maintainer) suspects there
could be a few situations where it will fail.
This feature is not earning its keep. Remove it. Drop command
treaty, consider treaty, offer treaty, xdump treaty, reject treaties.
Output of accept changed, obviously.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Two bugs. First, multifire() checks the condition only for surface
ships, not for submarines. Second, multifire() neglects to write back
the ship after retreating it. The player is told the ship retreats,
but it actually stays where it is.
Broken since retreat was added in Chainsaw. Previous fixes (commit
8065fe8, v4.3.1 and commit de2651e, v4.3.19) "fixed" only the
bulletin.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
It's not firing, yet.
While there, trim an unwanted blank line before reporting the first
sector ready.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Can't actually happen with the current damage formulas. If it could,
then the special treatment would be inconsistent with sectors and land
units.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
We check all necessary conditions for being able to fire before
prompting for a target. Except for land unit guns. Clean that up.
fort_fire(), shp_fire() or lnd_fire() fail only when the fort, ship or
land unit can't fire. If that happens, our checking is incomplete.
Oops then.
We recheck some of the necessary conditions after getting the target.
However, because the command fails when the firing sector, ship or
land unit has changed since the first check, these rechecks can't
fail. Drop them.
Note that the rechecks were just as useless before commit 66165f3
fixed fire to fail on change, because they rechecked the unchanged
cached copy instead of the possibly changed original.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
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.
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.
Before, they were stored as short. Wider uids use more space, but the
next commit will recover it by narrowing other members.
The use of short has always limited the number of ships, planes, land
units and nukes to SHRT_MAX (commonly 32768). Only the most extreme
games ever came close.
Commit 49780e2c (v4.3.12) added struct sctstr member sct_uid to make
struct empobj member uid work for sectors. This made the limit apply
to sectors as well. We've had games with more than 32768 sectors.
Pinpointed assignments within if conditionals with spatch -sp_file
tests/bad_assign.cocci (from coccinelle-0.1.4). Cherry-picked diff
hunks affecting conditionals split over multiple lines, and cleaned
them up.
When a ship is shelled, retreat condition 'i' (injured) applies. When
there's no return fire, 'h' (helpless) applies as well. Ships
retreated twice in that case. Fix that.
Failing a command with code RET_SYN prints help and doesn't charge
BTUs. Failing with code RET_FAIL doesn't print help and charges BTUs.
A couple of command failures were changed or added recently to fail
with RET_SYN, because they're due to invalid player input. Some of
them, however, can happen after the command already did something, so
BTUs must be charged, or else players can deliberately fail the
command to save BTUs:
* Commit 9eda5f87 adds RET_SYN failures when getting player input
fails for:
- arm third argument
- deliver fourth argument
- fire third argument
- lmine second argument
- order d fourth argument
- range second argument
- sail second argument
- tend third argument
* Commit be41e70f likewise for:
- designate second argument
- morale second argument
- set third argument
- tend fourth argument
* Commit d000bf92 likewise (with a bogus commit message) for bdes
second argument.
* Commit 9f4ce71a likewise for ltend third and fourth argument.
* Commit 9031b03b changes failure code from RET_FAIL when getting
player input fails for threshold third argument. It adds RET_SYN
failure when the argument is bad. Some bad arguments already failed
that way before.
* Commit a7cf69af changes it from RET_FAIL when designate second
argument is bad.
Change them all to fail with RET_FAIL.
Many other places have the same bug, but those are left for another
day.
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.
The old code didn't honor command abortion at the following prompts:
* arm third argument
* deliver fourth argument (also simplify)
* fire third argument
* fly and recon prompt for carrier to land on: pln_onewaymission()
treated abort like empty input, which made planes attempt landing in
the sector.
* lmine second argument
* order d fourth argument
* power c nat(s) argument
* range second argument
* sail second argument
* shutdown both arguments (first one was broken in commit 84cfd670,
v4.3.10, second one never worked).
* tend third argument
