#rb Per.Larsson
#jira UE-203597
- When we first parse the value from the config file we should check to see if it contains and path separators and if so reject the backend entirely. Providing a full path rather than just the filename is an easy enough mistake to make and it is better for the user to find out sooner than later.
- The utility function ::TryCreateSubmissionSessionDirectory was extended after it was first created to also create a dummy ignore file, updated the name to reflect that it does more than just create a directory.
- We were relying ont he creation of the dummy file to also create the directory but that means it is harder for us to inform the user if an encountered problem is due to the directory creation or the file creation. Now we ensure that the directory is created first (with error handling) and only then try to create the dummy file.
- Added logging of the system error if we encounter issues to give the user a better idea what the issue was.
[CL 30532386 by paul chipchase in ue5-main branch]
#rb Martin Ridgers
#jira UE-191791
#rnx
- Add a new ::IsProcessInteractive function to UE::Virtualization::Utils namespace that will return true if we think the current process is being run in a scenario where a user will be present and that it is okay to show an interactive error dialog.
- Use this new check to make sure that we do not try to display the "connection error" dialog on start up (even though as a slate dialog this will usually fail to display in non-interactive scenarios anyway)
- Use the new check to avoid displaying the "payload pull failure" dialog and default to the process shutting down instead.
-- Added some code comments to explain why we force the process to exit rather than logging a fatal error.
[CL 28183133 by paul chipchase in ue5-main branch]
#rb none
#jira UE-191791
#rnx
- When a connection attempt fails we now return the credentials used and the errors given by the system.
-- The credentials will be used to auto fill out the dialog, letting the user see what credentials were attempted in the last attempt.
-- The errors are shown in a box with red error text, letting them know what went wrong.
- We no longer let the revision control provider log errors when establishing a connection, so if the users correct info and successfully connects on a subsequent attempt they will not see an error in the logs.
-- If the connection completely fails we still log as before.
[CL 27302825 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-191791
#rnx
- Removed direct references to perforce from the dialog code, those strings are now passed in by the caller. This should make the dialog more usable by other revision control providers at some point.
- Make it more obvious that the dialog is related to virtualized assets.
- Add more references to the revision control provider name so that people realize which settings they are supposed to be entering.
- Removed some debug code for launching the dialog from a console command.
- Fixed a bug where clicking the button to reset the ini file settings and use the default connection would reset the settings but then not try to reconnect.
- Make better use of the dialog space by increasing the size of the input boxes at the expense of white space, so that longer server addresses can fit on screen.
[CL 26769098 by paul chipchase in ue5-main branch]
#rb none
### Dialog
- When the source control backend tries to connect and fails we will now show a dialog allowing the user to try new settings rather than let the editor continue and crash.
- The dialog will allow the user to enter the server address (P4PORT) and their username (P4USER).
- Once the dialog is displayed the user will have three options to continue.
-- "Reset To Defaults" which will remove all settings saved to the users local ini files and attempt to use the environment defaults to connect.
-- "Retry Connection" which will take the new address and username that the user provided and attempt to connect with those settings.
-- "Skip" no connection attempt will be made. This may cause instability later if the user needs data that they cannot access.
- If the connection succeeds after the user has reset to defaults or provided new settings, then they will be saved in the local ini file for future sessions.
- The dialog will not show if:
-- Slate is disabled for the current processor has not yet initialized
-- The error is found not on the game thread. This means that lazy init will not currently work well with this dialog. Attempts to mashal the error to the game thread have been spotty at best and there is a high chance of thread locking so attempts were abandoned.
-- If the -unattended flag is set on the process.
-- If the dialog is disabled via the config file when setting up the backend (UseRetryConnectionDialog=False)
-- If 'engine.ini:[Core.VirtualizationModule]:UseLocalIniFileSettings' is false as future sessions would not be able to use the new settings and would become tiresome. In these cases the environment settings will need to be fixed.
- If the dialog is now shown then the backend is marked as unconnected and will not work.
### Known Issues
- We currently cannot get the settings that the failed p4 connection used, nor the error message dueto how the API works. So for now we just display an empty server address and username edit box. This will be fixed when the API is changed.
- There is currently no good way to inform the user where their settings come from.
- There is no easy way to reset the settings once saved. To allow the user to change the server address to a better proxy for example.
### Initialization
- In order to allow a slate dialog to be used, the initialization of the VA system has been moved later in pre init, to the first avaliable point after slate has initialized and its shaders have been built. This in theory should not cause a a problem as engine content cannot be virtualized.
-- Added a new option 'engine.ini:[Core.ContentVirtualization]:InitPreSlate' with a default of false. When set to true the VA system will initialize in the original, earlier portion of engine pre init. This is only included in case an existing virtualized project encounters a problem with the new ordering.
[CL 26115392 by paul chipchase in ue5-main branch]
#rb none
#rnx
#preflight 645c95abaa3c584c0b1c91f0
- A url can now be provided via 'ini:Engine:[Core.VirtualizationModule]:ConnectionHelpUrl=""' that will be added to various connection errors. If the error message is via FMessageLog then the user will be able to click the url to go straight to the link.
- This sort of help will be very company specific, this url will allow teams to build up their own FAQ for their users.
- Currently used by the source control backend if the initial connection fails but will be expanded to other areas.
- The url is currently parsed and owned by the virtualization manager but at the moment the connection error is issued by each backend. So right now there is a static accessor allowing backends to get the value to use. This would not work properly for 3rd party backends and will be changed in the near term.
[CL 25424000 by paul chipchase in ue5-main branch]
#rb trivial
#jira none
#rnx
#preflight 6458e2596c35ad81e60f2fa9
- I thought I had already done this but I see nothing in the p4 history, so it is likely that I forgot to actually submit the change.
- Fix some additional warnings found under clang (C5219)
[CL 25370075 by paul chipchase in ue5-main branch]
#rb trivial
#jira none
#rnx
#preflight 6447d1954052d26a5a8c36cc
- Ends up pretty much the same,as FCompressedBuffer::FromCompressed will return a Null FCompressedBuffer if the provided data is null, but adding the check makes it clear when reading FSourceControlBackend::PullData that payloads might not be found and we are not setting them in that case.
[CL 25179230 by paul chipchase in ue5-main branch]
#rb none
#jira UE-181418
#rnx
#preflight 6447c5482804595a04d1e375
- Each pull operation now takes a FText that can be filled in with additional info about any failures that will cause the new error dialog to be displayed.
-- At the moment we only have info to provide from the source control backend if the connection itself failed.
-- In the future we might want to completely overhaul our error logging and display all messages to the user but our logging system is not really built for that level of redirection at the moment.
- Only pass on the error if it came from a persistent backend, we don't need to worry the user about cached backends which are allowed to fail.
[CL 25178752 by paul chipchase in ue5-main branch]
#rb none
#jira UE-182205
#preflight 643d2901c947f6523a2f5845
- Most backends support some form of existence check which is run before a push to prevent uploading data that is already there. Sometimes we want to ignore that check (bugs in the backend etc) and need a way to force that to happen.
- The new config option [Core.VirtualizationModule]:ForceCachingOnPull (default false) will do this when set to true.
- The file and DDC backends will respect the flag but for now the source control backend will continue to check to avoid accidental pointless revisions being added.
[CL 25066347 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-182205
#preflight 643932b2211b661dc4f594c6
- The enums don't contain any entries at the moment, the goal of this change is to reduce the friction that future API changes might have.
- The older versions of PullData/PushData have been deprecated but this won't flag compiler warnings if people have derived their own implementations of IVirtualizationBackend, so in addition to deprecating the methods have been set to 'final' and have checkNoEntry implementations which is the best we can do at the moment.
-- It is unlikely that people have actually done this as the documentation on adding new backend types will only go out in 5.3.
[CL 25039959 by paul chipchase in ue5-main branch]
#rb none
#jira UE-177956
#preflight
- Deprecate the config entry 'DepotRoot' and replace it with 'DepotPath'. The use of the term root was not clear and was causing people to think it was the name of the depot rather than the path to the root location to store payloads in.
- If 'DepotPath' is empty or missing, the value for 'ClientStream' will be used for it.
-- The 'DepotPath' can still be set when using streams, if the name of the stream is not a valid path (virtual streams for example.
[CL 25024791 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-180257
#preflight 6411cdbb5819afacaf06fbc9
- By default the value is true and the local ini file is used.
- Allows teams to opt out of loading settings from the locally saved ini file if they wish to.
[CL 24654053 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-170132
#rnx
#preflight 640f33c5363e9b40ab8cd7af
- Output the entire depot path when failing to download 'payload_metainfo' along with the address of the server.
- We ask the source control provider for the status and parse the p4 port from that as there is no way (currently) to get this info directly via the source control api, which currently works but is a bit flaky long term.
-- I am not sure if we want to keep the error reporting like this and so I am loath to make changes to the source control API unless I am sure we actually want the functionality.
- Removed duplicate code paths, there is now only one branch that will log an error.
- Removed call to FConnect as that does not do anything unless a client is set and we do not use one with this connection. (FConnect is mostly about client set up validation rather than actually connecting to the server since we always make a new perforce connection for each command anyway.
-- Instead we now check if the source control provider is available after we call Init, if this returns false then no connection was made.
- Documented the purpose behind calling ISourceControlProvider::Init
[CL 24613062 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-175158
#preflight 63f8cd34ae54ee4ce9ea207b
- We really should just read this value out of the users p4 environment settings, but that would require a source control api change and would need to be done in a source control agnostic way. Letting a project just define the name is a quick way to expose the functionality.
- At some point if we do extend the source control api then this option can be deprecated.
[CL 24400577 by paul chipchase in ue5-main branch]
#rb trivial
#jira UE-175157
#rnx
#preflight 63f8b493ae54ee4ce9de6b78
- This feature was requested so that a team could opt into submitting from their appdata or temp directories instead of the project saved directory (our default)
- Deprecated the option '-SubmitFromTempDir' as it could be replacated with "-WorkingDir=$(Temp)/UnrealEngine/VASubmission"
[CL 24399707 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-175189
#rnx
#preflight 63d27eae450d5cdd0b4df619
- We were raising a FMessageLog to print a warning, then using ::OnConnectionError to defer the FMessageLog::Notify call until the next editor tick so an easy fix is to pass the warning to ::OnConnectionError and piggyback onto the defferal if needed.
- The messages used to be errors but were demoted during development, now that FSourceControlBackend has SuppressNotifications option and can supress the notifications on a per project basis we can restore the messages to errors.
- When calling ::OnConnectionError we will send the message to FMessageLog if we are currently on the game thread, then defer the notification until next tick as we were doing so previously. The deferral is needed in case the notify comes during editor start up (which for a connection problem is likely) so that the notification appears at the correct time. If we are not on the game thread we should still print the message to the log file so that debugging is easier, only the FMessageLog part needs to be deferred to the GameThread. In this case we need to tell FMessageLog to not print to the log file to avoid duplicates.
[CL 23919185 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-174290
#rnx
#preflight 63c8fa22d15a37ae31487446
- Previously once the number of perforce connections allowed was reached we'd block all additional requests until an outstanding connection as been released at which point a random waiting thread would get the go ahead to make its own connection and so on. If the GameThread was making a request under these conditions the user could be left with a blocked editor while they wait for the GameThread to get a turn.
- This change allows FSourceControlBackend to create a semaphore that gives priority to the GameThread during contention so that the user gets the fastest possible response.
- FSemaphore now takes a bitfield of flags as a parameter in the constructor to allow us to specialize its behavior with the only support option being to prioritize the game thread.
- If the GameThread tries to acquire the semaphore and the prioritization option is enabled we use a slightly different path code path ::AcquireFromGameThread
-- Unlike the normal acquire code path, this path does not increment the count when waiting. This effectively reserves the next open slot for the calling thread.
-- Given that there is only one request on the GameThread at a time we don't have to check Counter once the event has been triggered. The even triggering means we know that one of the slots has been freed and we know we have already reserved that slot so we can just go ahead and return success at that point.
- Added a profile scope to acquire as that would've made the original problem easier to catch.
- Still need to look into replacing this with std::counting_semaphore once we can rely on c++ 20 support.
[CL 23771977 by paul chipchase in ue5-main branch]
Tested compiling fortnite, unrealeditor, lyra, qagame with non-unity/pch
#preflight 63635997876630122adeab9f
#rb none
[CL 22958990 by henrik karlsson in ue5-main branch]
#rb PJ.Kack
#jira UE-167578
#rnx
#preflight 635bdddd5b54febd38b7efd0
- Clean up the source control backend by removing the older code path that was ifdefed out and clean up the new path.
- FPerforceConnection no longer takes a TOptional<FSharedBuffer> but takes an optional pointer to a TArray of FSharedBuffer. When gathering files this should return one FSharedBuffer per file requested.
-- It might make more sense to attach the FSharedBuffer to the FP4Record but that would require some bigger changes and would be better suited to being some follow on work. All the code is private in the module and not exposed so it is fairly safe and easy to do these refactors later.
- Replaced the TArray buffer for gathering files with a purpose built class FP4DepotFile.
- When issuing a p4 print command with multiple files we will get one call to ClientUser::OutputStat followed by calls to either ClientUser::OutputText or ClientUser::OutputBinary until all of the file has been delivered, then we will get another call to ClientUser::OutputStat for the next file and so on.
-- Note with binary files we seem to get one final call to ClientUser::OutputBinary with zero size, which I assume is supposed to signal the end of the transfer, text files do not get this, they seem to just stop.
[CL 22888671 by paul chipchase in ue5-main branch]
#rb Per.Larsson
#jira UE-163093
#rnx
#preflight 63526abcae33b04ec1ee4a65
- The caller can now request more than one payload to be pulled froim the system at a time. In theory this will allow backends to pull data quicker.
-- The file system backend works with the new system but makes no attempt to take advantage as the backend is not intended for serious production use.
-- The DDC backend should be taking full advantage of the new batching API
-- Note that although the source control API does attempt to take advantage of batching, the internal source control API implementation is not doing so. This will be addressed in a future submit.
- This does make the pulling logic a bit more complicated in FVirtualizationManager as we need to deal with some payloads being found in the first backend, some in the second and some in the third etc.
-- To help deal with this a new class FPullRequestCollection has been added which abstracts a lot of complexity away.
[CL 22707955 by paul chipchase in ue5-main branch]
