#rb trivial
#rnx
#preflight 62fcc14d1e39eb26a0851cd0
- Sometimes it is useful to be able to dump the payload info from packages not under the current project as a quick form of debugging and given the format of the package trailer there is no reason why we should allow it.
- If we fail to turn the arg into a valid FPackagePath can now also check to see if the arg is an absolute file path via the FileManager
- If both of these options fail we then log an error as before.
[CL 21422989 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-160954
#rnx
#preflight 62fb9de9086f90bbc4394829
- The problem with the old logic was that we checked all of the payloads initially to see which needed to be submitted but if enough payloads were requested we would split the submission into a number of batches. If a payload was in more than once batch then it would be submitted in the first batch and then fail the MarkForAdd command in the subsequent batches. When we detected this error we would fail the virtualization process.
- In the long term we should probably eliminate duplicate requests at the highest level (FVirtualizationManager) so no backend has to worry about it, but we need this working now so it was easiest to add duplicate checks to the source control backend
-- Note that if another process submits a payload between the status check and the mark for add calls then we will also get errors, but that is true for a lot of our source control code.
- Now when pushing payloads we first iterate over the requests and create a unique list of requests to submit. This eliminates the possibility of duplicate payloads being submitted and failing the MarkForAdd command.
- We now record the push request state for each payload in a map and then only apply it to the original requests if the push succeeds.
-- We still only really have failed/success statuses and nothing recording "already in the backend" which is scheduled to be fixed elsewhere in the backlog.
#ROBOMERGE-AUTHOR: paul.chipchase
#ROBOMERGE-SOURCE: CL 21404920 via CL 21408168 via CL 21408189 via CL 21408222
#ROBOMERGE-BOT: UE5 (Release-Engine-Staging -> Main) (v975-21357124)
[CL 21411940 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-160943, UE-151671
#rnx
#preflight 62fa31bbae3edb54c979e53e
- By checking the state of the payloads first we can avoid loading and writing a payload to disk, just to find that it is already in the depot. If we are very lucky we might find that all of the payloads are already uploaded, in which case we can avoid creating the submission workspace as well.
- The lock to prevent too many concurrent p4 connections does cover a wider area of code than it did before, in theory we could relinquish the lock while creating the payload files for submission, but generally the time it takes to do that is much smaller than the source control operations themselves so there is probably not much point.
- Any request that we find is already uploaded we can just mark as a success. We then create an array of pointers to the requests that were not found and operate on that for the rest of submission.
- Note that we also split up the payload existence checks into batches with the same max size limit of submission, although checking if a payload exists should be much less taxing on the system I'd rather not risk overloading the perforce server where possible.
#robomerge FNMain
[CL 21386511 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-160943, UE-151671
#rnx
#preflight 62fa3334153b17e7462954b3
### Problem
- The virtualization process first checks to see if any of the local payloads are already stored in persistent storage and only try to submit those which aren't, which would mean if we are submitting a package with a local payload that is already in the persistent storage system but not in cached storage we have no way to cache it.
- This logic also introduced problems where filtered out payloads were being virtualized which was fixed with the addition of ENABLE_FILTERING_HACK which was not a robust long term solution.
- Checking if the payloads need to be pushed or not complicated the logic of the virtualization process and makes it harder to understand and make fixes too.
### Fix
- Removed code for ENABLE_FILTERING_HACK and UE_PRECHECK_PAYLOAD_STATUS from the virtualization process
- Removed some code loops from the virtualization process but we still end up with one loop finding all of the package trailers and a second loop setting up the FWorkspaceDomainPayloadProvider
-- This second loop will be removed in a future work item to avoid trying to push duplicate payloads (unless I end up doing that work at the virtualization manager level instead)
- We make one push for cached storage and another for persistent
-- Cached storage push failure will only result in a warning, failing the persistent push will error out as before
-- We need to reset the request states after the cache push. In a future work item we will likely allow a single push to specific both storage solutions, so I am leaving this code using the same set of requests for both pushes.
#robomerge FNMain
[CL 21386503 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-160083
#rnx
#preflight 62f64714185b21882a875427
- When this backend was written, it shared the source control provider interface with the rest of the editor, so if we were unable to restore the workspace it would likely break the editor in interesting and hard to diagnose ways.
- Given the scope for problems and the low change of the operatation I opted to make the error fatal.
- Now that the backend owned it's own source control provider interface, the worst thing that can happen is that future virtualization attempts will also fail, but the user will not be left with a non functioning editor. With this in mind there is no longer a reason to consider this a fatal failure.
[CL 21353746 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-156189
#rnx
#preflight 62f5f1a01e61d1ba0e6c893b
- Note that the older name 'SourceControl' will continue to work but will log a warning if used to notify the user to update their config files.
[CL 21352223 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-156189
#rnx
#preflight
- The constructor parameters should've been const, this doesn't show when compiling with MSVC as we have that warning currently disabled.
[CL 21335846 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-156189
#rnx
#preflight 62f4f504b66d5d9313969a92
- Use the new macro UE_REGISTER_VIRTUALIZATION_BACKEND_FACTORY_LEGACY when trying to rename how backends are identified.
[CL 21334721 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-156189
#rnx
#preflight 62f4c098e60c9215b9bcde08
- The orignal name implied tha the payloads would only be stored locally on the users machine, but the intent is to describe a number of backends that are a) faster than the persistent storage backends b) no guarantee that the payload will be found.
- The new name better describes this functionality.
- We still accept 'LocalStorageHierarchy' but will log a warning informing the user to update their graph but this backwards compatibility code will most likely be removed before 5.1 ships.
-- EStorageType::Local will follow the usual deprecation rules.
- Not strictly related to this change but I also cleaned up and added additional loggng during the initialization of the virtualization manager.
[CL 21333601 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-156189
#rnx
#preflight 62f3ab2023003b62a324f67a
- The older versions of this value ([Core.ContentVirtualization].EnablePushToBackend and [Core.VirtualizationModule].EnablePushToBackend) will continue to work but will log a warning to the user telling them to update it.
- It is very unlikely that anyone is actually using these old values and so we can remove this backwards compatibility when 5.1 ships. It is only present to allow us to fix up internal projects and to avoid interrupting people testing the system.
[CL 21317383 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#rnx
#jira UE-160619
#preflight 62f37c86b66d5d93133d67e3
- Based on work from Jess.Kube
### Virtualization
- The source control backend now takes an optional config option "ClientStream" which takes the name of the client stream to use.
- If a client stream is set then workspaces created for payload submission will use that and not provide a client-view mapping.
#ushell-cherrypick of 21195584 by Jess.Kube
### PerforceSourceControl
- Allow FCreateWorkspace to create workspaces with streams as well as classic workspaces cia FCreateWorkspace::SetStream.
- Add a method FCreateWorkspace::ClearClientViewMappings which will clear any client view mappings already added to the operation.
- If we detect that a FCreateWorkspace operation has both a stream set and client view mappings set then FPerforceCreateWorkspaceWorker will return an error. Perforce will allow us to creat a client spec with both entries, but will default to using the stream. Technically we could allow this too but it might cause unexpected behaviour to the caller. It is better to give a clear error and fail the workspace creation.
[CL 21316756 by paul chipchase in ue5-main branch]
#fyi paul.chipchase
Original CL Desc
-----------------------------------------------------------------
The virtualization manager will now read the newer config file location (Core.ContentVirtualization) first and only fall back to the older location (Core.ContentVirtualization) if it was not found.
#rb trivial
#jira UE-156189
#rnx
#preflight 62f38fed086f90bbc4d915f0
[CL 21316410 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-156189
#rnx
#preflight 62f38fed086f90bbc4d915f0
- This will help if someone has updated their set up to the new location but not removed the old entries.
- We still warn that the old enteries exist, even if not used.
[CL 21316332 by paul chipchase in ue5-main branch]
#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 none
#rnx
#jira UE-159597
#preflight
- The option is [Core.VirtualizationModule]VirtualizationProcessTag=
- If left blank then no tag will be added
- The internal virtualization process code no longer needs to return a tag since it will now be applied by the virtualization manager instead.
-- Note that we still follow the same logic, we only stamp if no errors were encountered.
[CL 21217286 by paul chipchase in ue5-main branch]
#rb trivial
#rnx
#preflight 62d96169110da0f944b69f4f
- The bucket entry was already marked as optional in the code documentation but was giving a fatal error if it was not set. The value is now optional and will default to 'BulkData' if not set.
- We no longer consider both the local and remote transfer policy being disabled as a fatal error, when the work UE-143676 is done this can be improved further.
[CL 21200968 by paul chipchase in ue5-main branch]
#rb none
#jira UE-159596
#rnx
#preflight 62d94160110da0f944afdd32
### Problem
- When rolling our virtualization to projects we have run into a number of set up problems which has caused users to fail to submit packages from the editor as when the virtualization failed we prevented the overall submit from going through.
- Although we will eventually work through all of these problems and add better fault tolerance for now it is annoying for the content team so we need an easy way for a project to allow package submits to be completed even if the virtualization process part of the submit failed.
- Worth noting that the virtualization process failing should never affect the state of the package files, the only downside is that the packages may not be virtualized.
### Fix
- The virtualization/rehydration of packages now return an enum instead of a bool although at the moment we are only returning Success and Failed, but it will allow us to expand in the future
- Combined the code calling the virtualization process from both the source control window and the source control changelist window into a single utility function.
-- Now the two windows only diverge on how they report failure to the user.
- This new utility function now checks to see if the virtualization manager suggests if failing the virtualization process should block any package submission to source control or not.
- I am not happy with this solution, but I haven't come up with anything that feels right, this solution will at least work.
[CL 21199597 by paul chipchase in ue5-main branch]
#rb trivial
#rnx
#preflight 62d92875185da2495f3a8d09
- The missing default values were taken from BaseEngine.ini.
- Made it a bit clearer in the header, which members are set from the config files and which are not.
[CL 21199008 by paul chipchase in ue5-main branch]
#rb trivial
#rnx
#jira UE-159992
#preflight 62d7c9eb110da0f944670ecb
- When the depot path is parsed from the config file we now add the trailing '/' if it is not already there, this allows us to make the input more forgiving without changing the existing code.
- Removed FString::Printf when creating the path for payload_metainfo in favour of using WriteToString, this doesn't add much but gets rid of the %s.
- Reformatted some messages that had odd tab alignment.
[CL 21176124 by paul chipchase in ue5-main branch]
#rb trivial
#rnx
#preflight 62d5509047779a730a1e4602
- The code in the files isn't really a check anymore but the actual process for virtualizing packages (taking the payloads stored in the package trailer, pushing to the backends then modifying the trailer) updating the name reflects this.
- It also helps match the PackageRehydrationProcess.
[CL 21147691 by paul chipchase in ue5-main branch]
#rb trivial
#rnx
#preflight 62d15f427e1c26dcd715699c
- Now that the IVirtualizationSystem interface supports both virtualization and rehydration of package files, it seems pointless to keep a delegate that occurs on pre-submit, rather than just calling the system we really want to call directly.
- Deprecating the public methods accessing the delegate from ISourceControlModule should prevent licensees from trying to use it. The delegate was added in 5.0 so even though it is unlikely to have been used by anyone we need to go through the deprecation steps to remove it.
-- This leaves the calling code a little more complicated than I'd like, but cannot be avoided.
[CL 21109464 by paul chipchase in ue5-main branch]
