Merge branch 'player-pointer-deletion' into 'master'

Player Pointer Deletion

Closes #529 and #576

See merge request KartKrew/Kart!1341

src/d_clisrv.c

@@ -2775,20 +2775,36 @@ void CL_ClearPlayer(INT32 playernum)
 			K_RemoveFollower(&players[playernum]);
 		}
 
-		if (players[playernum].mo)
-		{
-			P_RemoveMobj(players[playernum].mo);
-			P_SetTarget(&players[playernum].mo, NULL);
+#define PlayerPointerRemove(field) \
+		if (field) \
+		{ \
+			P_RemoveMobj(field); \
+			P_SetTarget(&field, NULL); \
 		}
 
+		// These are mostly subservient to the player, and may not clean themselves up.
+		PlayerPointerRemove(players[playernum].mo);
+		PlayerPointerRemove(players[playernum].followmobj);
+		PlayerPointerRemove(players[playernum].stumbleIndicator);
+		PlayerPointerRemove(players[playernum].sliptideZipIndicator);
+
+#undef PlayerPointerRemove
+
+		// These have thinkers of their own.
+		P_SetTarget(&players[playernum].whip, NULL);
+		P_SetTarget(&players[playernum].hand, NULL);
+		P_SetTarget(&players[playernum].hoverhyudoro, NULL);
+		P_SetTarget(&players[playernum].ringShooter, NULL);
+
+		// TODO: Any better handling in store?
+		P_SetTarget(&players[playernum].flickyAttacker, NULL);
+		P_SetTarget(&players[playernum].powerup.flickyController, NULL);
+
+		// These are camera items and possibly belong to multiple players.
 		P_SetTarget(&players[playernum].skybox.viewpoint, NULL);
 		P_SetTarget(&players[playernum].skybox.centerpoint, NULL);
 		P_SetTarget(&players[playernum].awayview.mobj, NULL);
-		P_SetTarget(&players[playernum].followmobj, NULL);
-		P_SetTarget(&players[playernum].hoverhyudoro, NULL);
-		P_SetTarget(&players[playernum].stumbleIndicator, NULL);
-		P_SetTarget(&players[playernum].sliptideZipIndicator, NULL);
-		P_SetTarget(&players[playernum].ringShooter, NULL);
+		
 	}
 
 	// Handle parties.

src/g_game.c

@@ -2455,10 +2455,8 @@ void G_PlayerReborn(INT32 player, boolean betweenmaps)
 	INT32 followerskin;
 	UINT16 followercolor;
 
-	mobj_t *follower; // old follower, will probably be removed by the time we're dead but you never know.
-	mobj_t *hoverhyudoro;
-	mobj_t *skyboxviewpoint;
-	mobj_t *skyboxcenterpoint;
+	mobj_t *ringShooter, *hoverhyudoro;
+	mobj_t *skyboxviewpoint, *skyboxcenterpoint;
 
 	INT32 charflags;
 	UINT32 followitem;
@@ -2677,22 +2675,40 @@ void G_PlayerReborn(INT32 player, boolean betweenmaps)
 
 	if (!betweenmaps)
 	{
-		follower = players[player].follower;
-		P_SetTarget(&players[player].follower, NULL);
-		P_SetTarget(&players[player].awayview.mobj, NULL);
-		P_SetTarget(&players[player].stumbleIndicator, NULL);
+		K_RemoveFollower(&players[player]);
+
+#define PlayerPointerRemove(field) \
+		if (field) \
+		{ \
+			P_RemoveMobj(field); \
+			P_SetTarget(&field, NULL); \
+		}
+
+		// These are mostly subservient to the player, and may not clean themselves up.
+		PlayerPointerRemove(players[player].followmobj);
+		PlayerPointerRemove(players[player].stumbleIndicator);
+		PlayerPointerRemove(players[player].sliptideZipIndicator);
+
+#undef PlayerPointerRemove
+
+		// These will erase themselves.
 		P_SetTarget(&players[player].whip, NULL);
 		P_SetTarget(&players[player].hand, NULL);
-		P_SetTarget(&players[player].ringShooter, NULL);
-		P_SetTarget(&players[player].followmobj, NULL);
 
+		// TODO: Any better handling in store?
+		P_SetTarget(&players[player].awayview.mobj, NULL);
+		P_SetTarget(&players[player].flickyAttacker, NULL);
+		P_SetTarget(&players[player].powerup.flickyController, NULL);
+
+		// The following pointers are safe to set directly, because the end goal should be refcount consistency before and after remanifestation.
+		ringShooter = players[player].ringShooter;
 		hoverhyudoro = players[player].hoverhyudoro;
 		skyboxviewpoint = players[player].skybox.viewpoint;
 		skyboxcenterpoint = players[player].skybox.centerpoint;
 	}
 	else
 	{
-		follower = hoverhyudoro = NULL;
+		ringShooter = hoverhyudoro = NULL;
 		skyboxviewpoint = skyboxcenterpoint = NULL;
 	}
 
