SafeServerUpdateSpectatorState() bug?

Development Programming & Scripting
1

When changing the APlayerController’s state to NAME_Spectating then every PlayerTick() the client calls:

void APlayerController::SafeServerUpdateSpectatorState()
{
	if (IsInState(NAME_Spectating))
	{
		if (GetWorld()->TimeSince(LastSpectatorStateSynchTime) > RetryServerCheckSpectatorThrottleTime)
		{
			ServerSetSpectatorLocation(GetFocalLocation());
			LastSpectatorStateSynchTime = GetWorld()->TimeSeconds;
		}
	}
}

If the client then is in NAME_Spectating state and some amount of time has passed it calls a function on the server:

void APlayerController::ServerSetSpectatorLocation_Implementation(FVector NewLoc)
{
	if ( IsInState(NAME_Spectating) )
	{
		if ( GetWorld()->TimeSeconds - LastSpectatorStateSynchTime > 2.f )
		{
			ClientGotoState(GetStateName());
			LastSpectatorStateSynchTime = GetWorld()->TimeSeconds;
		}
	}
	// if we receive this with !bIsSpectating, the client is in the wrong state; tell it what state it should be in
	else if (GetWorld()->TimeSeconds != LastSpectatorStateSynchTime)
	{
		.... // left out since not important for question
	}
}

This function on the server every two seconds calls ClientGotoState(GetStateName());. StateName is of course NAME_Spectating.

Conclusively, Client calls server function when in a certain state. This server function tells the client to be in exactly that state.
Loop?

I don’t understand the sense behind this.

2

It’s true that ServerSetSpectatorLocation is only ever called from a client when the client is in the spectating state. So the extra call inside ServerSetSpectatorLocation_Implementation to set the client to that very state seems redundant, and wasteful.

I suppose if the client called that function but wasn’t in the spectator state (but the server was), this would make more sense (though this flow seems to not be possible in the stock code).

It also seems benign though, unless this is causing issues that you know of. It definitely seems like an area where we should make sure there isn’t some unnecessary redundancy, and fix if possible, so thanks for pointing this out!

3

Thanks for your answer John!

I was just confused once I’ve overwritten the function ClientGoToState(…) and found myself ending up within that function quite a lot when setting a breakpoint without any obvious reason.

I first thought it might serve against some client cheat. But since it’s only called when client is in spectating state anyways…