commands: Rename the command functions Command functions are traditionally named like the command shortened to four characters. When this name collides with a keyword or library function, we abbreviate more: brea(), rea(). A few are unabbreviated, e.g. execute(). A few have different names, e.g. explain(), not list(). Commit 23726b379 (v4.3.0) suppressed a GCC warning about carg() colliding with its built-in function. Ron Koenderink reported Microsoft Visual Studio 2019 fails to link: "_carg already defined in ucrtd.lib(ucrtbased.dll)". Time to clean this up: rename the functions to c_FOO(), where FOO is the unabbreviated name of the command. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Update copyright notice Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Update copyright notice Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Update copyright notice Signed-off-by: Markus Armbruster <armbru@redhat.com>
navigate march: Say "to sweep mines" instead of "to minesweep" The choice of "to minesweep" in "`d' to drop mines, and `m' to minesweep" is obviously intentional. But saying it in standard English instead is at least as clear, so do that. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Update copyright notice Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Include "file.h" where it's needed Several headers define macros that use ef_ptr() without including "file.h". Fix that. Drop redundant inclusions elsewhere. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Update copyright notice Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
navigate march: Fix abort not to wipe out concurrent updates When the player aborts the command at the movement prompt, we write back stale ships or land units, triggering a generation oops. Any updates made by other threads meanwhile are wiped out, triggering a seqno mismatch oops. Broken in commit 24000b4, v4.3.33. Fix by restoring the lost shp_nav_stay_behind() and lnd_mar_stay_behind() calls. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
navigate march: Plug memory leaks When the player aborts the command at the movement prompt, or declines to abandon a sector, unit_move() returns without freeing the list. Found with valgrind. Broken in commit 24000b4 and commit 7c1b166, both v4.3.33. Free the list on these returns, too. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Change comment style to use @foo rather than FOO ... when referring to a function's parameter or a struct/union's member. The idea of using FOO comes from the GNU coding standards: The comment on a function is much clearer if you use the argument names to speak about the argument values. The variable name itself should be lower case, but write it in upper case when you are speaking about the value rather than the variable itself. Thus, "the inode number NODE_NUM" rather than "an inode". Upcasing names is problematic for a case-sensitive language like C, because it can create ambiguity. Moreover, it's too much shouting for my taste. GTK-Doc's convention to prefix the identifier with @ makes references to variables stand out nicely. The rest of the GTK-Doc conventions make no sense for us, however. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
retreat: Rewrite automatic retreat code to fix its many bugs Much of the retreat code duplicates navigate and march code. Worse, retreat's version is full of bugs: * Land units can sometimes retreat when they couldn't march: while on the trading block (forbidden with march since 4.0.9), crewless (likewise since 4.0.0), kidnapped in a foreign sector (inconsistent since land units were added in Chainsaw 3), loaded on a ship (likewise) or a land unit (inconsistent since trains were added in 4.0.0). * Ships can retreat while on the trading block (forbidden with navigate since 4.0.9) * Land units can't retreat into foreign sectors even though they could march there, namely when sector is allied or the land unit is a spy. They can march there since 4.0.0. * Land units keep their fortification on retreat. Has been that way since retreat was added in Chainsaw. Then there's group retreat. It's basically crazy: * It triggers retreat for everyone in the same fleet or army, one after the other, regardless of retreat path, conditions (including group retreat), or even location. The latter is quite abusable since retreats aren't interdicted. Has been that way since retreat was added in Chainsaw. * Group retreat fails to trigger when the originally retreating ship or land unit has no retreat path left when it's done. Broken in commit b860123. Finally, the reporting to the owner is sub-par: * When a retreat is cut short by insufficient mobility or obstructions, its end sector isn't reported, leaving the player guessing. * Non-retreats can be confusingly reported as retreat to the same sector. Can happen when the retreat path starts with 'h' (obscure feature to suppress a single retreat), or when a group retreat includes a ship or land unit without retreat orders. * Interaction with mines during retreat is reported before the retreat itself, which can be quite confusing. * Sweeping landmines isn't reported at all. * Much code and much bulletin text is dedicated to reporting what caused the retreat, even though it should be perfectly obvious. Rewrite this on top of common navigate and march code. Reuse of common code fixes the "can retreat when it couldn't navigate/march" and the "can't retreat into sectors it could navigate or march into" bugs, and improves the reporting. One special case isn't a bug fix but a rule change: mountains. The old code forbids that explicitly, and it's clearly intentional, if undocumented. The new code allows it by not doing anything special. Turn group retreat into an actual group retreat: everyone in the same fleet and sector with the the same retreat path and group retreat condition joins the group. The group retreats together, just like in navigate and march. Take care to always report the end sector. When retreat is impossible, report "can't retreat". When retreat is partial, report "and stays in X,Y". When it's complete, report "stopped at X,Y". Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
subs: Rename unit_path() to unit_move_route() and move it Move it next to its only caller unit_move(). Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
subs: Rename shp_nav(), lnd_mar() ... to shp_nav_stay_behind(), lnd_mar_stay_behind(), to better reflect their purpose. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
navigate march: Fix use-after-free and other bugs unit_move() is too big and has too many paths through its loop. Maintenance of the (unspoken) loop invariant isn't obvious. In fact, it isn't maintained on some paths. I found several bugs: * We check prerequisite conditions for moving before the first move and around prompts. When a condition becomes wrong on the move, movement continues all the same until the next prompt. I believe the only way this can happen is loss of crew due to hitting a mine. * We cache ships and land units in a list of struct ulist. When a ship or land unit gets left behind, its node is removed from the list and freed. We keep pointer flg pointing to the flagship in that list for convenience. However, the pointer isn't updated until the next prompt. It's referenced for automatic radar and all sub-commands other than the six directions and 'h'. Use after free when such a sub-command gets processed after a flagship change without a prompt. Same for land units. For instance, navigating a pair of ships "jh" where the flagship has no mobility leaves the flagship behind, then attempts to radar automatically using the ship in the freed list node. Likewise, marching a similar pair of land units "jl" examines the land unit in the freed list node to figure out how to look. * We cache mobility in the same list to support fractional mobility during movement. Movement deducts from cached mobility and writes the result back to the ship or land unit. If something else charges it mobility while it's in this list, the cache becomes stale. shp_nav() and lnd_nav() reload stale caches, but don't run often enough. For instance, when a ship hits mines, the mine damage makes the cache stale. If a direction or 'h' follows directly, the stale mobility is written back, clobbering the mine hit's mobility loss. This mess dates back to Empire 2, where it replaced a different mess. There may be more bugs. unit_move()'s complex control flow makes reasoning about its loop invariant too error-prone. Rewrite the mess instead, splitting off sensible subroutines. Also fixes a couple of minor annoyances: * White-space can confuse the parser. For instance, "jg l" is interpreted like "jgll". Fix to reject the space. Broken in commit 0c12d83, v4.3.7. * The flagship uses radar automatically before any sub-command (since Chainsaw), and all ships use it automatically after a move (since 4.2.2). Make them all use it before and after each sub-command, whether it's a move or not. * Land units don't use radar automatically. Make them use it just like ships. * Always report a flagship / leader change right when it happens, not only before and after a prompt. Left for another day, marked FIXME: BTU charging is unclean. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
subs: Move shared code from navi.c to subs Rename do_unit_move() to unit_move() to blend into unitsub.c. Give its helpers static linkage. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
Update copyright notice Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
unit: Drop ulist member chrp 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>
subs: Drop unit_path() parameter together It's always non-zero now. Signed-off-by: Markus Armbruster <armbru@pond.sub.org>
sail: Remove option SAIL 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>