bugfix(radar): Add hero radar objects into its own list to get rid of hero cache updates and its related issues (#1893)

Author: xezon

Core/GameEngine/Include/Common/Radar.h

@@ -77,13 +77,6 @@ enum RadarEventType CPP_11(: Int)
 
 };
 
-enum RadarObjectType CPP_11(: Int)
-{
-	RadarObjectType_None = 0,
-	RadarObjectType_Regular,
-	RadarObjectType_Local,
-};
-
 // PROTOTYPES /////////////////////////////////////////////////////////////////////////////////////
 
 //-------------------------------------------------------------------------------------------------
@@ -196,8 +189,8 @@ class Radar : public Snapshot,
  	Bool tryEvent( RadarEventType event, const Coord3D *pos );	///< try to make a "stealth" event
 
 	// adding and removing objects from the radar
-	virtual RadarObjectType addObject( Object *obj ); ///< add object to radar
-	virtual RadarObjectType removeObject( Object *obj ); ///< remove object from radar
+	virtual Bool addObject( Object *obj ); ///< add object to radar
+	virtual Bool removeObject( Object *obj ); ///< remove object from radar
 
 	// radar options
 	void hide( Int playerIndex, Bool hide ) { m_radarHidden[playerIndex] = hide; } ///< hide/show the radar
@@ -245,8 +238,6 @@ class Radar : public Snapshot,
 
 	inline Real getTerrainAverageZ() const { return m_terrainAverageZ; }
 	inline Real getWaterAverageZ() const { return m_waterAverageZ; }
-	inline const RadarObject* getObjectList() const { return m_objectList; }
-	inline const RadarObject* getLocalObjectList() const { return m_localObjectList; }
 
 	void clearAllEvents( void );					///< remove all radar events in progress
 
@@ -258,11 +249,18 @@ class Radar : public Snapshot,
 
 	Bool m_radarHidden[MAX_PLAYER_COUNT]; ///< true when radar is not visible
 	Bool m_radarForceOn[MAX_PLAYER_COUNT]; ///< true when radar is forced to be on
+
 	RadarObject *m_objectList;						///< list of objects in the radar
 	RadarObject *m_localObjectList;				/** list of objects for the local player, sorted
 																					* in exactly the same priority as the regular
 																					* object list for all other objects */
 
+	// TheSuperHackers @bugfix xezon 22/11/2025 Now stores local heroes in a separate list,
+	// because they are treated with special icons but should otherwise work like all other
+	// radar objects. In retail version, the cached hero object data was able to dangle
+	// for a few frames and cause undefined behavior.
+	RadarObject *m_localHeroObjectList; ///< list of hero objects for the local player
+
 	Real m_terrainAverageZ;								///< average Z for terrain samples
 	Real m_waterAverageZ;									///< average Z for water samples
 

Core/GameEngine/Source/Common/System/Radar.cpp

