The autonavigation feature has issues:
* Autonavigation orders are executed at the update. Crafty players
can use them to get around the update window.
* Usability is poor:
- The order command is overly complex, not least because it can do
five different things: clear, suspend, resume, declare route, set
cargo levels.
- Unlike every other command involving movement, order does not let
you specify routes, only destination sectors.
- Setting cargo levels can silently swap start and end point of a
circular route, because "this keeps the load_it() procedure
happy". Maybe it does, but it surely keeps players confused.
- Setting "start" cargo levels actually sets the "end" levels, and
vice versa. Has always been broken that way.
- Predicting what exactly autonavigation will do at the update isn't
easy.
* The info pages documenting it amount to almost 400 non-blank lines
formatted. They claim only merchant ships can be given orders.
This is wrong. Unlikely to be the only error.
* Few players use it, and its workings at the update a fairly opaque.
Makes it a nice hidey-hole for bugs. Here are two:
- Unlike the scuttle command, autonavigation happily scuttles trade
ships while they're on the trading block.
- Unlike the load command, autonavigation can load in friendly and
allied sectors.
* It's more than 700 lines of rather crufty code nobody wants to
touch. Thanks to a big effort in Empire 2, it shares code with the
navigation command. It still duplicates load 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 order, qorder and sorder.
Drop ship selectors xstart, xend, ystart, yend, cargostart, cargoend,
amtstart, amtend, autonav.
xdump ship sheds almost half its columns. struct shpstr shrinks, on
my system from 200 to 160 bytes.
Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Remote hole, can smash the stack. Additionally, the confirmation
prompt is misleading when the player supplies conditionals. Redesign
the flawed prompt.
Broken when Chainsaw added the confirmation prompt. Reported by Scott
C. Zielinski.
Trade ships are now enabled when a ship type with capability trade
exists. No such type exists by default; to enable trade ships,
deities have to customize table ship-chr.
Before, trade ship types were ignored when option TRADESHIPS was
disabled. Except for xdump ship-chr, which happily dumped unusable
trade ship types.
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.
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)...".
Factor unit_drop_cargo() out of scra(), scuttle_ship(),
scuttle_land(), fix it up:
* Some messages were sent as bulletins instead of printing them.
* Nukes were always destroyed. They're now treated exactly like other
cargo.
* scuttle destroyed some cargo silently, and listed other cargo as
"scuttled". It now simply lets unit_update_cargo() running from
carrier prewrite callbacks list all cargo "lost".
Simplify its callers. scuttle_ship() and scuttle_land() are now
trivial, inline and remove.
When called from the scuttle command, scuttle_tradeship() asks for
confirmation when scuttling doesn't pay. When called from the autonav
code, it can't ask. Change it to fail then, and use that in
nav_ship() to avoid scuttle where it doesn't pay. Also simplify some.
Fix scuttle to ask for confirmation when scuttling a tradeship in an
unsuitable sector even when the tradeship is pirated. Broken when
commit a99bc3be (v4.2.13) suppressed that for pirated tradeships
wholesale because it let pirates ferret out where the ship was built.
scra() and scut() printed their "scrapped in" / "scuttled in" message
in two parts. Messages for scrapped / scuttled cargo were printed
between the parts. Fix by printing in one go, after the cargo
messages.
Until commit 3e370da5, dead planes had to be explicitely taken off
their carrier to update load counters. This is no longer necessary;
simplify pln_put1() and scut(). scut() got it wrong, by the way: it
failed to take planes off land units.
Change snxtitem_all() loops that skip everything but a carrier's cargo
to use snxtitem_cargo() in scra(), scuttle_ship(), scuttle_land(),
takeover_ship(), takeover_land(), trade_desc(), feed_ship().
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.
other. Ensure headers in include/ can be included in any order
(except for econfig-spec.h, which is special). New header types.h to
help avoid inclusion cycles. Sort include directives. Remove some
superflous includes.
oprange, show_mission, nameofitem, build_mission_list_type,
unit_map, xdvisible, trdswitchown, ontradingblock, trad, check_trade,
unit_type_name, start_stop_unit, scut, scra, mission, multifire,
perform_mission, fuel, NSC_GENITEM): Replace struct genitem with
struct empobj. Remove genitem.h and create a new file empobj.h.
Replace multiple instances of unions of ef_type structures with
one standard union empobj_storage which is a superset of the individual
instances.
(launch_sat, scra, scut, scuttle_ship, scuttle_land, knockdown)
(ac_planedamage, detonate, attack_val, defense_val, air_damage)
(msl_intercept, msl_launch_mindam, pln_prewrite, shp_prewrite):
Simplify unit destruction: just zero efficiency, leave makelost()
etc. to the prewrite callback.
instead of declaring them all over the place. This uncovered type
errors:
(s_p_etu, adj_update): Defined long, sometimes declared int. Kills
big endian machines where sizeof(long) != sizeof(int). Change to
int.
(set_option, delete_option, optstrset, intset, floatset, doubleset,
longset, optionset, optiondel, worldxset): Change linkage to static.
