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:
- Launch a PIE session with at least 2 clients and the ability to change possession.
- The Actors involved should have full Push Model support, and Push Model should be enabled.
- On the second client, swap possession to a different pawn.
- 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:
- 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.
- 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.
- 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.
- 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.
- [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.
- 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
Hi,
Thank you for reporting this! With other tasks taking priority, we don’t currently have any estimate as to when full push-model support for the engine’s base pawn classes may be implemented, so for others looking to implement fully push-based pawns in their projects, it is good to know what kinds of issues to look out for.
That being said, I haven’t been able to reproduce this issue in my own test project. I am using the latest engine version, but I haven’t found any changes since CL 26916951 that may affect how bForceCompareProperties is handled for fully push model objects. Inspecting the logs, it does seem that the role properties get compared as expected in both the fully push-based and default case:
Compares After Changing Possession with Fully Push-Based Pawns:
LogRepCompares: VeryVerbose: CompareProperties: Owner: BlueprintGeneratedClass /Game/ThirdPerson/Blueprints/BP_ThirdPersonCharacter_ToPossess.BP_ThirdPersonCharacter_ToPossess_C CompareIndex: 49 HistoryIndex: 3
LogRepCompares: VeryVerbose: CompareParentProperties: Full push properties: Has Dirty: 1
LogRepCompares: VeryVerbose: CompareRoleProperty: bForceFail: 0, SavedRole: ROLE_SimulatedProxy ActorRole: ROLE_AutonomousProxy
LogRepCompares: VeryVerbose: CompareRoleProperty: bForceFail: 0, SavedRole: ROLE_Authority ActorRole: ROLE_Authority
...
LogRepCompares: VeryVerbose: CompareProperties: Owner: BlueprintGeneratedClass /Game/ThirdPerson/Blueprints/BP_ThirdPersonCharacter.BP_ThirdPersonCharacter_C CompareIndex: 51 HistoryIndex: 14
LogRepCompares: VeryVerbose: CompareParentProperties: Full push properties: Has Dirty: 1
LogRepCompares: VeryVerbose: CompareRoleProperty: bForceFail: 0, SavedRole: ROLE_AutonomousProxy ActorRole: ROLE_SimulatedProxy
LogRepCompares: VeryVerbose: CompareRoleProperty: bForceFail: 0, SavedRole: ROLE_Authority ActorRole: ROLE_Authority
...
Compares After Changing Possession with Default Path:
LogRepCompares: VeryVerbose: CompareProperties: Owner: BlueprintGeneratedClass /Game/ThirdPerson/Blueprints/BP_ThirdPersonCharacter_ToPossess.BP_ThirdPersonCharacter_ToPossess_C CompareIndex: 40 HistoryIndex: 3
LogRepCompares: VeryVerbose: CompareParentProperties: Default
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 0 CmdType: PropertyBool Property: bReplicateMovement
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 1 CmdType: PropertyBool Property: bHidden
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 2 CmdType: PropertyBool Property: bTearOff
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 3 CmdType: PropertyBool Property: bCanBeDamaged
LogRepCompares: VeryVerbose: CompareRoleProperty: bForceFail: 0, SavedRole: ROLE_SimulatedProxy ActorRole: ROLE_AutonomousProxy
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 5 CmdType: PropertyObject Property: AttachParent
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 6 CmdType: PropertyVector100 Property: LocationOffset
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 7 CmdType: PropertyVector100 Property: RelativeScale3D
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 8 CmdType: PropertyRotator Property: RotationOffset
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 9 CmdType: PropertyName Property: AttachSocket
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 10 CmdType: PropertyObject Property: AttachComponent
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 11 CmdType: RepMovement Property: ReplicatedMovement
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 12 CmdType: PropertyObject Property: Owner
LogRepCompares: VeryVerbose: CompareRoleProperty: bForceFail: 0, SavedRole: ROLE_Authority ActorRole: ROLE_Authority
...
LogRepCompares: VeryVerbose: CompareProperties: Owner: BlueprintGeneratedClass /Game/ThirdPerson/Blueprints/BP_ThirdPersonCharacter.BP_ThirdPersonCharacter_C CompareIndex: 41 HistoryIndex: 8
LogRepCompares: VeryVerbose: CompareParentProperties: Default
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 0 CmdType: PropertyBool Property: bReplicateMovement
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 1 CmdType: PropertyBool Property: bHidden
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 2 CmdType: PropertyBool Property: bTearOff
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 3 CmdType: PropertyBool Property: bCanBeDamaged
LogRepCompares: VeryVerbose: CompareRoleProperty: bForceFail: 0, SavedRole: ROLE_AutonomousProxy ActorRole: ROLE_SimulatedProxy
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 5 CmdType: PropertyObject Property: AttachParent
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 6 CmdType: PropertyVector100 Property: LocationOffset
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 7 CmdType: PropertyVector100 Property: RelativeScale3D
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 8 CmdType: PropertyRotator Property: RotationOffset
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 9 CmdType: PropertyName Property: AttachSocket
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 10 CmdType: PropertyObject Property: AttachComponent
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 11 CmdType: RepMovement Property: ReplicatedMovement
LogRepCompares: VeryVerbose: CompareProperties_r: CmdIndex: 12 CmdType: PropertyObject Property: Owner
LogRepCompares: VeryVerbose: CompareRoleProperty: bForceFail: 0, SavedRole: ROLE_Authority ActorRole: ROLE_Authority
...
If you have logs or a repro project of your own you’d be willing to share, I’d be happy to take a look at it.
In the meanwhile, we do have an internal task for implementing push-model support for APawn/ACharacter, and I’ve made a note of this issue on that tracker.
Thanks,
Alex