@@ -2769,9 +2785,8 @@ void G_PlayerReborn(INT32 player, boolean betweenmaps)
 	if (saveroundconditions)
 		memcpy(&p->roundconditions, &roundconditions, sizeof (p->roundconditions));
 
-	if (follower)
-		P_RemoveMobj(follower);
-
+	// See above comment about refcount consistency.
+	p->ringShooter = ringShooter;
 	p->hoverhyudoro = hoverhyudoro;
 	p->skybox.viewpoint = skyboxviewpoint;
 	p->skybox.centerpoint = skyboxcenterpoint;

src/k_kart.c

@@ -8065,9 +8065,6 @@ void K_KartPlayerThink(player_t *player, ticcmd_t *cmd)
 
 	if (player->guardCooldown)
 		player->guardCooldown--;
-		
-	if (player->whip && P_MobjWasRemoved(player->whip))
-		P_SetTarget(&player->whip, NULL);
 
 	if (player->startboost > 0 && onground == true)
 	{
@@ -8326,20 +8323,11 @@ void K_KartPlayerThink(player_t *player, ticcmd_t *cmd)
 		player->tripwireState = TRIPSTATE_NONE;
 	}
 
-	if (player->hand && P_MobjWasRemoved(player->hand))
-		P_SetTarget(&player->hand, NULL);
-
-	if (player->flickyAttacker && P_MobjWasRemoved(player->flickyAttacker))
-		P_SetTarget(&player->flickyAttacker, NULL);
-
-	if (player->powerup.flickyController && P_MobjWasRemoved(player->powerup.flickyController))
-		P_SetTarget(&player->powerup.flickyController, NULL);
-
 	if (player->spectator == false)
 	{
 		K_KartEbrakeVisuals(player);
 
-		Obj_ServantHandHandling(player);
+		Obj_ServantHandSpawning(player);
 	}
 
 	if (K_GetKartButtons(player) & BT_BRAKE &&

src/k_objects.h

@@ -144,7 +144,8 @@ void Obj_GachaBomReboundThink(mobj_t *mobj);
 void Obj_SpawnGachaBomRebound(mobj_t *source, mobj_t *target);
 
 /* Servant Hand */
-void Obj_ServantHandHandling(player_t *player);
+void Obj_ServantHandSpawning(player_t *player);
+void Obj_ServantHandThink(mobj_t *hand);
 void Obj_PointPlayersToXY(fixed_t x, fixed_t y);
 
 /* Super Flicky Controller */

src/objects/ring-shooter.c

@@ -63,8 +63,11 @@ static void RemoveRingShooterPointer(mobj_t *base)
 	}
 
 	// NULL the player's pointer.
-	player = &players[ rs_base_playerid(base) ];
-	P_SetTarget(&player->ringShooter, NULL);
+	if (playeringame[ rs_base_playerid(base) ])
+	{
+		player = &players[ rs_base_playerid(base) ];
+		P_SetTarget(&player->ringShooter, NULL);
+	}
 
 	// Remove our player ID
 	rs_base_playerid(base) = -1;

src/objects/servant-hand.c

@@ -8,7 +8,7 @@
 #include "../r_main.h"
 #include "../g_game.h"
 
-void Obj_ServantHandHandling(player_t *player)
+void Obj_ServantHandSpawning(player_t *player)
 {
 	if (player->pflags & PF_WRONGWAY || player->pflags & PF_POINTME)
 	{
@@ -36,71 +36,86 @@ void Obj_ServantHandHandling(player_t *player)
 				}
 			}
 		}
-
-		if (player->hand)
-		{
-			player->hand->destscale = mapobjectscale;
-		}
 	}
 	else if (player->handtimer != 0)
 	{
 		player->handtimer--;
 	}
