#rb Per.Larsson
#jira UE-156750
#rnx
#preflight 62f212e13b773d04161ee7dd
### Problem
- The payloads stored in map files tend to change more than other assets and would cause a lot more churn in the VA system.
- Some other systems like the landscape component are not able to sensibly continue if their payloads cannot be accessed (heightmaps for exmaple) and would prefer not to allow virtualization.
- As a short term fix we need an option to disable the virtualization on all payloads in map files. Future improvements to the filtering system will allow systems to more easily opt their payloads out of virtualization. When this is functional we might want to change the default from true to false.
### Feature
- The config optionf for this filtering is "[Core.VirtualizationModule]FilterMapContent=True"
- Testing if the owning UObject for a payload is in a umap can get tricky, because we not only need to check the umap but we also need to check if it is in a "_builddata.uasset" file, which is an additional file we store next to a umap containing things like lightmaps etc.
- At the moment we check for this by finding the outermost object for the given owner and check to see if it is a ULevel, UWorld or UMapBuildDataRegistry. This is a bit of a kludge but the types we need to check against are not accessible by this module and making them accessible will pull in a lot of dependencies that we'd prefer not to add.
-- One improvement might be to tag the FLinkerSave with the info we need and passing that into the serialization process rather than trying to work it out ourselves but I am wary of making that change until we are 100% sure that we want to keep this feature.
### Refactor
- Removed IVirtualizationSystem::IsDisabledForObject and replaced it with ::FilterPayload which can return multiple reasons for preventing a payload from virtualizing. (the method was added during 5.1 development so it should be fine to just replace it without deprecation)
-- The original behaviour for FVirtualizationManager::IsDisabledForObject has been moved to FVirtualizationManager::ShouldVirtualizeAsset
- Added a new header to declare enums/types used by the various parts of the virtualization system and started by moving EPayloadFilterReason there from the package trailer header. This allows both the core API and PackageTrailer to use EPayloadFilterReason without creating overburdened header dependencies.
-- EPayloadFilterReason has moved from the UE namespace to UE::Virtualization so the package trailer code needed updating accordingly.
- EditorBulkData will ask the virtualization system for the base filter reason, then add it's own reasons if UE_ENABLE_VIRTUALIZATION_TOGGLE is enabled. This bit of code will be removed for 5.1
[CL 21283179 by paul chipchase in ue5-main branch]
#rb Devin.Doucette
#jira UE-160218
#rnx
#preflight
- Before 21089705 we recorded the package path that were were saving an editor bulkdata to, but since we had no way to "attach" to the file on disk we would not load from it. The changes in 21089705 meant that we could try loading from the package, which fails in some odd ways when the editor domain is enabled.
- Adding the ::wasDetached flag when saving the package should prevent this and restore the previous behaviour.
[CL 21224848 by paul chipchase in ue5-main branch]
#rb Devin.Doucette
#jira UE-159985
#rnx
#preflight 62d6e8d2d76ea4b5032d5d56
### Problem
- When CL 21089705 was submitted, we changed the behaviour to assume that it is only safe to load payloads off disk if the AttachedAr pointer is valid rather than checking for the package path being valid. This allowed us to keep the package path intact after ::DetachFromDisk was called and give better error messages.
- However not all code paths were forcing the AttachedAr pointer to be set, TornOff bulkdata instances in the bulkdata registry for example causing these code paths to fail entirely.
- What we need is a way to not allow bulkdata instances that were attached but have been detached from loading (or at least force them to use the speculative loading path that was added in 21089705) and allow any other bulkdata instance that was never attached, to load it's payload as long as the path is set.
### Fix
- Instead of asking ::IsAttachedToPackageFile we instead ask ::HasAttachedArchive as in most places we are really checking if the AttachedAr pointer is null or not before we manipulate it
- There are two places where what we are really asking is if it is safe to load the payload from disk or not, these places now use a new method ::CanLoadDataFromDisk which checks that the bulkdata has access to a valid package path AND that the transient flag EFlags::WasDetached has not bee set.
-- This means that bulk data instances that are never attached to the packages FLinkerLoad can still load off disk, although this is less safe it reverts us back to the behaviour before CL 21089705was submitted.
### Misc
- Marked the transient flags as such in the code documentation.
- Ideally these flags would not have been added to the EFlags enum and a specific ETransientFlag enum added instead but the clean up for that would be irksome at this point.
[CL 21174617 by paul chipchase in ue5-main branch]
#rb none
#jira UE-159866
#rnx
#preflight 62d6930f164251d065d64113
- During 21147257 one of the checks to disable payload writing, that was calling ::IsReferencingByPackagePath was changed to checking the EFlags::ReferencesLegacyFile flag directly, which meant we were no longer taking referencing payloads from the workspace domain into account.
-- This was due to the CL being an attempt to merge 20965910 (which only checked the flag) with UE5 code (which called ::IsReferencingByPackagePath due to better editor domain support)
- This would cause the payload to be both referenced and written out into the editor domain, so on load we would end up with a flag telling the bulkdata to load from the original workspace domain file BUT the offset would reference where in the editor domain the payload was written out too.
-- This would only affect older versions of the package, newer versions would continue to use the package trailer to find the correct offet to use when referencing.
- This change requires that we change the EditorDomainVersion to clear out anything people might have saved with the incorrect info.
#ushell-cherrypick of 21169892 by paul.chipchase
[CL 21173890 by paul chipchase in ue5-main branch]
#rb Francis.Hurteau
#preflight 62d518cf3c3df32390a87183
- This is a rework of CL 20965910, to work with the newer code in editor bulkdata.
- bKeepFileDataByReference will be set to false as cooked packages will probably not be able to reference workspace domain payloads.
- When cooking we will use the legacy serialization path (i.e not to package trailer) BUT we will also not write out a payload at the moment.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 21147257 via CL 21147267 via CL 21147273
#ROBOMERGE-BOT: UE5 (Release-Engine-Staging -> Main) (v972-20964824)
[CL 21153400 by paul chipchase in ue5-main branch]
#rb Per.Larsson, Matt.Peters
#jira UE-153904 , UE-156201
#preflight 62ce802c471a2c2886f592e8
### Problem
- Because of the package trailer we can easily find out which payloads are present in a package file and where they are. This means we don't need to rely on the cached offset into the file that bulkdata generally uses for payload loading. This means as long as the payload is still actually in a payload file then we can load it no matter what.
- We will still give errors in the following scenarios
-- The payload is not stored in a package trailer (either referencing older bulkdata or an older editor bulkdata format)
-- We cannot get a read handle to the package file.
-- The payload is no longer in the package file.
- If the attachment is still valid then we can guarantee that the payload can be loaded. Without an attachment we make our best efforts but in the case of the package file being reomoved/replaced this can fail. So ::ResetLoader will still be used to indicate that we want to remove the attachment but expect the package file to be changed and to load the payload into memory for safety. Work on improving this is another work item for the future.
### Fix
- We no longer remove the package path when ::DetachFromDisk is called nor do we remove flags relating to the payload. From this point onwards we use the attached archive being null to determine how we load. We continue to set OffsetInFile to an invalid value as we can no longer safely use it.
- Add a new method ::IsAttachedToPackageFile to explicitly show when we are checking to see if the bulkdata is still 'attached' to the package file.
- Explicitly set AttachedAr to null ptr when ::DetachBulkData is called on it. In theory the ::DetachBulkData call will call FEditorBulkData::DetachFromDisk which in turn will set AttachedAr but it can look confusing when reading the code. Explicitly setting the AttachedAr member to nullptr helps make the intent clearer.
- When loading the payload from disk there is now a new code branch that is followed if we detect that we are loading from a package file that we are no longer attached to.
-- This path gives bespoke errors in cases where it is not safe for us to try loading the payload, in addition there is a bespoke error message if loading from the package trailer fails.
-- Note we can only risk loading from the package trailer as it's format allows for us to find if the payload is still on disk or not. With the older formats we will not know if we are loading garbage or not.
-- Fixed a bug with CVarShouldLoadFromTrailer. When it is true we will only attempt to load payloads from the trailer if the payload is stored in the trailer. The comment for this development CVar has been updated.
- There has been a pass made to error reporting. If we are reporting an error relating to the package we will output the package name, if we are reporting on a error relating to the file itself then we will output the package path + extension
[CL 21089705 by paul chipchase in ue5-main branch]
#rb pj.kack
#ROBOMERGE-AUTHOR: robert.millar
#ROBOMERGE-SOURCE: CL 21060094 via CL 21060128 via CL 21060415 via CL 21060535
#ROBOMERGE-BOT: UE5 (Release-Engine-Staging -> Main) (v972-20964824)
[CL 21078246 by robert millar in ue5-main branch]
#rb PJ.Kack
#rnx
#preflight 62cc18391961b487b3757b5e
- MSVC Static Analysis picked up that the Class pointer was still being used in the branch where we know Class == nullptr.
- When iterating over the pending CDOs and logging them, we should be logging the class name for that CDO.
- Added an assert to check that CurrentClass is valid before using it.
- Fixed some code alignment issues that occurred in a recent code merge.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 21053641 via CL 21053644 via CL 21053647
#ROBOMERGE-BOT: UE5 (Release-Engine-Staging -> Main) (v972-20964824)
[CL 21075787 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#rnx
#preflight 62c6a9faf85287873280cef7
EFlags::TransientFlags has been removed from the enum and instead is stored as a constant. This will allow the debugger to more easily display the flag member.
Move the removal of transient bulkdata flags when saving to ::BuildFlagsForSerialization so that all flag manipulation is done in the same place.
We now update the flags at the end of serialization and remove the EFlags::IsVirtualized bit (if set) if the payload is being stored in a package trailer
-- I originally tried to do this by removing the bit in ::BuildFlagsForSerialization, but that was partially reverted in CL 19976045 as it was causing the editor bulkdata is rehydrate as we check for EFlags::IsVirtualized in several places when saving out the bulkdata
-- Moving the bit removal to the end of serialization is the easiest and safest way to fix this. However with some more work we could probably remove more branches from this logic but I'd rather move that to a separate work item.
Moved the paranoid checks to make sure that the package trailer builder was created correctly to a static method ::ValidatePackageTrailerBuilder in order to make the ::Serialize method easier to read.
[CL 20982025 by paul chipchase in ue5-main branch]
Actual payload support will be implemented separately
#rb Paul.Chipchase
#preflight 62c367cf5751c961938bbd3f
#ushell-cherrypick of 20965910 by Francis.Hurteau
#ROBOMERGE-OWNER: francis.hurteau
#ROBOMERGE-AUTHOR: francis.hurteau
#ROBOMERGE-SOURCE: CL 20967184 via CL 20968121 via CL 20968405 via CL 20970220
#ROBOMERGE-BOT: UE5 (Release-Engine-Staging -> Main) (v972-20964824)
[CL 20973707 by robomerge in ue5-main branch]
#rb trivial
#rnx
#jira UE-158276
#preflight 62c415a0d94b57687c2c8c20
- The assert isn't really required since we reset the editor bulkdata before we change it to reference the older style bulkdata. So the only purpose that the assert serves is to force the caller to always call ::Reset anyway.
[CL 20940185 by paul chipchase in ue5-main branch]
#rb none
#rnx
#robomerge EngineMerge
#preflight 62bc6744fd062511870fdbcc
- Bulkdata2 made a distinction between allocating for the first time and re-allocating were as the original bulkdata system only called realloc even if we knew that we have no pre-existing allocation.
- Using the Bulkdata2 system in the editor was causing some code paths to crash as they were relying on being able to serialize a bulkdata instance repeatedly without resetting it.
- For now, I have changed the code to default to the old behavior so we only ever call reallocate.
-- A better fix in the future would be to ::Reset the bulkdata before a loading serialization call, but I will add a new JIRA ticket for that.
[CL 20889881 by paul chipchase in ue5-main branch]
#rb none
#rnx
#robomerge EngineMerge
#preflight
- Bulkdata2 has an error at runtime to catch the use of ::Realloc on bulkdata objects that represented a payload on disk, when this was merged with the original bulkdata code we should've removed the check from the editor.
- Note: That calling realloc on a bulkdata instance that still represents a payload on disk can have some weird effects as ::realloc will change the size, but not detach the bulkdata, so if we do try to later load it off disk, the size value will not match anymore. In the older code we would have a "SizeOnDisk" member that would still allow us to safely load the payload off disk, but then the payload and the size that we return will still be mismatched.
-- I will add a jira for further work on this, it is likely that calling ::Realloc will in future detach the bulkdata from disk to prevent potential problems in the future.
[CL 20872302 by paul chipchase in ue5-main branch]
#fyi andriy.tylychko
Original CL Desc
-----------------------------------------------------------------
[Backout] - CL20829812
Original CL Desc
-----------------------------------------------------------------
switched from TQueue<Mpsc> to TMpscQueue because it's a faster and modern version, and TSAN doesn't complain about it
#preflight 62b9a92dd0434894cac70823
#preflight 62bc1bbe30036d0db958fdf2
[CL 20871952 by andriy tylychko in ue5-main branch]
Original CL Desc
-----------------------------------------------------------------
switched from TQueue<Mpsc> to TMpscQueue because it's a faster and modern version, and TSAN doesn't complain about it
#preflight 62b9a92dd0434894cac70823
[CL 20832685 by andriy tylychko in ue5-main branch]
