Conversation
I think the duplicate removal logic is excessive it may hide issues with the save/restore procces
|
Thanks! Can anyone who uses WP put this through their paces? I'll ping the Discord to see. |
|
For those who want to test it, it's better to use console command: |
Dynamically spawned actors can be retained in a World Partition cell during streaming, until the cell's data is collected by the GC: https://dev.epicgames.com/community/learning/knowledge-base/r6wl/unreal-engine-world-building-guide#wp-limitations To avoid creating and deleting duplicatesl, I added a GUID check.
| TMap<FGuid, UObject*> PersistentObjectsByGuid; | ||
| TArray<AActor*> LevelActorsToRestore; | ||
| TArray<AActor*> RoamingActorsToRestore; | ||
|
|
||
| // Collect persistent actors and pre-populate guid map | ||
| for (AActor* Actor : Level->Actors) | ||
| { | ||
| if (!Actor) continue; | ||
| if (!SpudPropertyUtil::IsPersistentObject(Actor)) continue; | ||
|
|
||
| LevelActorsToRestore.Add(Actor); | ||
|
|
||
| const FGuid Guid = SpudPropertyUtil::GetGuidProperty(Actor); | ||
| if (Guid.IsValid()) | ||
| PersistentObjectsByGuid.Add(Guid, Actor); | ||
| } |
There was a problem hiding this comment.
If the level data was not collected by GC at the time of restoration, this can cause issues with duplicates. I added pre-collection of level actor GUIDs, since there may be dynamic actors among them. This fixes the behavior of WP cells if their data was not collected by GC between their save and load states, but it will also fix other errors caused by the same reason.
There was a problem hiding this comment.
Thanks for the clarification - maybe you can put this in the comments as well because it's slightly unintuitive and someone might think it's not needed without the GC angle being mentioned.
There was a problem hiding this comment.
@sinbad You might want to take a look at void USpudState::RestoreLevel(ULevel* Level), since the function has changed significantly and I might have missed some nuances.
There was a problem hiding this comment.
I had a brief look and it seemed sensible but I'll take a better look soon. Are you done with this PR barring feedback?
There was a problem hiding this comment.
Yes, I don’t plan to make any changes in the near future, unless I find some critical errors.
I spent some time working on actors that move between WorldPartition cells. The idea is that roaming actors "live" in the PersistentLevel, but are saved and loaded per WorldPartition cell. If a roaming actor crosses into the bounds of an inactive cell, it will save itself into that cell.
For a roaming actor to function correctly, it must implement the
ISpudRoamingActorinterface and register itself withSpudRoamingActorSubsystem. From that point, its save and destroy lifecycle is fully managed by the subsystem. Upon restoration, the actor will always belong to the PersistentLevel rather than the cell it was restored from.USpudRuntimeStoredActorComponentis kept as-is for now, it only subscribes and unsubscribes its owner from subsystem tracking.Currently, a separate interface is used to distinguish roaming actors during the SPUD save and restore process, there might be a better way to handle this.