+}
+
+void Obj_ServantHandThink(mobj_t *hand)
+{
+	UINT8 handtimer = 0;
+	player_t *player = NULL;
+
+	if (P_MobjWasRemoved(hand->target))
+	{
+		P_RemoveMobj(hand);
+		return;
+	}
+
+	if (hand->target->health && hand->target->player && hand->target->player->hand == hand)
+	{
+		player = hand->target->player;
+		handtimer = hand->target->player->handtimer;
+	}
 
-	if (player->hand)
 	{
 		const fixed_t handpokespeed = 4;
-		const fixed_t looping = handpokespeed - abs((player->hand->threshold % (handpokespeed*2)) - handpokespeed);
+		const fixed_t looping = handpokespeed - abs((hand->threshold % (handpokespeed*2)) - handpokespeed);
 		fixed_t xoffs = 0, yoffs = 0;
 
-		player->hand->color = player->skincolor;
-		player->hand->angle = player->besthanddirection;
-
-		if (player->hand->fuse != 0)
+		if (hand->fuse != 0)
 		{
 			;
 		}
 		else if (looping != 0)
 		{
-			xoffs = FixedMul(2 * looping * mapobjectscale, FINECOSINE(player->hand->angle >> ANGLETOFINESHIFT)),
-			yoffs = FixedMul(2 * looping * mapobjectscale, FINESINE(player->hand->angle >> ANGLETOFINESHIFT)),
+			xoffs = FixedMul(2 * looping * mapobjectscale, FINECOSINE(hand->angle >> ANGLETOFINESHIFT)),
+			yoffs = FixedMul(2 * looping * mapobjectscale, FINESINE(hand->angle >> ANGLETOFINESHIFT)),
 
-			player->hand->threshold++;
+			hand->threshold++;
 		}
-		else if (player->handtimer == 0)
+		else if (handtimer == 0)
 		{
-			player->hand->fuse = 8;
+			hand->fuse = 8;
 		}
 		else
 		{
-			player->hand->threshold++;
+			hand->threshold++;
 		}
 
-		if (player->hand->fuse != 0)
+		if (hand->fuse != 0)
 		{
-			if ((player->hand->fuse > 4) ^ (player->handtimer < TICRATE/2))
+			if ((hand->fuse > 4) ^ (handtimer < TICRATE/2))
 			{
-				player->hand->spritexscale = FRACUNIT/3;
-				player->hand->spriteyscale = 3*FRACUNIT;
+				hand->spritexscale = FRACUNIT/3;
+				hand->spriteyscale = 3*FRACUNIT;
 			}
 			else
 			{
-				player->hand->spritexscale = 2*FRACUNIT;
-				player->hand->spriteyscale = FRACUNIT/2;
+				hand->spritexscale = 2*FRACUNIT;
+				hand->spriteyscale = FRACUNIT/2;
 			}
 		}
 
-		P_MoveOrigin(player->hand,
-			player->mo->x + xoffs,
-			player->mo->y + yoffs,
-			player->mo->z + player->mo->height + 30*mapobjectscale
-		);
-		K_FlipFromObject(player->hand, player->mo);
+		if (player != NULL)
+		{
+			hand->color = player->skincolor;
+			hand->angle = player->besthanddirection;
 
-		player->hand->sprzoff = player->mo->sprzoff;
+			P_MoveOrigin(hand,
+				player->mo->x + xoffs,
+				player->mo->y + yoffs,
+				player->mo->z + player->mo->height + 30*mapobjectscale
+			);
+			K_FlipFromObject(hand, player->mo);
 
-		player->hand->renderflags &= ~RF_DONTDRAW;
-		player->hand->renderflags |= (RF_DONTDRAW & ~K_GetPlayerDontDrawFlag(player));
+			hand->sprzoff = player->mo->sprzoff;
+
+			hand->renderflags &= ~RF_DONTDRAW;
+			hand->renderflags |= (RF_DONTDRAW & ~K_GetPlayerDontDrawFlag(player));
+		}
 	}
 }
 

src/p_mobj.c

@@ -7380,6 +7380,13 @@ static boolean P_MobjRegularThink(mobj_t *mobj)
 			Obj_MantaRingThink(mobj);
 			break;
 		}
+	case MT_SERVANTHAND:
+		{
+			Obj_ServantHandThink(mobj);
+			if (P_MobjWasRemoved(mobj))
+				return false;
+			break;
+		}
 	case MT_BALLHOG:
 		{
 			mobj_t *ghost = P_SpawnGhostMobj(mobj);
@@ -9831,10 +9838,11 @@ static boolean P_FuseThink(mobj_t *mobj)
 	}
 	case MT_SERVANTHAND:
 	{
-		if (!mobj->target
-			|| P_MobjWasRemoved(mobj->target)
+		if (P_MobjWasRemoved(mobj->target)
+			|| !mobj->target->health
 			|| !mobj->target->player
-			|| mobj->target->player->handtimer == 0)
+			|| mobj->target->player->handtimer == 0
+			|| mobj->target->player->hand != mobj)
 		{
 			P_RemoveMobj(mobj);
 			return false;

src/p_user.c

@@ -4249,6 +4249,25 @@ void P_PlayerThink(player_t *player)
 		player->playerstate = PST_DEAD;
 	}
 
+	// Erasing invalid player pointers
+	{
+#define PlayerPointerErase(field) \
+		if (field && P_MobjWasRemoved(field)) \
+			P_SetTarget(&field, NULL); \
+
+		PlayerPointerErase(player->followmobj);
+		PlayerPointerErase(player->stumbleIndicator);
+		PlayerPointerErase(player->sliptideZipIndicator);
+		PlayerPointerErase(player->whip);
+		PlayerPointerErase(player->hand);
+		PlayerPointerErase(player->ringShooter);
+		PlayerPointerErase(player->hoverhyudoro);
+		PlayerPointerErase(player->flickyAttacker);
+		PlayerPointerErase(player->powerup.flickyController);
+
+#undef PlayerPointerErase
+	}
+
 	player->old_drawangle = player->drawangle;
 
 	P_TickAltView(&player->awayview);