@@ -81,6 +81,7 @@ void Radar::deleteListResources( void )
 {
 	deleteList(&m_objectList);
 	deleteList(&m_localObjectList);
+	deleteList(&m_localHeroObjectList);
 
 #ifdef DEBUG_CRASHING
 	for( Object *obj = TheGameLogic->getFirstObject(); obj; obj = obj->getNextObject() )
@@ -191,6 +192,7 @@ Radar::Radar( void )
 	m_radarWindow = NULL;
 	m_objectList = NULL;
 	m_localObjectList = NULL;
+	m_localHeroObjectList = NULL;
 	std::fill(m_radarHidden, m_radarHidden + ARRAY_SIZE(m_radarHidden), false);
 	std::fill(m_radarForceOn, m_radarForceOn + ARRAY_SIZE(m_radarForceOn), false);
 	m_terrainAverageZ = 0.0f;
@@ -379,13 +381,13 @@ void Radar::newMap( TerrainLogic *terrain )
 /** Add an object to the radar list.  The object will be sorted in the list to be grouped
 	* using it's radar priority */
 //-------------------------------------------------------------------------------------------------
-RadarObjectType Radar::addObject( Object *obj )
+Bool Radar::addObject( Object *obj )
 {
 
 	// get the radar priority for this object
 	RadarPriorityType newPriority = obj->getRadarPriority();
 	if( isPriorityVisible( newPriority ) == FALSE )
-		return RadarObjectType_None;
+		return FALSE;
 
 	// if this object is on the radar, remove it in favor of the new add
 	RadarObject **list;
@@ -408,26 +410,26 @@ RadarObjectType Radar::addObject( Object *obj )
 	// set a chunk of radar data in the object
 	obj->friend_setRadarData( newObj );
 
-	RadarObjectType objectType;
 	//
 	// we will put this on either the local object list for objects that belong to the
 	// local player, or on the regular object list for all other objects
 	//
 	if( obj->isLocallyControlled() )
 	{
-		list = &m_localObjectList;
-		objectType = RadarObjectType_Local;
+		if ( obj->isHero() )
+			list = &m_localHeroObjectList;
+		else
+			list = &m_localObjectList;
 	}
 	else
 	{
 		list = &m_objectList;
-		objectType = RadarObjectType_Regular;
 	}
 
 	// link object to master list at the head of it's priority section
 	linkRadarObject(newObj, list);
 
-	return objectType;
+	return TRUE;
 }
 
 //-------------------------------------------------------------------------------------------------
@@ -474,24 +476,24 @@ Bool Radar::deleteFromList( Object *obj, RadarObject **list )
 //-------------------------------------------------------------------------------------------------
 /** Remove an object from the radar, the object may reside in any list */
 //-------------------------------------------------------------------------------------------------
-RadarObjectType Radar::removeObject( Object *obj )
+Bool Radar::removeObject( Object *obj )
 {
 
 	// sanity
 	if( obj->friend_getRadarData() == NULL )
-		return RadarObjectType_None;
+		return FALSE;
 
+	if( deleteFromList( obj, &m_localHeroObjectList ) == TRUE )
+		return TRUE;
 	if( deleteFromList( obj, &m_localObjectList ) == TRUE )
-		return RadarObjectType_Local;
+		return TRUE;
 	else if( deleteFromList( obj, &m_objectList ) == TRUE )
-		return RadarObjectType_Regular;
+		return TRUE;
 	else
 	{
-
-		// sanity
 		DEBUG_ASSERTCRASH( 0, ("Radar: Tried to remove object '%s' which was not found",
 											 obj->getTemplate()->getName().str()) );
-		return RadarObjectType_None;
+		return FALSE;
 	}
 
 }
@@ -716,10 +718,14 @@ Object *Radar::objectUnderRadarPixel( const ICoord2D *pixel )
 	// to the radar location
 	//
 
-	// search the local object list
-	obj = searchListForRadarLocationMatch( m_localObjectList, &radar );
+	// search the local hero object list
+	obj = searchListForRadarLocationMatch( m_localHeroObjectList, &radar );
+
+	// search the local object list if not found
+	if( obj == NULL )
+		obj = searchListForRadarLocationMatch( m_localObjectList, &radar );
 
-	// search all other objects if not found
+	// search all other objects if still not found
 	if( obj == NULL )
 		obj = searchListForRadarLocationMatch( m_objectList, &radar );
 
@@ -1359,6 +1365,7 @@ static void xferRadarObjectList( Xfer *xfer, RadarObject **head )
 	* Version Info:
 	* 1: Initial version
 	* 2: TheSuperHackers @tweak Serialize m_radarHidden, m_radarForceOn for each player
+	* 3: TheSuperHackers @tweak Serialize m_localHeroObjectList
 	*/
 // ------------------------------------------------------------------------------------------------
 void Radar::xfer( Xfer *xfer )
@@ -1368,13 +1375,13 @@ void Radar::xfer( Xfer *xfer )
 #if RETAIL_COMPATIBLE_XFER_SAVE
 	XferVersion currentVersion = 1;
 #else
-	XferVersion currentVersion = 2;
+	XferVersion currentVersion = 3;
 #endif
 	XferVersion version = currentVersion;
 	xfer->xferVersion( &version, currentVersion );
 
 
-	if (version <= 1)
+	if (version < 2)
 	{
 		const Int localPlayerIndex = ThePlayerList->getLocalPlayer()->getPlayerIndex();
 		Bool value;
@@ -1398,12 +1405,66 @@ void Radar::xfer( Xfer *xfer )
 		xfer->xferUser(&m_radarForceOn, sizeof(m_radarForceOn));
 	}
 
+	if (version < 3)
+	{
+		if (xfer->getXferMode() == XFER_SAVE)
+		{
+			// TheSuperHackers @info For legacy xfer compatibility.
+			// Transfer all local hero objects to local object list.
+			RadarObject **fromList = &m_localHeroObjectList;
+			RadarObject **toList = &m_localObjectList;
+			while (*fromList != NULL)
+			{
+				RadarObject* nextObject = (*fromList)->friend_getNext();
+				(*fromList)->friend_setNext(NULL);
+				linkRadarObject(*fromList, toList);
+				*fromList = nextObject;
+			}
+		}
+	}
+	else
+	{
+		xferRadarObjectList( xfer, &m_localHeroObjectList );
+	}
+
 	// save our local object list
 	xferRadarObjectList( xfer, &m_localObjectList );
 
 	// save the regular object list
 	xferRadarObjectList( xfer, &m_objectList );
 
+	if (version < 3)
+	{
+		// TheSuperHackers @info For legacy xfer compatibility.
+		// Transfer hero local object(s) back to local hero object list.
+		// This needs to be done on both load and save.
+		RadarObject **fromList = &m_localObjectList;
+		RadarObject **toList = &m_localHeroObjectList;
+		RadarObject *currObject;
+		RadarObject *prevObject;
+		RadarObject *nextObject;
+		prevObject = NULL;
+		for (currObject = *fromList; currObject != NULL; currObject = nextObject)
+		{
+			nextObject = currObject->friend_getNext();
+			if (currObject->friend_getObject()->isHero())
+			{
+				if (prevObject != NULL)
+				{
+					prevObject->friend_setNext(nextObject);
+				}
+				else
+				{
+					*fromList = nextObject;
+				}
+				currObject->friend_setNext(NULL);
+				linkRadarObject(currObject, toList);
+				continue;
+			}
+			prevObject = currObject;
+		}
+	}
+
 	// save the radar event count and data
 	UnsignedShort eventCountVerify = MAX_RADAR_EVENTS;
 	UnsignedShort eventCount = eventCountVerify;

Core/GameEngineDevice/Include/W3DDevice/Common/W3DRadar.h

@@ -58,9 +58,6 @@ class W3DRadar : public Radar
 	virtual void update( void );																	///< subsystem update
 	virtual void reset( void );																		///< subsystem reset
 
-	virtual RadarObjectType addObject( Object *obj ); ///< add object to radar
-	virtual RadarObjectType removeObject( Object *obj ); ///< remove object from radar
-
 	virtual void newMap( TerrainLogic *terrain );				///< reset radar for new map
 
 	virtual void draw( Int pixelX, Int pixelY, Int width, Int height );		///< draw the radar
@@ -85,7 +82,7 @@ class W3DRadar : public Radar
 	void drawIcons( Int pixelX, Int pixelY, Int width, Int height );	///< draw all of the radar icons
 	void updateObjectTexture(TextureClass *texture);
 	static Bool canRenderObject( const RadarObject *rObj, const Player *localPlayer );
-	void renderObjectList( const RadarObject *listHead, TextureClass *texture, Bool calcHero = FALSE );			 ///< render an object list to the texture
+	void renderObjectList( const RadarObject *listHead, TextureClass *texture );
 	void interpolateColorForHeight( RGBColor *color,
 																	Real height,
 																	Real hiZ,
@@ -122,6 +119,4 @@ class W3DRadar : public Radar
 	Real m_viewAngle;															///< camera angle used for the view box we have
 	Real m_viewZoom;															///< camera zoom used for the view box we have
 	ICoord2D m_viewBox[ 4 ];											///< radar cell points for the 4 corners of view box
-
-	std::vector<const Object *> m_cachedHeroObjectList; //< cache of hero objects for drawing icons in radar overlay
 };

Core/GameEngineDevice/Source/W3DDevice/Common/System/W3DRadar.cpp

@@ -602,12 +602,13 @@ void W3DRadar::drawEvents( Int pixelX, Int pixelY, Int width, Int height )
 //-------------------------------------------------------------------------------------------------
 void W3DRadar::drawIcons( Int pixelX, Int pixelY, Int width, Int height )
 {
-	// draw the hero icons
-	std::vector<const Object *>::const_iterator iter = m_cachedHeroObjectList.begin();
-	while (iter != m_cachedHeroObjectList.end())
+	Player *player = rts::getObservedOrLocalPlayer();
+	for (RadarObject *heroObj = m_localHeroObjectList; heroObj; heroObj = heroObj->friend_getNext())
 	{
-		drawHeroIcon( pixelX, pixelY, width, height, (*iter)->getPosition() );
-		++iter;
+		if (canRenderObject(heroObj, player))
+		{
+			drawHeroIcon(pixelX, pixelY, width, height, heroObj->friend_getObject()->getPosition());
+		}
 	}
 }
 
@@ -621,8 +622,9 @@ void W3DRadar::updateObjectTexture(TextureClass *texture)
 	REF_PTR_RELEASE(surface);
 
 	// rebuild the object overlay
-	renderObjectList( getObjectList(), texture );
-	renderObjectList( getLocalObjectList(), texture, TRUE );
+	renderObjectList( m_objectList, texture );
+	renderObjectList( m_localObjectList, texture );
+	renderObjectList( m_localHeroObjectList, texture );
 }
 
 //-------------------------------------------------------------------------------------------------
