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>
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>
It's set to zero via memset(). Incorrect, but harmless, because it's
not actually used. Correct it anyway.
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>
dist(), att_reacting_units() and s_commod() are only interested in
cost, not the actual path. BestLandPath() and BestDistPath() compute
both cost and path. Use path_find() directly instead.
Destinations are no longer treated as unreachable when the best path
is longer than 1023 characters.
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.
Land units pay a mobility penalty when marching into a non-old-owned
sector without sector mobility, to slow them down in newly taken
sectors. Attacking land units pay this penalty regardless of sector
mobility.
When attacking out of an allied sector, the penalty was computed as if
the land unit was owned by that ally. Attacking sectors old-owned by
that ally was too cheap, and taking back one's own was too expensive.
Broken since attacking land units pay the "newly taken" mobility
penalty: commit 2e693275, v4.3.6.
When an attacking sector got lost while the player was at a prompt,
and the new owner was allied to the player, the server got confused:
1. If the sector attacked with mil, the server let the ghost mil
attack, but not occupy.
2. If the sector was allied, the server reported the sector loss and
land units dropping out of the attack, but claimed the lost sector was
yours.
Fix 1. by dropping sectors from attack when they change owner away
from the player, regardless of relations. Side effect: also drops any
surviving land units there. Before, they dropped out only if the new
owner wasn't allied to the player. That change's okay.
Fix 2. the obvious way: change the messages.
Broken in 4.0.0.
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.
It passed def->own to lnd_sweep(), which looks like a bug. But it's
actually player->cnum there, because take_def() already set def->own
to player->owner: take_def() first changes the owner of the attacked
sector by calling takeover(), then updates def->own from that in
att_get_combat().
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.
getland() is obviously redundant in get_dlist(), because the land unit
can't have changed since the previous read.
What's up with ask_olist() is less obvious. For each capable land
unit, ask_olist() asks the player whether to attack with it. If it
attacks, it gets copied into olist. Since asking the player yields
the processor, these copies can become stale. However, re-reading
fixes that just for the last one. Moreover, the code copes with stale
copies just fine: ask_olist() is always directly followed by
att_get_offense() or att_move_in_off(), and both replace the
potentially stale copy in olist.
ask_olist() and get_dlist() called get_land() with llp->chrp still
null, which made it initialize parts of llp instead of performing its
usual integrity check. att_reacting_units() had the initialization
inline. Change the other two to match, and simplify get_land(). No
functional change.
Seamines and landmines share storage. Sea and bridge span sectors can
hold only sea mines, other sector types only landmines. Sector type
checks were missing or incorrect in several places:
* Seamines under bridge spans were mistaken for landmines in several
places:
- ground combat mine defense bonus, in get_mine_dsupport() and
stre(),
- land units retreating from bombs, in retreat_land1(),
- non-land unit ground movement (commands explore, move, transport,
and INTERDICT_ATT of military), in check_lmines(),
Fix them to check the sector type with new SCT_MINES_ARE_SEAMINES(),
SCT_LANDMINES().
* plane_sweep() mistook landmines for seamines in harbors. Bug could
not bite, because it's only called for sea sectors. Drop the bogus
check for harbor.
* Collapsing a bridge tower magically converted landmines into
seamines. Make knockdown() clear landmines.
Also use SCT_MINES_ARE_SEAMINES() and SCT_LANDMINES() in mine(),
landmine(), lnd_sweep() and lnd_check_mines(). No functional change
there.
Keep checking only for sea in pln_mine(), plane_sweep(),
retreat_ship1(), shp_sweep() and shp_check_one_mines(). This means
seamines continue not to work under bridges. Making them work there
is tempting, but as long as finding seamines clobbers the sector
designation in the bmap, it's better to have them in sea sectors only.
Historical notes:
Mines started out simple enough: you could mine sea and bridge spans,
and ships hit and swept mines in foreign sectors.
Chainsaw 2 introduced aerial mining and sweeping. Unlike ships,
planes could not mine bridge spans. plane_sweep() could sweep
harbors, which was wrong, but it was never called there, so the bug
could not bite.
Chainsaw 3 introduced landmines. The idea was to permit only seamines
in some sector types, and only landmines in the others, so they can
share storage. To figure out whether a sector has a particular kind
of mines, you need to check the sector type. Such checks already
existed in mine, drop and sweep, and they were kept unchanged. The
new lmine command also got the check. Everything else did not.
Ground movement and combat could hit and sweep seamines in bridge
spans. Ships could hit and sweep landmines in harbors.
Empire 2 fixed land unit movement (march, INTERDICT_ATT) not to
mistake seamines for landmines on bridge spans. It fixed ships not to
mistake landmines for seamines. The fix also neutered seamines under
bridge spans: ships could neither hit nor sweep them anymore. Both
fixes missed retreat.
Commit 5663713b (v4.3.1) made ship retreat consistent with other ship
movement.
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.
A victorious attacker can move attacking land units into the newly
conquered sector or leave them behind. Normally, the player is asked
what to do, but when the land unit's army has already been told to
stay behind, or the command has been aborted, the land unit stays
behind without asking. In that case, a copy of the land unit made
right after the victory was written back. Any updates since the
victory were wiped out, triggering a seqno mismatch oops.
Fix by moving the re-read of the land unit in ask_move_in() out of the
prompt conditional.
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().
Being in supply is relevant for defending and reacting units. The
code used has_supply() to check that.
Contrary to its name, has_supply() does not check whether the land
unit has enough supplies to be in supply, but whether it has or could
draw enough. So, defending and reacting units did not actually draw
any missing supplies.
Fix that in get_dlist() and att_reacting_units() by calling
resupply_all(), then checking with new lnd_in_supply() instead of
has_supply(). The fix of att_reacting_units() is complicated by the
fact that it is also used in the strength command, and should keep not
drawing supplies there.
Rename has_supply() to lnd_could_be_supplied(). Replace its uses
immediately after resupply_all() by lnd_in_supply().
Land units were erroneously charged the much lower raw path cost,
except when attacking high-mobility terrain (mountains). Broken in
commit 2673a258, v4.3.6.
Land unit reactions are overly complex because we have two different
concepts controlling them: reaction radius (set with lrange) and
reserve mission (set with mission). You need to deal with both to set
up or query reactions.
Commit 8d0e1af5 "fixed" this by making reserve missions meaningless.
The previous commit made reserve missions meaningful again: they
support an op-area now. This brought back the problem of having to
deal with two separate commands to accomplish one thing.
Fix this for good by removing non-mission land unit reaction
alltogether. The only feature we lose by that is the ability to order
land units to react until the order is explicitely cancelled. That's
because missions are implicitely cleared by many commands and events,
while non-mission reaction wasn't. Closes#858121 and #858122.
Remove the non-mission reaction case from att_reacting_units().
Don't limit reserve missions to the land unit's reaction radius: make
lnd_reaction_range() return the type's maximum radius instead of
lnd_rad_max.
The reaction radius is now useless. Remove the lrange command, and
struct lndstr member lnd_rad_max along with its selector react.
Remove land command's column rd. Make ldump show column react as
zero. Deprecate edit key 'P' in dounit(), and don't show it in
pr_land().
