correct bug#980355 [fog] crash (assert) with multiple containers

src/action/action_follow.cpp

@@ -136,7 +136,6 @@ void HandleActionFollow(Unit* unit)
 
 				// Teleport the unit
 				RemoveUnit(unit, NULL);
-				UnitCacheRemove(unit);
 				unit->X = goal->Goal->X;
 				unit->Y = goal->Goal->Y;
 				DropOutOnSide(unit, unit->Direction, 1, 1);

src/action/action_move.cpp

@@ -74,14 +74,12 @@
 */
 static int ActionMoveGeneric(Unit* unit, const Animation* anim)
 {
-	int xd;
+	int xd;     // X movement in tile.
-	int yd;
+	int yd;     // Y movement in tile.
 	int state;
 	int d;
-	int i;
+	int x;      // Unit->X
-	int x;
+	int y;      // Unit->Y
-	int y;
-	Unit* uninside;
 
 	// FIXME: state 0?, should be wrong, should be Reset.
 	// FIXME: Reset flag is cleared by HandleUnitAction.

src/action/action_resource.cpp

@@ -229,8 +229,6 @@ static int StartGathering(Unit* unit)
 		unit->Orders[0].Goal = NoUnitP;
 
 		RemoveUnit(unit, goal);
-		unit->X = goal->X;
-		unit->Y = goal->Y;
 	}
 
 	unit->Data.ResWorker.TimeToHarvest = resinfo->WaitAtResource /
@@ -598,8 +596,6 @@ static int MoveToDepot(Unit* unit)
 	// Place unit inside the depot
 	//
 	RemoveUnit(unit, goal);
-	unit->X = goal->X;
-	unit->Y = goal->Y;
 
 	//
 	// Update resource.

src/action/action_unload.cpp

@@ -153,8 +153,6 @@ int UnloadUnit(Unit* unit)
 	if (!FindUnloadPosition(unit->X, unit->Y, &x, &y, UnitMovementMask(unit))) {
 		return 0;
 	}
-	unit->X = x;
-	unit->Y = y;
 	unit->Wait = 1; // should be correct unit has still action
 	unit->Boarded = 0;
 	PlaceUnit(unit, x, y);

src/action/command.cpp

