OutdatedKnownStaticMeshDetected caused by the order of RepNotifies between UStaticMeshComponent vs Actor and other components

Development Programming & Scripting Multiplayer & Networking
, ,
1

REPRO STEPS

Run the repro in Play in Editor with a client server setup.

Have a replicated actor with a replicated static mesh component.

Give it a staticmesh.

Give the actor another replicated variable with an OnRep.

In the OnRep for that other variable, on the client, call SetMaterial on the static mesh component

On the server, on the same frame, change the mesh with SetStaticMesh, and also change the other replicated variable.

Observe that the ensure in OutdatedKnownStaticMeshDetected gets triggered on the Client.

COMMENTARY

We ran in to this issue in 5.4. Our use case was very much like what was described in the repro. A developer in a blueprint wanted to change the mesh and the material at the same time on a replicated actor. The UStaticMeshComponent handles the StaticMesh pointer but Material, being non-replicated, needs to be driven through another replicated actor variable.

Im hoping for an easy way to make this non-fragile for other developers.

Hopefully these details help you reconstruct the repro if it isnt as simple with the steps I provided.

Consider how UStaticMeshComponent works.

If StaticMesh is changed, it is in a problematic state until NotifyIfStaticMeshChanged() is called, and will do OutdatedKnownStaticMeshDetected() if you call GetStaticMesh().

OnRep_StaticMesh will call SetStaticMesh(NewStaticMesh) which will result in the UStaticMeshComponent::GetStaticMesh being ok to call again.

Now consider how replication for an actor works, roughly:

  1. Get Data from server about actor (UActorChannel::ProcessBunch)
  2. Sets all the variables in the actor and its components (in UActorChannel::ProcessBunch calling Replicator->ReceivedBunch)
  3. Calls all the rep-notifies, starting with the actor followed by the components. (In UActorChannel::ProcessBunch iterates ReplicationMap calling PostReceivedBunch)

When we comine all this, we see that step 2 will modify UStaticMeshComponent::StaticMesh, but not call OnRep_StaticMesh until step 3. All the other onrep code for the Actor and potentially other components will run before OnRep_StaticMesh has fixed up UStaticMeshComponent and so doing things such as SetMaterial will call GetStaticMesh and lead to OutdatedKnownStaticMeshDetected() even through replication is about to call OnRep_StaticMesh and fix it.

2

Hi,

This is a known issue with the static mesh property’s OnRep: Unreal Engine Issues and Bug Tracker (UE\-156362)

The original report’s problem was caused by the ordering of OnRep_ReplicatedMovment and OnRep_StaticMesh, but I’ve added your info here to the internal tracker for the issue.

I can’t provide an estimate as to when that bug may be fixed, and in the meanwhile, I’m unfortunately not sure there’s a straightforward way to guard against this ordering issue.

One option could be to move the SetMaterial call from an OnRep to PostRepNotifies, which is called after all of the object’s rep notify functions. However, this may not help in the case where the replicated property is on a different object than the static mesh property.

You could set the static mesh component as non-replicated and instead have both the mesh and material replication driven through separate properties, giving better control over when the actual static mesh and material are set on the client. If you need other properties on the static mesh component to be replicated, you could maybe create a derived class and override the component’s GetLifetimeReplicatedProps function to mark the static mesh property as not replicated.

Another option could be to move this functionality to a RPC, rather than handling it through property replication.

Thanks,

Alex

3

Hi Alex, always good to see you on a replication-related question :slight_smile:

Thanks for the suggested workarounds. I wanted to suggest one other possibility.

I suspect that the issue is benign, i.e this SetMaterial still seems to works fine, even though the NotifyIfStaticMeshChanged comes in late.

I was thinking of just downgrading the `ensureAlwaysMsgf` from an ensure to a log warning. Because it doesnt seem likely to catch an issue introduced by our game-engineers, outside of them messing with the implementation of UStaticMeshComponent which is rare. So it might just not be worth having an ensure in here for us vs the hassle of false positives.

I just wanted to sanity check that this is indeed a false positive and that this doesnt indicate a real problem with the SetMaterial happening in the gap between replication variable changes and OnRep ?

What do you think? Does it seem reasonable for our game to just downgrade the ensure to a log message?

4

Hi,

After discussing this a bit, I don’t think we’d recommend downgrading this ensure. If the StaticMesh value has not been properly updated yet, the FObjectCacheContext won’t have the correct mapping, so queries relying on this data can update the wrong things.

Thanks,

Alex

5

So you’re saying this ensure is detecting an actually problem with the way replication updates the pointer on the client without immediately calling the notify method? Even though the client will call the notify when the getter is called because of the detection logic?

Essentially would you actually recommend we change our feature implementation here? Is the feature implementation something that is unsafe in unreal? Because if the implementation is safe and simple. It seems the ensure is the real problem?

6

Hi,

I do believe the issue here lies in the interaction between the replication system and the StaticMeshComponent, as like you pointed out, the engine will update the StaticMesh pointer property before later calling OnRep_StaticMesh, allowing other OnRep functions to attempt to access the static mesh before it has been properly set. The static mesh component expects that the handling in SetStaticMesh will be done when the StaticMesh is changed, rather than this property just getting set directly, and the replication code seems to break this expectation.

I have observed what you’ve described here, where OutdatedKnownStaticMeshDetected will still call NotifyIfStaticMeshChanged as a last resort, leading to the material being set without issue. Even in a packaged repro project, I didn’t see any errors or other issues. Having discussed this some more with a dev more familiar with the static mesh component, you can silence the ensure, but it does come with the risk that materials/textures/etc. may not apply correctly in this gap where the reverse lookup is out of date. If you do run into issues with this ensure disabled, please don’t hesitate to reach out.

I’ve also updated UE-156362 to reflect this new repro, as I believe the previous situation that caused that issue to be opened has been addressed.

Thanks,

Alex