Announcement

Collapse
No announcement yet.

Weird replication problem if replicated property set to class default

Collapse
X
 
  • Filter
  • Time
  • Show
Clear All
new posts

  • replied
    I hate to bump, but I just tried this on 4.26 and still having this problem. I submitted a bug report back when I first had this issue.

    Is replication just broken in this scenario? How are other people handling things such as the replication of lock status on doors that default to locked (or inventories that don't default to empty and are emptied when the client joins the server)?

    Leave a comment:


  • replied
    If its any use I downloaded your project and tried it in the latest 4.26 preview and it is still occurring, so it hasn't been accidentally fixed or anything

    Leave a comment:


  • replied
    Originally posted by TheJamsh View Post
    As another workaround, and something a bit more reliable - my suggestion would be to remove the door actors from the level, and instead create a "Door Spawner" object instead. The Spawner will spawn a new actor at runtime Server-Side, and set properties accordingly. I do this quite a lot in our current game as the different lifetimes of pre-placed actors vs runtime-spawned ones in the networking can create some issues for us, so it's just easier to place a spawner which ensures the same code path is used for all those actors.
    Thanks for the suggestion. I'll give this a try and see if it helps.

    Originally posted by TheJamsh View Post
    Also post it here if you do so, I'd be interested to test it out)
    I did actually build a project to add to my bug report this morning. Attaching it to this post.

    Contains C++ versions as well as Blueprint (since it happens in both) and some test maps (each containing 3 instances of the replication test actor) to demonstrate the issue:
    • _0_0_0_to_0_0_0: Editor has values set to 0,0,0 and shows correctly that no replication occurs when the value is set to 0,0,0
    • _0_1_2_to_0_1_2: Editor has values set to 0,1,2 and shows correctly that no replication occurs when the value is set to 0,1,2
    • _0_1_2_to_0_0_0: Editor has values set to 0,1,2 and shows failed replication on two of the actors when values are set to 0,0,0
    • _0_1_2_to_3_4_5: Editor has values set to 0,1,2 and shows correctly that replication occurs when values are set to 3,4,5
    There's also an actor ReplicationTestMinusOne which has the ReplicationTest property defaulting to -1. Replication works correctly in the above maps with this example.

    BP_ReplicationTest is a blueprint version of the actor which shows that the same replication problem occurs with blueprints.

    There's also a folder which demonstrates how it works okay with a delay between setting the ReplicationTests to 0,0,0 in the TestWithDelay folder. I test this using the experimental 'Allow late joining' option. I'll run the server+client 1 and then join a client 2 once the existing server and client have reported their values to the console.

    The C++ classes all log to console and I filter with AReplicationTest. BP test prints to screen.
    Attached Files

    Leave a comment:


  • replied
    The `Channel` will be null unless the actor is currently network relevant and non-dormant for that connection (since channels are closed and reopened as actors go in and out of relevancy and dormancy), so it's an unreliable method really.

    It's hard to know exactly what's occuring here, but if ForceNetUpdate() and ForcePropertyCompare() don't fix the issue, then the next logical explanation is the Server simply thinks the Client is up-to-date (since all of those functions result in the same thing ultimately). You may be able to drop some breakpoints in RepLayout.cpp to try and get some deeper understanding as to why, but it's going to take some deep digging I suspect.

    As another workaround, and something a bit more reliable - my suggestion would be to remove the door actors from the level, and instead create a "Door Spawner" object instead. The Spawner will spawn a new actor at runtime Server-Side, and set properties accordingly. I do this quite a lot in our current game as the different lifetimes of pre-placed actors vs runtime-spawned ones in the networking can create some issues for us, so it's just easier to place a spawner which ensures the same code path is used for all those actors.

    BTW - I certainly think it would be worth submitting a bug report with a small repro project to Epic. It might take some time, but if there is an underlying issue they'll be able to fix it quicker. (Also post it here if you do so, I'd be interested to test it out)

    Leave a comment:


  • replied
    If that isn't working then it's because the changes made for Fortnite
    I don't know about alternative methods.

    Leave a comment:


  • replied
    Thanks, BrUnO!

    What's the best place to do this on the server? I thought doing it in PostLogin would work, so popped this code in GameModeBase's PostLogin(APlayerController* NewPlayer)

    Code:
    TArray<AActor*>Reps;
    UGameplayStatics::GetAllActorsOfClass(GetWorld(), AReplicationTest::StaticClass(), Reps);
    for (auto& Actor : Reps)
    {
        if (bDebugPrint) UE_LOG(LogTemp, Display, TEXT("%s [Client %i] AReplicationTest %s (ReplicationTest: %i)"), __FUNCTIONW__, GPlayInEditorID, *Actor->GetName(), Cast<AReplicationTest>(Actor)->ReplicationTest);
    
        if (NewPlayer->GetNetDriver())
        {
             for (UNetConnection* Connection : NewPlayer->GetNetDriver()->ClientConnections)
             {
                UActorChannel* Channel = Connection->FindActorChannelRef(Actor);
    
                if (Channel != nullptr)
                   Channel->ReplicateActor();
             }
         }
    }
    (ActorChannels is a private in 4.24, so I replaced that with FindActorChannelRef, which I think is the right function?)

    Channel always seems to be nullptr there and so it never gets to the ReplicateActor part.

    I also tried in the PlayerController's BeginPlay but this was the same. The only place I could get Channel to be not nullptr is in the Tick (after having been connected for a while), but it still didn't replicate or ever trigger the onRep. I'm guessing it's still assuming the values have't changed and the client has the correct values, and so doesn't replicate?

    Leave a comment:


  • replied
    You can try to force a replication once your client has joined. Maybe something like:

    Code:
    if (myActor->GetNetDriver())
    {
        for (UNetConnection* Connection : myActor->GetNetDriver()->ClientConnections)
        {
            UActorChannel* Channel = Connection->ActorChannels.FindRef(myActor);
    
            if​​​ (Channel != nullptr)
                Channel->ReplicateActor();
        }
    }

    Leave a comment:


  • replied
    Originally posted by TheJamsh View Post
    Looking through again, I think I can spot a bug which I also ran into and took a while to track down. I've brought this up with Epic on UDN, and the advice I recieved from Epic was to NOT call SetReplicates() in the constructor, and instead set bReplicates = true; directly (also, be sure to call the parents' constructor).

    What happened was that SetReplicates() actually adds the actor to the networking system in it's state there and then, but it somehow also caused the system to ignore any deserialized values after that.
    Thanks for the suggestion! That didn't seem to change anything, unfortunately. It does look like SetReplicate adds the actor and its state but that doesn't seem to affect anything with this issue.

    A nice easy workaround would be if I could just clear the 'cache' that the server replication is using to compare for changes (or force a property cache update). If I could clear it before loading a save game I'm pretty sure it would fix this problem. I've tried ForceNetUpdate, ForcePropertyCompare etc. but none of them seem to do much.

    Leave a comment:


  • replied
    It does - so I would file a bug report anyway.

    Looking through again, I think I can spot a bug which I also ran into and took a while to track down. I've brought this up with Epic on UDN, and the advice I recieved from Epic was to NOT call SetReplicates() in the constructor, and instead set bReplicates = true; directly (also, be sure to call the parents' constructor).

    What happened was that SetReplicates() actually adds the actor to the networking system in it's state there and then, but it somehow also caused the system to ignore any deserialized values after that.

    I was setting Network Dormancy to DORM_Initial for example, but Replication Graph was adding it in DORM_Awake state (the parents' default value). It took a while to work out why that was happening, but the callstack revealed that the actor was being added to the networking system before it was fully setup and deserialized.

    It's stupid I know. My suggestion to Epic was that the block/assert on calls to SetReplicates in the constructor because it can create all kinds of problems, but it doesn't look like they've changed that yet.

    ---

    RPC calls make no sense here, and it's just a recipe for a buggy unmaintanable system if you have to use them to "fix" some other underlying issue, so I agree you shouldn't use them. Properties should be used for anything that determines state.

    That being said, sometimes structs with wrapping counters in place of bools can be useful to detect changes you would otherwise miss (e.g, a door opening and closing on same network frame), but that doesn't seem like the issue here. There's nothing wrong with struct replication BTW, it works fine.
    Last edited by TheJamsh; 10-27-2020, 07:19 AM.

    Leave a comment:


  • replied
    Surely it's a bug, though, that replicated properties on the authority can be mismatched from the newly connecting clients? Doesn't this defeat the point of replicated properties and undermine server authority?

    Leave a comment:


  • replied
    I doubt they will accept a bug report, this is part of core optimizations for development of Fortnite.

    Leave a comment:


  • replied
    I had considered an RPC call, but it seemed like it would be redundant -
    • If i had a door that has bLocked as false in the umap, but true in the savegame, replication would work correctly and so I'd be sending an RPC on top of onrep,
    • With something like a 'locked door' actor, the server owns the actor and so I'd have to use NetMulticast. All the door, chest, etc. statuses would be sent again to every player every time someone joins the game.
    I guess I could send an RPC to something like the PlayerController via Client, Reliable on BeginPlay so that only the new player receives it.

    I'm not too keen on using a struct as it feels like it would make actor setup in the editor more obfuscated/complicated/hacky. RIght now I pop down a 'Door' actor and just tick a 'Locked' checkbox. A struct would look a bit messier. I'd also read bad things about structs in structs and so have avoided them so far where not needed (though this might be older UE4 versions). It sounds like this may be the only way to get around this, though - thanks for the suggestion!

    The only other idea I had was to use a hidden enum for the door locked state that had 'Default', 'Unlocked' and 'Locked', but this too would result in redundant data being sent (ie. if a door is unlocked in the umap it would be sending this unlocked replication, even though the door is already unlocked on the client from the client umap serialization).

    Perhaps I should rethink how I'm doing doors/chests/etc. so that it's RPC based rather than property replication. It just seemed that property replication was perfect for something like this and what it should be used for.

    I've submitted this as a bug. It feels like post-serialization/duplication should be the starting point for FProperty replication and not the constructor?
    Last edited by BtotheLaker; 10-26-2020, 10:36 PM.

    Leave a comment:


  • replied
    Some people make a struct with a byte flag or a boolean property then flip it true/false just to force a replication where nothing really changed.

    Others just make a RPC call somewhere to force update values.

    Leave a comment:


  • replied
    Originally posted by BrUnO XaVIeR View Post
    Default server behavior is that a FProperty which current value didn't change (on server), since last replication cycle, will not be picked for a replication broadcast next frame; That also applies to members of a replicated structure (UE3/UDK sends the whole struct, UE4 picks a member value change).
    Taking an example where ReplicationTest is set to 1 in the umap, is this caused because ReplicationTest is set to 0 in the constructor,1 when read from serialized and then 0 in the BeginPlay? (ie this all happening too fast/i the same frame and so the server thinks it never changed from 0 and doesn't replicate)?

    Is there any kind of workaround for this problem? This seems like a bit of a big flaw - as it stands I can't figure out a way for the client to know if the value should be the constructor default value, or the map serialize value in this situation. There seems to be no way to tell? It seems less than ideal that replicated values differ on the client from the authority as soon as a player connects.

    Is there a way to force a replication cycle, or force the caching of an FProperty for replication?
    Last edited by BtotheLaker; 10-26-2020, 10:01 PM.

    Leave a comment:


  • replied
    Default server behavior is that a FProperty which current value didn't change (on server), since last replication cycle, will not be picked for a replication broadcast next frame; That also applies to members of a replicated structure (UE3/UDK sends the whole struct, UE4 picks a member value change).

    Leave a comment:

Working...
X