We’ve been tracking down a problem recently with the Gameplay Ability System where a gameplay effect is properly removed on the server, but the client has the gameplay effect stuck on permanently.
This appears to be caused by multiple items in the client’s ActiveGameplayEffects fast array having the same FFastArraySerializerItem::ReplicationID.
The first item gets sent down to the client with that ReplicationID from the server via PostReplicatedAdd. The second item is a locally predicted effect that calls MarkItemDirty in FActiveGameplayEffectsContainer::OnMagnitudeDependencyChange. That client MarkItemDirty is not generating IDs in relation to the server, so those ReplicationID numbers can both match.
Then when the client calls FFastArraySerializer::TFastArraySerializeHelper<Type, SerializerType>::ConditionalRebuildItemMap upon receiving an active gameplay effect removal from the server, it will try to place both items with the same ReplicationID in the map. Since the locally predicted effect is further down in the ActiveGameplayEffects list, it will take the spot in the ArraySerializer.ItemMap.
And then in FFastArraySerializer::TFastArraySerializeHelper<Type, SerializerType>::ReadDeltaHeader, the client looks at that map and will find the incorrect, predictive effect and remove that instead of the server gameplay effect that initially had that ReplicationID (and that the server intended to remove).
As far as I can tell, this is not working as intended. It doesn’t seem like ConditionalRebuildItemMap should ever have multiple items with the same key that it’s trying to place in the map. Additionally, it doesn’t seem like the client should ever be calling MarkItemDirty on these elements, since it opens up a potential issue with overlapping ReplicationIDs. This seems to be backed up by this existing comment in FActiveGameplayEffectsContainer::ApplyGameplayEffectSpec:
if (InPredictionKey.IsLocalClientKey() == false || IsNetAuthority()) // Clients predicting a GameplayEffect must not call MarkItemDirty
Our fix for the problem is to make that same check before calling MarkItemDirty in OnMagnitudeDependencyChange to ensure that the client isn’t marking elements dirty.
Can we get confirmation on these conclusions (about the intended use of MarkItemDirty for the gameplay effects array, the potential solution, etc.)? If those conclusions are accurate, is a similar change needed for other cases of MarkItemDirty for the active gameplay effects container (there are many calls to MarkItemDirty in GameplayEffect.cpp)? Moreover, should we be making sure that any use of FastArrays (in other parts of our project) prevent clients from marking items dirty?