@@ -587,27 +587,6 @@ void CommandUnload(Unit* unit, int x, int y, Unit* what, int flush)
 	// Check if unit is still valid? (NETWORK!)
 	//
 	if (!unit->Removed && unit->Orders[0].Action != UnitActionDie) {
-		//
-		// For bunkers, don't go into an action. Just drop everything here and now.
-		//
-		if (unit->Type->Building) {
-			int i;
-			Unit* uins;
-
-			// Unload all units.
-			uins = unit->UnitInside;
-			for (i = unit->InsideCount; i; --i, uins = uins->NextContained) {
-				if (uins->Boarded) {
-					uins->X = unit->X;
-					uins->Y = unit->Y;
-					if (UnloadUnit(uins)) {
-						unit->BoardCount--;
-					}
-				}
-			}
-			return;
-		}
-
 		if (!(order = GetNextOrder(unit, flush))) {
 			return;
 		}

src/editor/editloop.cpp

@@ -1372,7 +1372,6 @@ static void EditorCallbackKeyDown(unsigned key, unsigned keychar)
 				Unit* unit;
 
 				RemoveUnit(unit = UnitUnderCursor, NULL);
-				UnitCacheRemove(unit);
 				UnitLost(unit);
 				UnitClearOrders(unit);
 				ReleaseUnit(unit);

src/include/map.h

@@ -345,19 +345,22 @@ extern int AnyMapAreaVisibleInViewport(const Viewport*, int , int , int , int);
 //
 // in map_fog.c
 //
+/// Function to (un)mark the vision table.
+typedef void MapMarkerFunc(const Player*, int x, int y);
+
 	/// Filter map flags through fog
 extern int MapFogFilterFlags(Player* player, int x, int y, int mask);
 	/// Mark a tile for normal sight
-extern void MapMarkTileSight(const Player* player, int x, int y);
+extern MapMarkerFunc MapMarkTileSight;
 	/// Unmark a tile for normal sight
-extern void MapUnmarkTileSight(const Player* player, int x, int y);
+extern MapMarkerFunc MapUnmarkTileSight;
 	/// Mark a tile for cloak detection
-extern void MapMarkTileDetectCloak(const Player* player,int x,int y);
+extern MapMarkerFunc MapMarkTileDetectCloak;
 	/// Unmark a tile for cloak detection
-extern void MapUnmarkTileDetectCloak(const Player* player,int x,int y);
+extern MapMarkerFunc MapUnmarkTileDetectCloak;
 
 	/// Mark sight changes
-extern void MapSight(const Player* player, int x, int y, int w, int h, int range, void (*marker)(const Player*, int, int));
+extern void MapSight(const Player* player, int x, int y, int w, int h, int range, MapMarkerFunc *marker);
 	/// Find if a tile is visible (With shared vision)
 extern unsigned char IsTileVisible(const Player* player, int x, int y);
 	/// Mark tiles with fog of war to be redrawn
@@ -483,6 +486,13 @@ extern void PreprocessMap(void);
 	/// Set wall on field
 extern void MapSetWall(unsigned x, unsigned y, int humanwall);
 
+// in unit.c
+
+/// Mark on vision table the Sight of the unit.
+void MapMarkUnitSight(Unit* unit);
+/// Unmark on vision table the Sight of the unit.
+void MapUnmarkUnitSight(Unit* unit);
+
 /*----------------------------------------------------------------------------
 --  Defines
 ----------------------------------------------------------------------------*/
@@ -494,30 +504,6 @@ extern void MapSetWall(unsigned x, unsigned y, int humanwall);
 #define MapMarkSight(player,x,y,w,h,range) MapSight((player),(x),(y),(w),(h),(range),MapMarkTileSight)
 #define MapUnmarkSight(player,x,y,w,h,range) MapSight((player),(x),(y),(w),(h),(range),MapUnmarkTileSight)
 
-#define MapMarkUnitSight(unit) \
-{ \
-	MapSight((unit)->Player, (unit)->X,(unit)->Y, (unit)->Type->TileWidth,\
-			(unit)->Type->TileHeight, (unit)->CurrentSightRange, MapMarkTileSight); \
-	if (unit->Type->DetectCloak) { \
-		MapSight((unit)->Player, (unit)->X,(unit)->Y, (unit)->Type->TileWidth,\
-				(unit)->Type->TileHeight, (unit)->CurrentSightRange, MapMarkTileDetectCloak); \
-	}\
-}
-
-#define MapUnmarkUnitSight(unit) \
-{ \
-	MapSight((unit)->Player,(unit)->X,(unit)->Y, (unit)->Type->TileWidth,\
-			(unit)->Type->TileHeight,(unit)->CurrentSightRange,MapUnmarkTileSight); \
-	if (unit->Type->DetectCloak) { \
-		MapSight((unit)->Player, (unit)->X,(unit)->Y, (unit)->Type->TileWidth,\
-				(unit)->Type->TileHeight, (unit)->CurrentSightRange, MapUnmarkTileDetectCloak); \
-	}\
-}
-
-#define MapMarkUnitOnBoardSight(unit,host) MapSight((unit)->Player,(host)->X,(host)->Y, \
-	(host)->Type->TileWidth,(host)->Type->TileHeight,(unit)->CurrentSightRange,MapMarkTileSight)
-#define MapUnmarkUnitOnBoardSight(unit,host) MapSight((unit)->Player,(host)->X,(host)->Y, \
-	(host)->Type->TileWidth,(host)->Type->TileHeight,(unit)->CurrentSightRange,MapUnmarkTileSight)
 	/// Check if a field for the user is explored
 #define IsMapFieldExplored(player,x,y) \
 	(IsTileVisible((player),(x),(y)))

src/include/unit.h

@@ -763,10 +763,10 @@ extern Unit* MakeUnit(UnitType* type,Player* player);
 extern void PlaceUnit(Unit* unit, int x, int y);
 	/// Create a new unit and place on map
 extern Unit* MakeUnitAndPlace(int x, int y, UnitType* type,Player* player);
+	/// Move unit to tile(x, y). (Do special stuff : vision, cachelist, pathfinding)
+extern void MoveUnitToXY(Unit* unit, int x, int y);
 	/// Add an unit inside a container. Only deal with list stuff.
 extern void AddUnitInContainer(Unit* unit, Unit* host);
-	/// Remove an unit from inside a container. Only deals with list stuff.
-extern void RemoveUnitFromContainer(Unit* unit);
 	/// Remove unit from map/groups/...
 extern void RemoveUnit(Unit* unit, Unit* host);
 	/// Handle the loose of an unit (food,...)

