From f7dc15b64fb36eddc662707d1936095973865340 Mon Sep 17 00:00:00 2001
From: Enno Rehling <enno.rehling@gmail.com>
Date: Wed, 12 Aug 2020 20:07:17 +0200
Subject: [PATCH] move_cmd caller no longer has to call init_order.

refactor move_cmd to be more like other command processors.
---
 src/kernel/unit.c |  2 --
 src/move.c        | 24 +++++++++++++++---------
 src/move.h        |  1 +
 src/move.test.c   |  4 +---
 src/piracy.c      | 11 +----------
 src/piracy.test.c |  6 +++---
 6 files changed, 21 insertions(+), 27 deletions(-)

diff --git a/src/kernel/unit.c b/src/kernel/unit.c
index 99f1d76ad..91a6f4803 100644
--- a/src/kernel/unit.c
+++ b/src/kernel/unit.c
@@ -1881,8 +1881,6 @@ bool translate_order(order *ord, const struct locale *from_lang, const struct lo
     default:
         return false;
     }
-
-    return false;
 }
 
 void translate_orders(unit *u, const struct locale *lang, order **list, bool del)
diff --git a/src/move.c b/src/move.c
index 173aa709d..f0b38bb48 100644
--- a/src/move.c
+++ b/src/move.c
@@ -2175,9 +2175,17 @@ static void travel(unit * u, order *ord)
     }
 }
 
-void move_cmd(unit * u, order * ord)
+void move_cmd_ex(unit * u, order * ord, const char *directions)
 {
     assert(u->number);
+
+    if (directions) {
+        /* Befehl ignorieren und Parsing vom String initialisieren. */
+        init_tokens_str(directions);
+    }
+    else {
+        init_order(ord, u->faction->locale);
+    }
     if (u->ship && u == ship_owner(u->ship)) {
         bool drifting = (getkeyword(ord) == K_MOVE);
         sail(u, ord, drifting);
@@ -2187,6 +2195,11 @@ void move_cmd(unit * u, order * ord)
     }
 
     fset(u, UFL_LONGACTION | UFL_NOTMOVING);
+}
+
+void move_cmd(unit * u, order * ord)
+{
+    move_cmd_ex(u, ord, NULL);
     set_order(&u->thisorder, NULL);
 }
 
@@ -2266,8 +2279,6 @@ int follow_ship(unit * u, order * ord)
     }
 
     sbs_init(&sbcmd, command, sizeof(command));
-    sbs_strcat(&sbcmd, LOC(u->faction->locale, keyword(K_MOVE)));
-    sbs_strcat(&sbcmd, " ");
     sbs_strcat(&sbcmd, LOC(u->faction->locale, directions[dir]));
 
     moves = 1;
@@ -2292,11 +2303,8 @@ int follow_ship(unit * u, order * ord)
 
     /* In command steht jetzt das NACH-Kommando. */
 
-    /* NACH ignorieren und Parsing initialisieren. */
-    init_tokens_str(command);
-    getstrtoken();
     /* NACH ausfuehren */
-    move_cmd(u, ord);
+    move_cmd_ex(u, ord, command);
     return 1;                     /* true -> Einheitenliste von vorne durchgehen */
 }
 
