<URL: http://bugs.freeciv.org/Ticket/Display.html?id=39792 >
Unfortunately, it appears that the activity and explore code has so many
problems that it's not practical to completely fix at this time.
One of the most egregious problems is that folks didn't always use access
functions for setting activities, and/or didn't uniformly use the same
functions. In this case, one code path set ACTIVITY_IDLE directly with
set_unit_activity(), while another used an intermediate function that in
turn calls unit_activity_dependencies(). They achieve different results....
There are far too many such paths used for all activities. This will need
considerable cleanup to find them all, such as PR#39870.
With 24 set_unit_activity() call variants, 44 unit_activity_handling()
call variants, and 31 ai.done references, not to mention 198 ai.control
references, taming this beast will take some doing!
===
I've checked in this pair of patches (for 2.1 and 2.2) to at least allow
the current savegame(s) to go forward without crashing.
First and foremost, combine the similar code paths into do_explore(), which
will indirectly call unit_activity_dependencies().
Secondly, but perhaps more importantly, use enum unit_move_result returns
for ai_manage_explorer(), and pass along the new MR_DEATH result. This
eliminates several crashing bugs where ai.done was set for dead units.
More FIXME added for future work!
Committed S2_1 revision 14246.
Committed S2_2 revision 14247.
Committed trunk revision 14248.
Index: server/unittools.c
===================================================================
--- server/unittools.c (revision 14245)
+++ server/unittools.c (working copy)
@@ -652,24 +652,7 @@
unit_restore_movepoints(pplayer, punit);
if (activity == ACTIVITY_EXPLORE) {
- bool more_to_explore = ai_manage_explorer(punit);
-
- if (!player_find_unit_by_id(pplayer, id)) {
- /* Died */
- return;
- }
-
- /* ai_manage_explorer isn't supposed to change the activity but we
- * don't count on this. */
- if (punit->activity != ACTIVITY_EXPLORE || !more_to_explore) {
- handle_unit_activity_request(punit, ACTIVITY_IDLE);
-
- /* FIXME: When the ai_manage_explorer call changes the activity from
- * EXPLORE to IDLE, then for some reason the ai.control value gets left
- * set. We reset it here. See PR#12931. */
- punit->ai.control = FALSE;
- }
- send_unit_info(NULL, punit);
+ do_explore(punit);
return;
}
@@ -2041,6 +2024,35 @@
}
/**************************************************************************
+ ...
+**************************************************************************/
+void do_explore(struct unit *punit)
+{
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ return;
+ case MR_OK:
+ /* FIXME: ai_manage_explorer() isn't supposed to change the activity,
+ * but don't count on this. See PR#39792.
+ */
+ if (punit->activity == ACTIVITY_EXPLORE) {
+ break;
+ }
+ /* fallthru */
+ default:
+ handle_unit_activity_request(punit, ACTIVITY_IDLE);
+
+ /* FIXME: When the ai_manage_explorer() call changes the activity from
+ * EXPLORE to IDLE, in unit_activity_handling() ai.control is left
+ * alone. We reset it here. See PR#12931. */
+ punit->ai.control = FALSE;
+ break;
+ };
+ send_unit_info(NULL, punit); /* probably duplicate */
+}
+
+/**************************************************************************
Returns whether the drop was made or not. Note that it also returns 1
in the case where the drop was succesful, but the unit was killed by
barbarians in a hut.
Index: server/unittools.h
===================================================================
--- server/unittools.h (revision 14245)
+++ server/unittools.h (working copy)
@@ -81,6 +81,7 @@
/* doing a unit activity */
void do_nuclear_explosion(struct player *pplayer, struct tile *ptile);
bool do_airline(struct unit *punit, struct city *city2);
+void do_explore(struct unit *punit);
bool do_paradrop(struct unit *punit, struct tile *ptile);
void load_unit_onto_transporter(struct unit *punit, struct unit *ptrans);
void unload_unit_from_transporter(struct unit *punit);
Index: server/unithand.c
===================================================================
--- server/unithand.c (revision 14245)
+++ server/unithand.c (working copy)
@@ -591,34 +591,35 @@
return;
}
- if (punit->activity != activity ||
- punit->activity_target != activity_target ||
- punit->ai.control) {
+ if (punit->activity == activity
+ && punit->activity_target == activity_target
+ && !punit->ai.control) {
/* Treat change in ai.control as change in activity, so
- * idle autosettlers behave correctly when selected --dwp
+ * idle autosettlers behave correctly when selected --dwp
*/
- punit->ai.control = FALSE;
- punit->goto_tile = NULL;
+ return;
+ }
+
+ punit->ai.control = FALSE;
+ punit->goto_tile = NULL;
+
+ switch (activity) {
+ case ACTIVITY_EXPLORE:
handle_unit_activity_request_targeted(punit, activity, activity_target);
/* Exploring is handled here explicitly, since the player expects to
* see an immediate response from setting a unit to auto-explore.
* Handling it deeper in the code leads to some tricky recursive loops -
* see PR#2631. */
- if (punit->moves_left > 0 && activity == ACTIVITY_EXPLORE) {
- int id = punit->id;
- bool more_to_explore = ai_manage_explorer(punit);
+ if (punit->moves_left > 0) {
+ do_explore(punit);
+ }
+ break;
- if ((punit = game_find_unit_by_number(id))) {
- assert(punit->activity == ACTIVITY_EXPLORE);
- if (!more_to_explore) {
- set_unit_activity(punit, ACTIVITY_IDLE);
- punit->ai.control = FALSE;
- }
- send_unit_info(NULL, punit);
- }
- }
- }
+ default:
+ handle_unit_activity_request_targeted(punit, activity, activity_target);
+ break;
+ };
}
/**************************************************************************
@@ -952,23 +953,24 @@
}
/**************************************************************************
-...
+ see also aiunit could_unit_move_to_tile()
**************************************************************************/
static bool can_unit_move_to_tile_with_notify(struct unit *punit,
struct tile *dest_tile,
bool igzoc)
{
- enum unit_move_result reason;
struct tile *src_tile = punit->tile;
-
- reason =
+ enum unit_move_result reason =
test_unit_move_to_tile(unit_type(punit), unit_owner(punit),
punit->activity,
- punit->tile, dest_tile, igzoc);
- if (reason == MR_OK)
+ src_tile, dest_tile, igzoc);
+
+ switch (reason) {
+ case MR_OK:
return TRUE;
- if (reason == MR_BAD_TYPE_FOR_CITY_TAKE_OVER) {
+ case MR_BAD_TYPE_FOR_CITY_TAKE_OVER:
+ {
const char *units_str = role_units_translations(F_MARINES);
if (units_str) {
notify_player(unit_owner(punit), src_tile,
@@ -979,21 +981,33 @@
notify_player(unit_owner(punit), src_tile,
E_BAD_COMMAND, _("Cannot attack from sea."));
}
- } else if (reason == MR_NO_WAR) {
+ break;
+ }
+
+ case MR_NO_WAR:
notify_player(unit_owner(punit), src_tile,
E_BAD_COMMAND,
_("Cannot attack unless you declare war first."));
- } else if (reason == MR_ZOC) {
+ break;
+
+ case MR_ZOC:
notify_player(unit_owner(punit), src_tile, E_BAD_COMMAND,
_("%s can only move into your own zone of control."),
unit_name_translation(punit));
- } else if (reason == MR_PEACE) {
+ break;
+
+ case MR_PEACE:
notify_player(unit_owner(punit), src_tile, E_BAD_COMMAND,
_("Cannot invade unless you break peace with "
"%s first."),
player_name(tile_owner(dest_tile)));
- }
+ break;
+ default:
+ /* FIXME: need more explanations someday! */
+ break;
+ };
+
return FALSE;
}
Index: server/unithand.h
===================================================================
--- server/unithand.h (revision 14245)
+++ server/unithand.h (working copy)
@@ -13,7 +13,6 @@
#ifndef FC__UNITHAND_H
#define FC__UNITHAND_H
-#include "packets.h"
#include "unit.h"
#include "hand_gen.h"
Index: common/unit.h
===================================================================
--- common/unit.h (revision 14245)
+++ common/unit.h (working copy)
@@ -89,14 +89,19 @@
};
enum unit_move_result {
- MR_OK, MR_BAD_TYPE_FOR_CITY_TAKE_OVER,
+ MR_OK,
+ MR_DEATH,
+ MR_PAUSE,
+ MR_BAD_TYPE_FOR_CITY_TAKE_OVER,
MR_NO_WAR, /* Can't move here without declaring war. */
MR_PEACE, /* Can't move here because of a peace treaty. */
MR_ZOC,
- MR_BAD_ACTIVITY, MR_BAD_DESTINATION, MR_BAD_MAP_POSITION,
+ MR_BAD_ACTIVITY,
+ MR_BAD_DESTINATION,
+ MR_BAD_MAP_POSITION,
+ MR_DESTINATION_OCCUPIED_BY_NON_ALLIED_CITY,
MR_DESTINATION_OCCUPIED_BY_NON_ALLIED_UNIT,
MR_NO_SEA_TRANSPORTER_CAPACITY,
- MR_DESTINATION_OCCUPIED_BY_NON_ALLIED_CITY
};
enum add_build_city_result {
Index: ai/aiferry.c
===================================================================
--- ai/aiferry.c (revision 14245)
+++ ai/aiferry.c (working copy)
@@ -1012,11 +1012,19 @@
}
UNIT_LOG(LOGLEVEL_FERRY, punit, "Passing control of ferry to explorer code");
- if (!ai_manage_explorer(punit)) {
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ return;
+ case MR_OK:
+ /* FIXME: continue moving? */
+ break;
+ default:
punit->ai.done = TRUE;
- }
+ break;
+ };
- if (game_find_unit_by_number(sanity) && punit->moves_left > 0) {
+ if (punit->moves_left > 0) {
struct city *pcity = find_nearest_safe_city(punit);
if (pcity) {
punit->goto_tile = pcity->tile;
Index: ai/aiexplorer.c
===================================================================
--- ai/aiexplorer.c (revision 14245)
+++ ai/aiexplorer.c (working copy)
@@ -229,9 +229,12 @@
Handle eXplore mode of a unit (explorers are always in eXplore mode
for AI) - explores unknown territory, finds huts.
- Returns whether there is any more territory to be explored.
+ MR_OK: there is more territory to be explored.
+ MR_DEATH: unit died.
+ MR_PAUSE: unit cannot explore further now.
+ Other results: unit cannot explore further.
**************************************************************************/
-bool ai_manage_explorer(struct unit *punit)
+enum unit_move_result ai_manage_explorer(struct unit *punit)
{
struct player *pplayer = unit_owner(punit);
/* Loop prevention */
@@ -265,7 +268,7 @@
if (pplayer->ai.control && unit_has_type_flag(punit, F_GAMELOSS)) {
UNIT_LOG(LOG_DEBUG, punit, "exploration too dangerous!");
- return FALSE; /* too dangerous */
+ return MR_BAD_ACTIVITY; /* too dangerous */
}
TIMING_LOG(AIT_EXPLORER, TIMER_START);
@@ -344,7 +347,7 @@
* which goes beside the unknown, with a good EC callback... */
if (!ai_unit_goto(punit, best_tile)) {
/* Died? Strange... */
- return FALSE;
+ return MR_DEATH;
}
UNIT_LOG(LOG_DEBUG, punit, "exploration GOTO succeeded");
if (punit->moves_left > 0) {
@@ -368,18 +371,18 @@
/* we're a trireme with non-full complement of movement points,
* so wait until next turn. */
UNIT_LOG(LOG_DEBUG, punit, "done exploring (had to hold)...");
- return TRUE;
+ return MR_OK;
}
UNIT_LOG(LOG_DEBUG, punit, "done exploring (all finished)...");
- return FALSE;
+ return MR_PAUSE;
}
}
UNIT_LOG(LOG_DEBUG, punit, "done exploring (but more go go)...");
- return TRUE;
+ return MR_OK;
} else {
/* Didn't find anything. */
UNIT_LOG(LOG_DEBUG, punit, "failed to explore more");
- return FALSE;
+ return MR_BAD_MAP_POSITION;
}
#undef DIST_FACTOR
}
Index: ai/aiexplorer.h
===================================================================
--- ai/aiexplorer.h (revision 14245)
+++ ai/aiexplorer.h (working copy)
@@ -13,17 +13,10 @@
#ifndef FC__AIEXPLORER_H
#define FC__AIEXPLORER_H
-#include "shared.h" /* bool type */
+#include "unit.h"
-#include "fc_types.h"
-/*
- * Handle eXplore mode of a unit (explorers are always in eXplore mode
- * for AI) - explores unknown territory, finds huts.
- *
- * Returns whether there is any more territory to be explored.
- */
-bool ai_manage_explorer(struct unit *punit);
+enum unit_move_result ai_manage_explorer(struct unit *punit);
/*
* TODO: Enable AI build new explorers. Either (the old way) write a
Index: ai/aiunit.c
===================================================================
--- ai/aiunit.c (revision 14245)
+++ ai/aiunit.c (working copy)
@@ -190,24 +190,28 @@
0 if can't move
1 if zoc_ok
-1 if zoc could be ok?
+
+ see also unithand can_unit_move_to_tile_with_notify()
**************************************************************************/
int could_unit_move_to_tile(struct unit *punit, struct tile *dest_tile)
{
- enum unit_move_result result;
-
- result =
+ enum unit_move_result reason =
test_unit_move_to_tile(unit_type(punit), unit_owner(punit),
ACTIVITY_IDLE, punit->tile,
dest_tile, unit_has_type_flag(punit, F_IGZOC));
- if (result == MR_OK) {
+ switch (reason) {
+ case MR_OK:
return 1;
- }
- if (result == MR_ZOC) {
+ case MR_ZOC:
if (could_be_my_zoc(punit, punit->tile)) {
return -1;
}
- }
+ break;
+
+ default:
+ break;
+ };
return 0;
}
@@ -1648,6 +1652,7 @@
/*************************************************************************
Go berserk, assuming there are no targets nearby.
TODO: Is it not possible to remove this special casing for barbarians?
+ FIXME: enum unit_move_result
**************************************************************************/
static void ai_military_attack_barbarian(struct player *pplayer,
struct unit *punit)
@@ -1802,11 +1807,17 @@
(void) ai_unit_goto(punit, pcity->tile);
} else if (!is_barbarian(pplayer)) {
/* Nothing else to do, so try exploring. */
- if (ai_manage_explorer(punit)) {
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ return;
+ case MR_OK:
UNIT_LOG(LOG_DEBUG, punit, "nothing else to do, so exploring");
- } else if (game_find_unit_by_number(id)) {
+ break;
+ default:
UNIT_LOG(LOG_DEBUG, punit, "nothing to do - no more exploring either");
- }
+ break;
+ };
} else {
/* You can still have some moves left here, but barbarians should
not sit helplessly, but advance towards nearest known enemy city */
@@ -2073,7 +2084,18 @@
TIMING_LOG(AIT_BODYGUARD, TIMER_STOP);
break;
case AIUNIT_EXPLORE:
- punit->ai.done = !(ai_manage_explorer(punit) && punit->moves_left > 0);
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ return;
+ case MR_OK:
+ UNIT_LOG(LOG_DEBUG, punit, "more exploring");
+ break;
+ default:
+ UNIT_LOG(LOG_DEBUG, punit, "no more exploring either");
+ break;
+ };
+ punit->ai.done = (punit->moves_left <= 0);
break;
case AIUNIT_RECOVER:
TIMING_LOG(AIT_RECOVER, TIMER_START);
@@ -2224,15 +2246,20 @@
TIMING_LOG(AIT_MILITARY, TIMER_STOP);
return;
} else {
- int id = punit->id;
-
- UNIT_LOG(LOG_DEBUG, punit, "fell through all unit tasks, defending");
/* what else could this be? -- Syela */
- if (!ai_manage_explorer(punit)
- && game_find_unit_by_number(id)) {
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ break;
+ case MR_OK:
+ UNIT_LOG(LOG_DEBUG, punit, "now exploring");
+ break;
+ default:
+ UNIT_LOG(LOG_DEBUG, punit, "fell through all unit tasks, defending");
ai_unit_new_role(punit, AIUNIT_DEFEND_HOME, NULL);
ai_military_defend(pplayer, punit);
- }
+ break;
+ };
return;
}
}
Index: server/unittools.c
===================================================================
--- server/unittools.c (revision 14245)
+++ server/unittools.c (working copy)
@@ -709,27 +709,8 @@
break;
case ACTIVITY_EXPLORE:
- {
- bool more_to_explore = ai_manage_explorer(punit);
-
- if (!player_find_unit_by_id(pplayer, id)) {
- /* Died */
- return;
- }
-
- /* ai_manage_explorer isn't supposed to change the activity but we
- * don't count on this. */
- if (punit->activity != ACTIVITY_EXPLORE || !more_to_explore) {
- unit_activity_handling(punit, ACTIVITY_IDLE);
-
- /* FIXME: When the ai_manage_explorer call changes the activity from
- * EXPLORE to IDLE, then for some reason the ai.control value gets left
- * set. We reset it here. See PR#12931. */
- punit->ai.control = FALSE;
- }
- send_unit_info(NULL, punit);
+ do_explore(punit);
return;
- }
case ACTIVITY_PILLAGE:
if (punit->activity_target == S_LAST) { /* case for old save files */
@@ -2109,6 +2090,35 @@
}
/**************************************************************************
+ ...
+**************************************************************************/
+void do_explore(struct unit *punit)
+{
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ return;
+ case MR_OK:
+ /* FIXME: ai_manage_explorer() isn't supposed to change the activity,
+ * but don't count on this. See PR#39792.
+ */
+ if (punit->activity == ACTIVITY_EXPLORE) {
+ break;
+ }
+ /* fallthru */
+ default:
+ unit_activity_handling(punit, ACTIVITY_IDLE);
+
+ /* FIXME: When the ai_manage_explorer() call changes the activity from
+ * EXPLORE to IDLE, in unit_activity_handling() ai.control is left
+ * alone. We reset it here. See PR#12931. */
+ punit->ai.control = FALSE;
+ break;
+ };
+ send_unit_info(NULL, punit); /* probably duplicate */
+}
+
+/**************************************************************************
Returns whether the drop was made or not. Note that it also returns 1
in the case where the drop was succesful, but the unit was killed by
barbarians in a hut.
Index: server/unittools.h
===================================================================
--- server/unittools.h (revision 14245)
+++ server/unittools.h (working copy)
@@ -81,6 +81,7 @@
/* doing a unit activity */
void do_nuclear_explosion(struct player *pplayer, struct tile *ptile);
bool do_airline(struct unit *punit, struct city *city2);
+void do_explore(struct unit *punit);
bool do_paradrop(struct unit *punit, struct tile *ptile);
void load_unit_onto_transporter(struct unit *punit, struct unit *ptrans);
void unload_unit_from_transporter(struct unit *punit);
Index: server/unithand.c
===================================================================
--- server/unithand.c (revision 14245)
+++ server/unithand.c (working copy)
@@ -669,44 +669,44 @@
return;
}
- if (punit->activity != activity
- || punit->activity_target != activity_target
- || punit->activity_base != activity_base
- || punit->ai.control) {
+ if (punit->activity == activity
+ && punit->activity_target == activity_target
+ && punit->activity_base == activity_base
+ && !punit->ai.control) {
/* Treat change in ai.control as change in activity, so
- * idle autosettlers behave correctly when selected --dwp
+ * idle autosettlers behave correctly when selected --dwp
*/
- punit->ai.control = FALSE;
- punit->goto_tile = NULL;
+ return;
+ }
- if (activity != ACTIVITY_BASE) {
- unit_activity_handling_targeted(punit, activity, activity_target);
- } else {
- if (!base_by_number(activity_base)) {
- /* Illegal base type */
- return;
- }
- unit_activity_handling_base(punit, activity_base);
+ punit->ai.control = FALSE;
+ punit->goto_tile = NULL;
+
+ switch (activity) {
+ case ACTIVITY_BASE:
+ if (!base_by_number(activity_base)) {
+ /* Illegal base type */
+ return;
}
+ unit_activity_handling_base(punit, activity_base);
+ break;
+ case ACTIVITY_EXPLORE:
+ unit_activity_handling_targeted(punit, activity, activity_target);
+
/* Exploring is handled here explicitly, since the player expects to
* see an immediate response from setting a unit to auto-explore.
* Handling it deeper in the code leads to some tricky recursive loops -
* see PR#2631. */
- if (punit->moves_left > 0 && activity == ACTIVITY_EXPLORE) {
- int id = punit->id;
- bool more_to_explore = ai_manage_explorer(punit);
+ if (punit->moves_left > 0) {
+ do_explore(punit);
+ }
+ break;
- if ((punit = game_find_unit_by_number(id))) {
- assert(punit->activity == ACTIVITY_EXPLORE);
- if (!more_to_explore) {
- set_unit_activity(punit, ACTIVITY_IDLE);
- punit->ai.control = FALSE;
- }
- send_unit_info(NULL, punit);
- }
- }
- }
+ default:
+ unit_activity_handling_targeted(punit, activity, activity_target);
+ break;
+ };
}
/**************************************************************************
@@ -1072,23 +1072,24 @@
}
/**************************************************************************
-...
+ see also aiunit could_unit_move_to_tile()
**************************************************************************/
static bool can_unit_move_to_tile_with_notify(struct unit *punit,
struct tile *dest_tile,
bool igzoc)
{
- enum unit_move_result reason;
struct tile *src_tile = punit->tile;
-
- reason =
+ enum unit_move_result reason =
test_unit_move_to_tile(unit_type(punit), unit_owner(punit),
punit->activity,
- punit->tile, dest_tile, igzoc);
- if (reason == MR_OK)
+ src_tile, dest_tile, igzoc);
+
+ switch (reason) {
+ case MR_OK:
return TRUE;
- if (reason == MR_BAD_TYPE_FOR_CITY_TAKE_OVER) {
+ case MR_BAD_TYPE_FOR_CITY_TAKE_OVER:
+ {
const char *units_str = role_units_translations(F_MARINES);
if (units_str) {
notify_player(unit_owner(punit), src_tile,
@@ -1099,25 +1100,39 @@
notify_player(unit_owner(punit), src_tile,
E_BAD_COMMAND, _("Cannot attack from sea."));
}
- } else if (reason == MR_NO_WAR) {
+ break;
+ }
+
+ case MR_NO_WAR:
notify_player(unit_owner(punit), src_tile,
E_BAD_COMMAND,
_("Cannot attack unless you declare war first."));
- } else if (reason == MR_ZOC) {
+ break;
+
+ case MR_ZOC:
notify_player(unit_owner(punit), src_tile, E_BAD_COMMAND,
_("%s can only move into your own zone of control."),
unit_name_translation(punit));
- } else if (reason == MR_TRIREME) {
+ break;
+
+ case MR_TRIREME:
notify_player(unit_owner(punit), src_tile, E_BAD_COMMAND,
_("%s cannot move that far from the coast line."),
unit_name_translation(punit));
- } else if (reason == MR_PEACE) {
+ break;
+
+ case MR_PEACE:
notify_player(unit_owner(punit), src_tile, E_BAD_COMMAND,
_("Cannot invade unless you break peace with "
"%s first."),
player_name(tile_owner(dest_tile)));
- }
+ break;
+ default:
+ /* FIXME: need more explanations someday! */
+ break;
+ };
+
return FALSE;
}
Index: server/unithand.h
===================================================================
--- server/unithand.h (revision 14245)
+++ server/unithand.h (working copy)
@@ -13,7 +13,6 @@
#ifndef FC__UNITHAND_H
#define FC__UNITHAND_H
-#include "packets.h"
#include "unit.h"
#include "hand_gen.h"
Index: common/unit.h
===================================================================
--- common/unit.h (revision 14245)
+++ common/unit.h (working copy)
@@ -68,14 +68,20 @@
};
enum unit_move_result {
- MR_OK, MR_BAD_TYPE_FOR_CITY_TAKE_OVER,
+ MR_OK,
+ MR_DEATH,
+ MR_PAUSE,
+ MR_BAD_TYPE_FOR_CITY_TAKE_OVER,
MR_NO_WAR, /* Can't move here without declaring war. */
MR_PEACE, /* Can't move here because of a peace treaty. */
MR_ZOC,
- MR_BAD_ACTIVITY, MR_BAD_DESTINATION, MR_BAD_MAP_POSITION,
+ MR_BAD_ACTIVITY,
+ MR_BAD_DESTINATION,
+ MR_BAD_MAP_POSITION,
+ MR_DESTINATION_OCCUPIED_BY_NON_ALLIED_CITY,
MR_DESTINATION_OCCUPIED_BY_NON_ALLIED_UNIT,
MR_NO_TRANSPORTER_CAPACITY,
- MR_DESTINATION_OCCUPIED_BY_NON_ALLIED_CITY, MR_TRIREME
+ MR_TRIREME,
};
enum add_build_city_result {
Index: ai/aiferry.c
===================================================================
--- ai/aiferry.c (revision 14245)
+++ ai/aiferry.c (working copy)
@@ -1094,11 +1094,19 @@
}
UNIT_LOG(LOGLEVEL_FERRY, punit, "Passing control of ferry to explorer code");
- if (!ai_manage_explorer(punit)) {
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ return;
+ case MR_OK:
+ /* FIXME: continue moving? */
+ break;
+ default:
punit->ai.done = TRUE;
- }
+ break;
+ };
- if (game_find_unit_by_number(sanity) && punit->moves_left > 0) {
+ if (punit->moves_left > 0) {
struct city *pcity = find_nearest_safe_city(punit);
if (pcity) {
punit->goto_tile = pcity->tile;
Index: ai/aiexplorer.c
===================================================================
--- ai/aiexplorer.c (revision 14245)
+++ ai/aiexplorer.c (working copy)
@@ -182,9 +182,12 @@
Handle eXplore mode of a unit (explorers are always in eXplore mode
for AI) - explores unknown territory, finds huts.
- Returns whether there is any more territory to be explored.
+ MR_OK: there is more territory to be explored.
+ MR_DEATH: unit died.
+ MR_PAUSE: unit cannot explore further now.
+ Other results: unit cannot explore further.
**************************************************************************/
-bool ai_manage_explorer(struct unit *punit)
+enum unit_move_result ai_manage_explorer(struct unit *punit)
{
struct player *pplayer = unit_owner(punit);
/* Loop prevention */
@@ -218,7 +221,7 @@
if (pplayer->ai.control && unit_has_type_flag(punit, F_GAMELOSS)) {
UNIT_LOG(LOG_DEBUG, punit, "exploration too dangerous!");
- return FALSE; /* too dangerous */
+ return MR_BAD_ACTIVITY; /* too dangerous */
}
TIMING_LOG(AIT_EXPLORER, TIMER_START);
@@ -297,7 +300,7 @@
* which goes beside the unknown, with a good EC callback... */
if (!ai_unit_goto(punit, best_tile)) {
/* Died? Strange... */
- return FALSE;
+ return MR_DEATH;
}
UNIT_LOG(LOG_DEBUG, punit, "exploration GOTO succeeded");
if (punit->moves_left > 0) {
@@ -311,15 +314,15 @@
return ai_manage_explorer(punit);
} else {
UNIT_LOG(LOG_DEBUG, punit, "done exploring (all finished)...");
- return FALSE;
+ return MR_PAUSE;
}
}
UNIT_LOG(LOG_DEBUG, punit, "done exploring (but more go go)...");
- return TRUE;
+ return MR_OK;
} else {
/* Didn't find anything. */
UNIT_LOG(LOG_DEBUG, punit, "failed to explore more");
- return FALSE;
+ return MR_BAD_MAP_POSITION;
}
#undef DIST_FACTOR
}
Index: ai/aiexplorer.h
===================================================================
--- ai/aiexplorer.h (revision 14245)
+++ ai/aiexplorer.h (working copy)
@@ -13,17 +13,10 @@
#ifndef FC__AIEXPLORER_H
#define FC__AIEXPLORER_H
-#include "shared.h" /* bool type */
+#include "unit.h"
-#include "fc_types.h"
-/*
- * Handle eXplore mode of a unit (explorers are always in eXplore mode
- * for AI) - explores unknown territory, finds huts.
- *
- * Returns whether there is any more territory to be explored.
- */
-bool ai_manage_explorer(struct unit *punit);
+enum unit_move_result ai_manage_explorer(struct unit *punit);
/*
* TODO: Enable AI build new explorers. Either (the old way) write a
Index: ai/aiunit.c
===================================================================
--- ai/aiunit.c (revision 14245)
+++ ai/aiunit.c (working copy)
@@ -192,24 +192,28 @@
0 if can't move
1 if zoc_ok
-1 if zoc could be ok?
+
+ see also unithand can_unit_move_to_tile_with_notify()
**************************************************************************/
int could_unit_move_to_tile(struct unit *punit, struct tile *dest_tile)
{
- enum unit_move_result result;
-
- result =
+ enum unit_move_result reason =
test_unit_move_to_tile(unit_type(punit), unit_owner(punit),
ACTIVITY_IDLE, punit->tile,
dest_tile, unit_has_type_flag(punit, F_IGZOC));
- if (result == MR_OK) {
+ switch (reason) {
+ case MR_OK:
return 1;
- }
- if (result == MR_ZOC) {
+ case MR_ZOC:
if (could_be_my_zoc(punit, punit->tile)) {
return -1;
}
- }
+ break;
+
+ default:
+ break;
+ };
return 0;
}
@@ -1648,6 +1652,7 @@
/*************************************************************************
Go berserk, assuming there are no targets nearby.
TODO: Is it not possible to remove this special casing for barbarians?
+ FIXME: enum unit_move_result
**************************************************************************/
static void ai_military_attack_barbarian(struct player *pplayer,
struct unit *punit)
@@ -1802,11 +1807,17 @@
(void) ai_unit_goto(punit, pcity->tile);
} else if (!is_barbarian(pplayer)) {
/* Nothing else to do, so try exploring. */
- if (ai_manage_explorer(punit)) {
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ return;
+ case MR_OK:
UNIT_LOG(LOG_DEBUG, punit, "nothing else to do, so exploring");
- } else if (game_find_unit_by_number(id)) {
+ break;
+ default:
UNIT_LOG(LOG_DEBUG, punit, "nothing to do - no more exploring either");
- }
+ break;
+ };
} else {
/* You can still have some moves left here, but barbarians should
not sit helplessly, but advance towards nearest known enemy city */
@@ -2073,7 +2084,18 @@
TIMING_LOG(AIT_BODYGUARD, TIMER_STOP);
break;
case AIUNIT_EXPLORE:
- punit->ai.done = !(ai_manage_explorer(punit) && punit->moves_left > 0);
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ return;
+ case MR_OK:
+ UNIT_LOG(LOG_DEBUG, punit, "more exploring");
+ break;
+ default:
+ UNIT_LOG(LOG_DEBUG, punit, "no more exploring either");
+ break;
+ };
+ punit->ai.done = (punit->moves_left <= 0);
break;
case AIUNIT_RECOVER:
TIMING_LOG(AIT_RECOVER, TIMER_START);
@@ -2239,15 +2261,20 @@
TIMING_LOG(AIT_MILITARY, TIMER_STOP);
return;
} else {
- int id = punit->id;
-
- UNIT_LOG(LOG_DEBUG, punit, "fell through all unit tasks, defending");
/* what else could this be? -- Syela */
- if (!ai_manage_explorer(punit)
- && game_find_unit_by_number(id)) {
+ switch (ai_manage_explorer(punit)) {
+ case MR_DEATH:
+ /* don't use punit! */
+ break;
+ case MR_OK:
+ UNIT_LOG(LOG_DEBUG, punit, "now exploring");
+ break;
+ default:
+ UNIT_LOG(LOG_DEBUG, punit, "fell through all unit tasks, defending");
ai_unit_new_role(punit, AIUNIT_DEFEND_HOME, NULL);
ai_military_defend(pplayer, punit);
- }
+ break;
+ };
return;
}
}
_______________________________________________
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
