Incorrect networking role for Actors during possession swaps when using push model

Development Programming & Scripting
, , ,
1

I believe I’ve found an edge case for Push Model Replication that causes Actors to be in the incorrect Network Role when swapping possession. This is similar to UE-66313, but only when Push Model is enabled and Actors with full Push Model support are involved.

A fix was made in 5.4 for this issue here, but the bForceCompareProperties flag is not respected for objects that have full Push Model support. This happens in CompareParentProperties()in “RepLayout.cpp” for the branch considering the case where ERepLayoutFlags::FullPushProperties is true for the object.

The issue requires multiple network connections to repro. It does not reproduce for the first client connection–only for other connections. The steps are as follows:

  1. Launch a PIE session with at least 2 clients and the ability to change possession.
  2. The Actors involved should have full Push Model support, and Push Model should be enabled.
  3. On the second client, swap possession to a different pawn.
  4. Observe the previously-possessed pawn has the role ROLE_AutonomousProxy instead of the expected ROLE_SimulatedProxy.

The reason this only happens on the second client is subtle. The outline of the problem is roughly as follows:

  1. On a possession swap, AActor::SetAutonomousProxy() is used to change the network role when the Actor is unpossessed. This calls AActor::ForcePropertyCompare(), which should ensure all properties are checked–regardless of the dirty state of properties for that frame, essentially disabling shared shadow state.
  2. On the first connection during replication, the previously-possessed Actor has all properties compared. Prior to the swap, FScopedRoleDowngrade would have downgrade the role to the first connection (since the Actor is owned by the second connection). In ~FScopedRoleDowngrade(), the role would be restored after replication and the RemoteRole property would be marked dirty. However, now that the Actor has changed its role to ROLE_SimulatedProxy, no downgrade happens and RemoteRole is not marked dirty at the end of replication to the first connection.
  3. Once replication to the first connection finishes, the dirty state is cleared in CompareParentProperties()using SharedParams.PushModelState->ResetDirtyStates(). Now the Actor has no dirty properties.
  4. Next, replication starts on the second connection. Since the Actor has full Push Model support, it takes the branch in CompareParentProperties() where EnumHasAnyFlags(SharedParams.Flags, ERepLayoutFlags::FullPushProperties) is true.
  5. [BUG] This is where the trouble is. This branch only iterates over dirty properties by calling SharedParams.PushModelState->GetDirtyProperties(), which bypasses IsPropertyDirty(). The flag bForceCompareProperties is not respected. This means CompareRoleProperty() never gets called to check if the saved remote role has changed from the Actors new role. See SavedRemoteRole in FSendingRepState.
  6. Now, the previously-possessed Actor has the wrong network role.

My proposed fix is to update CompareParentProperties() in “RepLayout.cpp” to have the following:

// Old
if (UNLIKELY(SharedParams.bForceFail))
{
  ...
}

// New
if (UNLIKELY(SharedParams.bForceFail || SharedParams.bForceCompareProperties))
{
  ...
}

This feels like an opportunity to simplify IsPropertyDirty() in “RepLayout.cpp” to remove the check for bForceCompareProperties. The only code path that looks like it may be circumvented would be the validation path with WITH_PUSH_VALIDATION_SUPPORT.

1 Like