@@ -2446,13 +2454,11 @@ void movement(void)
                     else {
                         if (ships) {
                             if (u->ship && ship_owner(u->ship) == u) {
-                                init_order_depr(u->thisorder);
                                 move_cmd(u, u->thisorder);
                             }
                         }
                         else {
                             if (!u->ship || ship_owner(u->ship) != u) {
-                                init_order_depr(u->thisorder);
                                 move_cmd(u, u->thisorder);
                             }
                         }
diff --git a/src/move.h b/src/move.h
index 4362091b2..333514862 100644
--- a/src/move.h
+++ b/src/move.h
@@ -69,6 +69,7 @@ extern "C" {
         const struct region *dest);
     bool can_takeoff(const struct ship * sh, const struct region * from, const struct region * to);
     void move_cmd(struct unit * u, struct order * ord);
+    void move_cmd_ex(struct unit * u, struct order * ord, const char *directions);
     int follow_ship(struct unit * u, struct order * ord);
 
 #define SA_HARBOUR 1
diff --git a/src/move.test.c b/src/move.test.c
index cfcd692db..0e942251e 100644
--- a/src/move.test.c
+++ b/src/move.test.c
@@ -550,7 +550,7 @@ static void test_follow_ship_msg(CuTest * tc) {
         "unit:unit", "region:region", "command:order", MT_NEW_END);
 
     init_order_depr(ord);
-    getstrtoken();
+    skip_token();
 
     follow_ship(u, ord);
 
@@ -651,7 +651,6 @@ static void test_route_cycle(CuTest *tc) {
     CuAssertIntEquals(tc, RCF_WALK, u->_race->flags & RCF_WALK);
     u->orders = create_order(K_ROUTE, u->faction->locale, "WEST EAST NW");
     CuAssertStrEquals(tc, "route WEST EAST NW", get_command(u->orders, lang, buffer, sizeof(buffer)));
-    init_order(u->orders, u->faction->locale);
     move_cmd(u, u->orders);
     CuAssertPtrNotNull(tc, test_find_messagetype(u->faction->msgs, "moveblocked"));
     CuAssertIntEquals(tc, 1, u->region->x);
@@ -700,7 +699,6 @@ static void test_route_pause(CuTest *tc) {
     CuAssertIntEquals(tc, RCF_WALK, u->_race->flags & RCF_WALK);
     u->orders = create_order(K_ROUTE, u->faction->locale, "PAUSE EAST NW");
     CuAssertStrEquals(tc, "route PAUSE EAST NW", get_command(u->orders, lang, buffer, sizeof(buffer)));
-    init_order(u->orders, u->faction->locale);
     move_cmd(u, u->orders);
     CuAssertIntEquals(tc, 2, u->region->x);
     CuAssertStrEquals(tc, "route PAUSE EAST NW", get_command(u->orders, lang, buffer, sizeof(buffer)));
diff --git a/src/piracy.c b/src/piracy.c
index ced1f61aa..77121aade 100644
--- a/src/piracy.c
+++ b/src/piracy.c
@@ -114,7 +114,6 @@ direction_t find_piracy_target(unit *u, int *il) {
 
 void piracy_cmd(unit * u)
 {
-    order *ord;
     region *r = u->region;
     ship *sh = u->ship, *sh2;
     direction_t target_dir;
@@ -200,15 +199,7 @@ void piracy_cmd(unit * u)
     ADDMSG(&u->faction->msgs, msg_message("piratesawvictim",
         "ship unit region dir", sh, u, r, target_dir));
 
-    /* Befehl konstruieren */
-    /* TODO: why change u->thisorder? */
-    /* FIXME: when u->thisorder == ord, set_order calls free, destroys both. */
-    ord = create_order(K_MOVE, u->faction->locale, "%s", LOC(u->faction->locale, directions[target_dir]));
-
-    /* Bewegung ausfuehren */
-    init_order_depr(ord);
-    move_cmd(u, ord);
-    free_order(ord);
+    move_cmd_ex(u, u->thisorder, LOC(u->faction->locale, directions[target_dir]));
 }
 
 void age_piracy(region *r) {
diff --git a/src/piracy.test.c b/src/piracy.test.c
index 74e79103d..ef5971a6d 100644
--- a/src/piracy.test.c
+++ b/src/piracy.test.c
@@ -108,7 +108,7 @@ static void test_piracy_cmd(CuTest * tc) {
     u->thisorder = create_order(K_PIRACY, f->locale, "%s", itoa36(u2->faction->no));
 
     piracy_cmd(u);
-    CuAssertPtrEquals(tc, NULL, u->thisorder);
+    CuAssertPtrNotNull(tc, u->thisorder);
     CuAssertTrue(tc, u->region != r);
     CuAssertPtrEquals(tc, u2->region, u->region);
     CuAssertPtrEquals(tc, u2->region, u->ship->region);
@@ -213,7 +213,7 @@ static void test_piracy_cmd_land_to_land(CuTest * tc) {
     u->thisorder = create_order(K_PIRACY, f->locale, "%s", itoa36(target));
 
     piracy_cmd(u);
-    CuAssertPtrEquals(tc, NULL, u->thisorder);
+    CuAssertPtrNotNull(tc, u->thisorder);
     CuAssertPtrEquals(tc, r, u->region);
 
     test_teardown();
@@ -228,7 +228,7 @@ static void test_piracy_cmd_swimmer(CuTest * tc) {
     r = pirate->region;
 
     piracy_cmd(pirate);
-    CuAssertPtrEquals(tc, NULL, pirate->thisorder);
+    CuAssertPtrNotNull(tc, pirate->thisorder);
     CuAssertTrue(tc, pirate->region != r);
     CuAssertPtrEquals(tc, victim->region, pirate->region);
     CuAssertPtrNotNullMsg(tc, "successful PIRACY message", test_find_messagetype(pirate->faction->msgs, "piratesawvictim"));