@@ -676,7 +678,7 @@ Bool W3DRadar::canRenderObject( const RadarObject *rObj, const Player *localPlay
 //-------------------------------------------------------------------------------------------------
 /** Render an object list into the texture passed in */
 //-------------------------------------------------------------------------------------------------
-void W3DRadar::renderObjectList( const RadarObject *listHead, TextureClass *texture, Bool calcHero )
+void W3DRadar::renderObjectList( const RadarObject *listHead, TextureClass *texture )
 {
 
 	// sanity
@@ -691,12 +693,6 @@ void W3DRadar::renderObjectList( const RadarObject *listHead, TextureClass *text
 
 	Player *player = rts::getObservedOrLocalPlayer();
 
-	if( calcHero )
-	{
-		// clear all entries from the cached hero object list
-		m_cachedHeroObjectList.clear();
-	}
-
 	for( const RadarObject *rObj = listHead; rObj; rObj = rObj->friend_getNext() )
 	{
 		if (!canRenderObject(rObj, player))
@@ -734,12 +730,6 @@ void W3DRadar::renderObjectList( const RadarObject *listHead, TextureClass *text
 
 		}
 
-		// cache hero objects for drawing in icon layer
-		if( calcHero && obj->isHero() )
-		{
-			m_cachedHeroObjectList.push_back(obj);
-		}
-
 		// draw the blip, but make sure the points are legal
 		if( legalRadarPoint( radarPoint.x, radarPoint.y ) )
 			surface->DrawPixel( radarPoint.x, radarPoint.y, c );
@@ -988,8 +978,6 @@ void W3DRadar::reset( void )
 	// extending functionality, call base class
 	Radar::reset();
 
-	m_cachedHeroObjectList.clear();
-
 	// clear our texture data, but do not delete the resources
 	SurfaceClass *surface;
 
@@ -1024,39 +1012,6 @@ void W3DRadar::update( void )
 
 }
 
-//-------------------------------------------------------------------------------------------------
-//-------------------------------------------------------------------------------------------------
-RadarObjectType W3DRadar::addObject( Object* obj )
-{
-	RadarObjectType addedType = Radar::addObject(obj);
-
-	if (addedType == RadarObjectType_Local)
-	{
-		if (obj->isHero() && !RadarObject::isTemporarilyHidden(obj))
-		{
-			m_cachedHeroObjectList.push_back(obj);
-		}
-	}
-
-	return addedType;
-}
-
-//-------------------------------------------------------------------------------------------------
-// TheSuperHackers @bugfix xezon 05/07/2025 Now removes the cached hero immediately because
-// otherwise the object pointer could be dangling and used for a bit too long.
-//-------------------------------------------------------------------------------------------------
-RadarObjectType W3DRadar::removeObject( Object* obj )
-{
-	RadarObjectType removedType = Radar::removeObject(obj);
-
-	if (removedType == RadarObjectType_Local)
-	{
-		stl::find_and_erase_unordered(m_cachedHeroObjectList, obj);
-	}
-
-	return removedType;
-}
-
 //-------------------------------------------------------------------------------------------------
 /** Reset the radar for the new map data being given to it */
 //-------------------------------------------------------------------------------------------------