src/map/map_fog.cpp

@@ -212,6 +212,8 @@ void MapMarkTileSight(const Player* player, int x, int y)
 {
 	unsigned char v;
 
+	Assert(0 <= x && x < TheMap.Width);
+	Assert(0 <= y && y < TheMap.Height);
 	v = TheMap.Fields[x + y * TheMap.Width].Visible[player->Player];
 	switch (v) {
 		case 0:  // Unexplored
@@ -247,6 +249,8 @@ void MapUnmarkTileSight(const Player* player, int x, int y)
 {
 	unsigned char v;
 
+	Assert(0 <= x && x < TheMap.Width);
+	Assert(0 <= y && y < TheMap.Height);
 	v = TheMap.Fields[x + y * TheMap.Width].Visible[player->Player];
 	switch (v) {
 		case 255:

src/stratagus/spells.cpp

@@ -206,10 +206,7 @@ int CastSpawnPortal(Unit* caster, const SpellType* spell __attribute__((unused))
 	DebugPrint("Spawning a portal exit.\n");
 	portal = caster->Goal;
 	if (portal) {
-		// FIXME: if cop is already defined --> move it, but it doesn't work?
+		MoveUnitToXY(portal, x, y);
-		RemoveUnit(portal, NULL);
-		UnitCacheRemove(portal);
-		PlaceUnit(portal, x, y);
 	} else {
 		portal = MakeUnitAndPlace(x, y, ptype, &Players[PlayerNumNeutral]);
 	}
@@ -568,7 +565,6 @@ int CastPolymorph(Unit* caster, const SpellType* spell,
 
 	// as said somewhere else -- no corpses :)
 	RemoveUnit(target, NULL);
-	UnitCacheRemove(target);
 	for (i = 0; i < type->TileWidth; ++i) {
 		for (j = 0; j < type->TileHeight; ++j) {
 			if (!UnitTypeCanMoveTo(x + i, y + j, type)) {
@@ -643,27 +639,17 @@ int CastSummon(Unit* caster, const SpellType* spell,
 		// FIXME: do summoned units count on food?
 		//
 		target = MakeUnit(unittype, caster->Player);
-		target->X = x;
+		// This is a hack to walk around behaviour of DropOutOnSide
+		target->X = x + 1;
 		target->Y = y;
+		DropOutOnSide(target, LookingW, 0, 0); // FIXME : 0,0) : good parameter ?
 		//
 		//  set life span. ttl=0 results in a permanent unit.
 		//
 		if (ttl) {
 			target->TTL = GameCycle + ttl;
 		}
-		//
+
-		// Revealers are always removed, since they don't have graphics
-		//
-		if (target->Type->Revealer) {
-			DebugPrint("summoned unit is a revealer, removed.\n");
-			target->Removed = 1;
-			target->CurrentSightRange = target->Stats->SightRange;
-			MapMarkUnitSight(target);
-		} else {
-			// This is a hack to walk around behaviour of DropOutOnSide
-			target->X++;
-			DropOutOnSide(target, LookingW, 0, 0);
-		}
 		caster->Mana -= spell->ManaCost;
 		return 1;
 	}

src/unit/unit.cpp

@@ -431,6 +431,303 @@ Unit* MakeUnit(UnitType* type, Player* player)
 	return unit;
 }
 
+/**
+**  (Un)Mark on vision table the Sight of the unit
+**  (and units inside for transporter (recursively))
+**
+**  @param unit      Unit to (un)mark.
+**  @param x         X coord of first container of unit.
+**  @param y         Y coord of first container of unit.
+**  @param width     Width of the first container of unit.
+**  @param height    Height of the first container of unit.
+**  @param f         Function to (un)mark for normal vision.
+**  @param f2        Function to (un)mark for cloaking vision.
+*/
+static void MapMarkUnitSightRec(Unit* unit, int x, int y, int width, int height,
+	MapMarkerFunc* f, MapMarkerFunc* f2)
+{
+	Unit* unit_inside; // iterator on units inside unit. 
+	int i;             // number of units inside to process.
+
+	Assert(unit);
+	Assert(f);
+	MapSight(unit->Player, x, y, width, height, unit->CurrentSightRange, f);
+
+	if (unit->Type && unit->Type->DetectCloak && f2) {
+		MapSight(unit->Player, x, y, width, height, unit->CurrentSightRange, f2);
+	}
+
+	unit_inside = unit->UnitInside;
+	for (i = unit->InsideCount; i--; unit_inside = unit_inside->NextContained) {
+		MapMarkUnitSightRec(unit_inside, x, y, width, height, f, f2);
+	}
+}
+
+/**
+**  Return the unit not transported, by viewing the container recursively.
+**
+**  @param unit    unit from where look the first conatiner.
+**
+**  @return        Container of container of ... of unit. It is not null.
+*/
+static Unit* GetFirstContainer(const Unit* unit)
+{
+	Assert(unit);
+	while (unit->Container) {
+		unit = unit->Container;
+	}
+	return (Unit *) unit;
+}
+
+/**
+**  Mark on vision table the Sight of the unit
+**  (and units inside for transporter)
+**
+**  @param unit    unit to unmark its vision.
+**  @see MapUnmarkUnitSight.
+*/
+void MapMarkUnitSight(Unit* unit)
+{
+	Unit* container;  // First container of the unit.
+
+	Assert(unit);
+
+	container = GetFirstContainer(unit);
+	Assert(container->Type);
+	MapMarkUnitSightRec(unit,
+		container->X, container->Y, container->Type->TileWidth, container->Type->TileHeight,
+		MapMarkTileSight, MapMarkTileDetectCloak);
+}
+
+/**
+**  Unmark on vision table the Sight of the unit
+**  (and units inside for transporter)
+**
+**  @param unit    unit to unmark its vision.
+**  @see MapMarkUnitSight.
+*/
+void MapUnmarkUnitSight(Unit* unit)
+{
+	Unit* container;  // First container of the unit.
+
+	Assert(unit);
+	Assert(unit->Type);
+
+	container = GetFirstContainer(unit);
+	Assert(container->Type);
+	MapMarkUnitSightRec(unit,
+		container->X, container->Y, container->Type->TileWidth, container->Type->TileHeight,
+		MapUnmarkTileSight, MapUnmarkTileDetectCloak);
+}
+
+/**
+**  Update the Unit Current sight range to good value and transported units inside.
+**
+**  @param unit    unit to update SightRange
+**
+**  @internal before use it, MapUnmarkUnitSight(unit)
+**  and after MapMarkUnitSight(unit)
+**  are often necessary.
+**
+**  FIXME @todo manage differently unit inside with option.
+**  (no vision, min, host value, own value, bonus value, ...)
+*/
+static void UpdateUnitSightRange(Unit* unit)
+{
+	Unit* unit_inside; // iterator on units inside unit. 
+	int i;             // number of units inside to process.
+
+#if 0 // which is the better ? caller check ?
+	if (SaveGameLoading) {
+		return ;
+	}
+#else
+	Assert(!SaveGameLoading);
+#endif
+	// FIXME : these values must be configurable.
+	if (unit->Constructed) { // Units under construction have no sight range.
+		unit->CurrentSightRange = 0;
+	} else if (!unit->Container) { // proper value.
+		unit->CurrentSightRange = unit->Stats->SightRange;
+	} else { // value of it container.
+		unit->CurrentSightRange = unit->Container->CurrentSightRange;
+	}
+
+	unit_inside = unit->UnitInside;
+	for (i = unit->InsideCount; i--; unit_inside = unit_inside->NextContained) {
+		UpdateUnitSightRange(unit_inside);
+	}
+}
+
+/**
+**  Mark the field with the FieldFlags.
+**
+**  @param unit    unit to mark.
+*/
+static void MarkUnitFieldFlags(const Unit* unit)
+{
+	UnitType* type; // Type of the unit.
+	unsigned flags; //
+	int h;          // Tile height of the unit.
+	int w;          // Tile width of the unit.
+	int x;          // X tile of the unit.
+	int y;          // Y tile of the unit.
+
+	Assert(unit);
+	type = unit->Type;
+	x = unit->X;
+	y = unit->Y;
+	flags = type->FieldFlags;
+	for (h = type->TileHeight; h--;) {
+		for (w = type->TileWidth; w--;) {
+			TheMap.Fields[x + w + (y + h) * TheMap.Width].Flags |= flags;
+		}
+	}
+#ifdef MAP_REGIONS
+	// Update map splitting.
+	if (type->Building && (flags &
+		 (MapFieldLandUnit | MapFieldSeaUnit | MapFieldBuilding |
+		  MapFieldUnpassable | MapFieldWall | MapFieldRocks | MapFieldForest))) {
+		MapSplitterTilesOccuped(x, y, x + type->TileWidth - 1, y + type->TileHeight - 1);
+	}
+#endif
+
+}
+
+/**
+**  Mark the field with the FieldFlags.
+**
+**  @param unit    unit to mark.
+*/
+static void UnmarkUnitFieldFlags(const Unit* unit)
+{
+	UnitType* type; // Type of the unit.
+	unsigned flags; //
+	int h;          // Tile height of the unit.
+	int w;          // Tile width of the unit.
+	int x;          // X tile of the unit.
+	int y;          // Y tile of the unit.
+
+	Assert(unit);
+	type = unit->Type;
+	x = unit->X;
+	y = unit->Y;
+	flags = type->FieldFlags;
+	for (h = type->TileHeight; h--;) {
+		for (w = type->TileWidth; w--;) {
+			TheMap.Fields[x + w + (y + h) * TheMap.Width].Flags &= ~flags;
+		}
+	}
+#ifdef MAP_REGIONS
+	// Update map splitting.
+	if (type->Building && (flags &
+		 (MapFieldLandUnit | MapFieldSeaUnit | MapFieldBuilding |
+		  MapFieldUnpassable | MapFieldWall | MapFieldRocks | MapFieldForest))){
+		MapSplitterTilesCleared(x, y, x + type->TileWidth - 1, y + type->TileHeight - 1);
+	}
+#endif
+}
+
+/**
+** Add unit to a container. It only updates linked list stuff
+**
+** @param unit    Pointer to unit.
+** @param host    Pointer to container.
+*/
+void AddUnitInContainer(Unit* unit, Unit* host)
+{
+	Assert(host && unit->Container == 0);
+	unit->Container = host;
+	if (host->InsideCount == 0) {
+		unit->NextContained = unit->PrevContained = unit;
+	} else {
+		unit->NextContained = host->UnitInside;
+		unit->PrevContained = host->UnitInside->PrevContained;
+		host->UnitInside->PrevContained->NextContained = unit;
+		host->UnitInside->PrevContained = unit;
+	}
+	host->UnitInside = unit;
+	host->InsideCount++;
+}
+
+/**
+** Remove unit from a container. It only updates linked list stuff
+**
+** @param unit    Pointer to unit.
+*/
+static void RemoveUnitFromContainer(Unit* unit)
+{
+	Unit* host;  // transporter which contain unit.
+
+	host = unit->Container;
+	Assert(unit->Container);
+	Assert(unit->Container->InsideCount > 0);
+	host->InsideCount--;
+	unit->NextContained->PrevContained = unit->PrevContained;
+	unit->PrevContained->NextContained = unit->NextContained;
+	if (host->InsideCount == 0) {
+		host->UnitInside = NoUnitP;
+	} else {
+		if (host->UnitInside == unit) {
+			host->UnitInside = unit->NextContained;
+		}
+	}
+	unit->Container = NoUnitP;
+}
+
+
+/**
+**  Affect Tile coord of an unit (with units inside) to tile (x, y).
+**
+**  @param unit    unit to move.
+**  @param x       X map tile position.
+**  @param y       Y map tile position.
+**
+**  @internal before use it, UnitCacheRemove(unit), MapUnmarkUnitSight(unit)
+**  and after UnitCacheInsert(unit), MapMarkUnitSight(unit)
+**  are often necessary. Check Flag also for Pathfinder.
+*/
+static void UnitInXY(Unit* unit, int x, int y)
+{
+	Unit* unit_inside;      // iterator on units inside unit. 
+	int i;                  // number of units inside to process.
+
+	Assert(unit);
+	unit->X = x;
+	unit->Y = y;
+
+	unit_inside = unit->UnitInside;
+	for (i = unit->InsideCount; i--; unit_inside = unit_inside->NextContained) {
+		UnitInXY(unit_inside, x, y);
+	}
+}
+
+/**
+**  Move an unit (with units inside) to tile (x, y).
+**  (Do stuff with vision, cachelist and pathfinding).
+**
+**  @param unit    unit to move.
+**  @param x       X map tile position.
+**  @param y       Y map tile position.
+**
+*/
+void MoveUnitToXY(Unit* unit, int x, int y)
+{
+	MapUnmarkUnitSight(unit);
+	UnitCacheRemove(unit);
+	UnmarkUnitFieldFlags(unit);
+
+	// Move the unit.
+	UnitInXY(unit, x, y);
+
+	UnitCacheInsert(unit);
+	MarkUnitFieldFlags(unit);
+	MapMarkUnitSight(unit);
+}
+
+
+
+
 /**
 ** Place unit on map.
 **
@@ -440,63 +737,23 @@ Unit* MakeUnit(UnitType* type, Player* player)
 */
 void PlaceUnit(Unit* unit, int x, int y)
 {
-	const UnitType* type;
-	int h;
-	int w;
-	unsigned flags;
-
 	Assert(unit->Removed);
 
-	type = unit->Type;
+	if (unit->Container) {
-
+		RemoveUnitFromContainer(unit);
-
-	unit->X = x;
-	unit->Y = y;
-
-	//
-	// Place unit on the map, mark the field with the FieldFlags.
-	//
-	flags = type->FieldFlags;
-	for (h = type->TileHeight; h--;) {
-		for (w = type->TileWidth; w--;) {
-			TheMap.Fields[x + w + (y + h) * TheMap.Width].Flags |= flags;
-		}
 	}
-
-#ifdef MAP_REGIONS
-	if (type->Building &&
-			(type->FieldFlags &
-		 (MapFieldLandUnit | MapFieldSeaUnit | MapFieldBuilding |
-		  MapFieldUnpassable | MapFieldWall | MapFieldRocks | MapFieldForest))){
-		MapSplitterTilesOccuped(x, y, x + type->TileWidth - 1, y + type->TileHeight - 1);
-	}
-#endif
-
-	x += unit->Type->TileWidth / 2;
-	y += unit->Type->TileHeight / 2;
-
-	//
-	// Units under construction have no sight range.
-	//
-	if (!unit->Constructed) {
-		//
-		// Update fog of war, if unit belongs to player on this computer
-		//
-		if (unit->Container && unit->Removed) {
-			MapUnmarkUnitOnBoardSight(unit, unit->Container);
-		}
-		if (unit->Container) {
-			RemoveUnitFromContainer(unit);
-		}
-		if (!SaveGameLoading) {
-			unit->CurrentSightRange = unit->Stats->SightRange;
-		}
-		MapMarkUnitSight(unit);
-	}
-
-	unit->Removed = 0;
 	unit->Next = 0;
+	if (!SaveGameLoading) {
+		UpdateUnitSightRange(unit);
+	}
+	unit->Removed = 0;
+	UnitInXY(unit, x, y);
+	// Pathfinding info.
+	MarkUnitFieldFlags(unit);
+	// Tha cache list.
 	UnitCacheInsert(unit);
+	// Vision
+	MapMarkUnitSight(unit);
 
 	MustRedraw |= RedrawMinimap;
 	UnitCountSeen(unit);
@@ -522,61 +779,6 @@ Unit* MakeUnitAndPlace(int x, int y, UnitType* type, Player* player)
 	return unit;
 }
 
-/**
-** Add unit to a container. It only updates linked list stuff
-**
-** @param unit    Pointer to unit.
-** @param host    Pointer to container.
-*/
-void AddUnitInContainer(Unit* unit, Unit* host)
-{
-	if (unit->Container) {
-		DebugPrint("Unit is already contained.\n");
-		exit(0);
-	}
-	unit->Container = host;
-	if (host->InsideCount == 0) {
-		unit->NextContained = unit->PrevContained = unit;
-	} else {
-		unit->NextContained = host->UnitInside;
-		unit->PrevContained = host->UnitInside->PrevContained;
-		host->UnitInside->PrevContained->NextContained = unit;
-		host->UnitInside->PrevContained = unit;
-	}
-	host->UnitInside = unit;
-	host->InsideCount++;
-}
-
-/**
-** Remove unit from a container. It only updates linked list stuff
-**
-** @param unit    Pointer to unit.
-*/
-void RemoveUnitFromContainer(Unit* unit)
-{
-	Unit* host;
-	host = unit->Container;
-	if (!unit->Container) {
-		DebugPrint("Unit not contained.\n");
-		exit(0);
-	}
-	if (host->InsideCount == 0) {
-		DebugPrint("host's inside count reached -1.");
-		exit(0);
-	}
-	host->InsideCount--;
-	unit->NextContained->PrevContained = unit->PrevContained;
-	unit->PrevContained->NextContained = unit->NextContained;
-	if (host->InsideCount == 0) {
-		host->UnitInside = NoUnitP;
-	} else {
-		if (host->UnitInside == unit) {
-			host->UnitInside = unit->NextContained;
-		}
-	}
-	unit->Container = NoUnitP;
-}
-
 /**
 ** Remove unit from map.
 **
@@ -589,27 +791,24 @@ void RemoveUnitFromContainer(Unit* unit)
 */
 void RemoveUnit(Unit* unit, Unit* host)
 {
-	int h;
-	int w;
-	const UnitType* type;
-	unsigned flags;
-
-	if (unit->Removed && unit->Container) {
-		MapUnmarkUnitOnBoardSight(unit, unit->Container);
-	} else {
-		MapUnmarkUnitSight(unit);
-	}
-	if (host) {
-		unit->CurrentSightRange = host->CurrentSightRange;
-		MapMarkUnitOnBoardSight(unit, host);
-		AddUnitInContainer(unit, host);
-	}
-
 	if (unit->Removed) { // could happen!
 		// If unit is removed (inside) and building is destroyed.
+		DebugPrint("unit '%s' already remove" _C_ unit->Type->Ident);
 		return;
 	}
+	UnitCacheRemove(unit);
+	MapUnmarkUnitSight(unit);
+	UnmarkUnitFieldFlags(unit);
+	if (host) {
+		AddUnitInContainer(unit, host);
+		UpdateUnitSightRange(unit);
+		UnitInXY(unit, host->X, host->Y);
+		MapMarkUnitSight(unit);
+		unit->Next = host; // What is it role ?
+	}
+
 	unit->Removed = 1;
+
 	//  Remove unit from the current selection
 	if (unit->Selected) {
 		if (NumSelected == 1) { //  Remove building cursor
@@ -627,37 +826,6 @@ void RemoveUnit(Unit* unit, Unit* host)
 	if (unit == UnitUnderCursor) {
 		UnitUnderCursor = NULL;
 	}
-
-	type = unit->Type;
-
-	//
-	// Update map
-	//
-	flags = ~type->FieldFlags;
-	for (h = type->TileHeight; h--;) {
-		for (w = type->TileWidth; w--;) {
-			TheMap.Fields[unit->X + w + (unit->Y + h) * TheMap.Width].Flags &= flags;
-		}
-	}
-
-#ifdef MAP_REGIONS
-	//
-	// Update map splitting.
-	//
-	if (type->Building &&
-			(type->FieldFlags &
-		 (MapFieldLandUnit | MapFieldSeaUnit | MapFieldBuilding |
-		  MapFieldUnpassable | MapFieldWall | MapFieldRocks | MapFieldForest))){
-		MapSplitterTilesCleared(unit->X, unit->Y,
-			unit->X + type->TileWidth - 1, unit->Y + type->TileHeight - 1);
-	}
-#endif
-
-	if (host) {
-		UnitCacheRemove(unit);
-		unit->Next = host;
-	}
-
 	MustRedraw |= RedrawMinimap;
 }
 
@@ -1357,15 +1525,9 @@ void ChangeUnitOwner(Unit* unit, Player* newplayer)
 	}
 	*unit->PlayerSlot = unit;
 
-	if (unit->Removed && unit->Container) {
+	MapUnmarkUnitSight(unit);
-		MapUnmarkUnitOnBoardSight(unit, unit->Next);
+	unit->Player = newplayer;
-		unit->Player = newplayer;
+	MapMarkUnitSight(unit);
-		MapMarkUnitOnBoardSight(unit, unit->Next);
-	} else {
-		MapUnmarkUnitSight(unit);
-		unit->Player = newplayer;
-		MapMarkUnitSight(unit);
-	}
 
 	unit->Stats = &unit->Type->Stats[newplayer->Player];
 	//
@@ -2534,7 +2696,6 @@ void LetUnitDie(Unit* unit)
 	// removed units,  just remove.
 	if (unit->Removed) {
 		DebugPrint("Killing a removed unit?\n");
-		RemoveUnit(unit, NULL);
 		UnitLost(unit);
 		UnitClearOrders(unit);
 		ReleaseUnit(unit);
@@ -2580,8 +2741,8 @@ void LetUnitDie(Unit* unit)
 			// Restore value for oil-patch
 			unit->Value = unit->Data.Builded.Worker->Value;
 		}
-		DestroyAllInside(unit);
 
+		DestroyAllInside(unit);
 		RemoveUnit(unit, NULL);
 		UnitLost(unit);
 		UnitClearOrders(unit);
@@ -2592,7 +2753,6 @@ void LetUnitDie(Unit* unit)
 			unit->State = unit->Type->CorpseScript;
 			Assert(type->TileWidth == type->CorpseType->TileWidth &&
 					type->TileHeight == type->CorpseType->TileHeight);
-			MapMarkUnitSight(unit);
 			type = unit->Type = type->CorpseType;
 
 #ifdef DYNAMIC_LOAD
@@ -2604,7 +2764,7 @@ void LetUnitDie(Unit* unit)
 			unit->IY = (type->Height - VideoGraphicHeight(type->Sprite)) / 2;
 
 			unit->SubAction = 0;
-			//unit->Removed = 0;
+			unit->Removed = 0;
 			unit->Frame = 0;
 			unit->Orders[0].Action = UnitActionDie;
 
@@ -2612,13 +2772,11 @@ void LetUnitDie(Unit* unit)
 				unit->Type->Animations->Die);
 			UnitShowAnimation(unit, unit->Type->Animations->Die);
 			DebugPrint("Frame %d\n" _C_ unit->Frame);
-			MapUnmarkUnitSight(unit);
 			unit->CurrentSightRange = type->Stats[unit->Player->Player].SightRange;
 			MapMarkUnitSight(unit);
+			UnitCacheInsert(unit);
 		} else {
 			// no corpse available
-			MapMarkUnitSight(unit);
-			MapUnmarkUnitSight(unit);
 			unit->CurrentSightRange = 0;
 		}
 		return;
@@ -2629,7 +2787,6 @@ void LetUnitDie(Unit* unit)
 		// FIXME: destroy or unload : do a flag.
 		DestroyAllInside(unit);
 	}
-
 	RemoveUnit(unit, NULL);
 	UnitLost(unit);
 	UnitClearOrders(unit);
@@ -2640,18 +2797,18 @@ void LetUnitDie(Unit* unit)
 
 	// Not good: UnitUpdateHeading(unit);
 	unit->SubAction = 0;
-	//unit->Removed = 0;
 	unit->State = 0;
 	unit->Reset = 0;
 	unit->Wait = 1;
 	unit->Orders[0].Action = UnitActionDie;
-
 	if (unit->Type->CorpseType) {
 		unit->CurrentSightRange = unit->Type->CorpseType->Stats[unit->Player->Player].SightRange;
+		MapMarkUnitSight(unit);
 	} else {
 		unit->CurrentSightRange = 0;
 	}
-	MapMarkUnitSight(unit);;
+	unit->Removed = 0;
+	UnitCacheInsert(unit);
 }
 
 /**
@@ -2669,7 +2826,6 @@ void DestroyAllInside(Unit* source)
 		if (unit->UnitInside) {
 			DestroyAllInside(unit);
 		}
-		RemoveUnit(unit, NULL);
 		UnitLost(unit);
 		UnitClearOrders(unit);
 		ReleaseUnit(unit);

src/unit/unit_cache.cpp

@@ -58,6 +58,8 @@ void UnitCacheInsert(Unit* unit)
 	MapField* mf;
 	UnitListItem* listitem;
 
+	Assert(!unit->Removed);
+
 	for (i = 0; i < unit->Type->TileHeight; ++i) {
 		for (j = 0; j < unit->Type->TileWidth; ++j) {
 			mf = TheMap.Fields + (i + unit->Y) * TheMap.Width + j + unit->X;
@@ -174,8 +176,9 @@ int UnitCacheSelect(int x1, int y1, int x2, int y2, Unit** table)
 				// It should only be used in here, unless you somehow want the unit
 				// to be out of cache.
 				//
-				if (!listitem->Unit->CacheLock) {
+				if (!listitem->Unit->CacheLock && !listitem->Unit->Type->Revealer) {
 					listitem->Unit->CacheLock = 1;
+					Assert(!listitem->Unit->Removed);
 					table[n++] = listitem->Unit;
 				}
 			}
@@ -214,6 +217,7 @@ int UnitCacheOnTile(int x, int y, Unit** table)
 	listitem = TheMap.Fields[y * TheMap.Width + x].UnitCache;
 	for (; listitem; listitem = listitem->Next) {
 		if (!listitem->Unit->CacheLock) {
+			Assert(!listitem->Unit->Removed);
 			table[n++] = listitem->Unit;
 		}
 	}