#rb trivial
#rnx
#preflight 626f823667326112684d0c24
- Because the commands do not return data in tagged format it was being written directly to stdout, this would cause odd log messages stating that a workspace had been create/deleted that would be displayed outside of our normal logging format.
[CL 20006887 by paul chipchase in ue5-main branch]
#rb martin.ridgers, wouter.burgers
#rnx
#preflight 626be80153253f874bcbee3c
- Some commands do not return tagged output even when we set that as our preference and will print the output to stdout as we are not currently collecting the standard output messages.
-- We could collect this output and return as info to the caller but that would involve adding more parameters which I am loath to do at the moment. At some point I plan to replace all of the FPerforceConnection::Run input and output parameters with single data structures and unify the different connection types at which point adding support for returning all info output would make sense.
- To supress this output we would normally pass -q as a global-opt for the command, which in the case of the cpp API requires us to call ::SetQuiet on the client users but for that we need to be able to pass in the request to FPerforceConnection::Run.
-- Note that calling ::SetQuiet is pretty much the same as overriding ClientUser::OuputInfo
- At this point we had two bool parameters in ::Run already, so when adding a third we might as well convert this to a bitfield of flags.
- It seems that the former parameter bInAllowRetry was never hooked to anything since the last large source conde API refactor near a decade a go, so that could just be removed.
- I felt that the former parameter 'bInStandardDebugOutput' was not clearly named, in the end if it was true we would log the full p4 command (and timing of the command in VeryVerbose mode) and in almost all cases this was set to true. So I flipped the flag so it is not set to opt out of logging a command.
- I removed one overload of FPerforceConnection::Run by setting the flags to have a default value. This should help discourage anyone from adding more parameters to the method.
[CL 20006796 by paul chipchase in ue5-main branch]
#rb trivial
#rnx
#preflight 6256abb45f20a0a34d9d62e9
- This way of invoking an operation was missing a call to FPerforceSourceControlCommand::ReturnResults which will pass the info/error messages from the command to the operation that the caller passed into the provider in the first place.
- This will allow the caller to gain access to any messages generated during the operation.
[CL 19737642 by paul chipchase in ue5-main branch]
#jira UE-145489 - Perforce source control doesn't update a specific set of P4 change list states
#rb Paul.Chipchase
#preflight 62335a876666d7e7539c378f
[CL 19422016 by Patrick Laflamme in ue5-main branch]
#rb Per.Larsson
#rnx
#preflight 6232edad736af8e082347531
- When executing FCheckOut, passing in a valid ISourceControlChangelist to ISourceControlProvider::Execute will cause the checked out files to be added to the given changelist.
- FDelete now both have the same functionality, based on the FCheckOut code.
- Note that we follow the existing convention that a changelist provided on the command line always overrides a changelist passed into ::Execute
- This code exists in some form or other in most of the workers, we probably should move it to a single function somewhere to avoid potential divergence.
[CL 19418324 by paul chipchase in ue5-main branch]
#rb Sebastian.Nordgren
#rnx
#preflight 62309ccae65a7e65d686d1dc
#robomerge FNNC
- The behaviour for the settings for the default provider (that takes it's settings from the command line) should be OverrideExisting not OverrideAll to maintain the original behaviour. This was a mistake due to misreading the original code. As it was, setting one setting via the command line would clear any previously set or saved settings.
- When parsing the cmdline we used the form "KeyName=" as the key, as FParse::Value does not strip the = automatically, but when we were looking up the values to override settings we were just using "KeyName" which would not match. The lambda ::ParseCmdLineSetting now adds = to any key passed in when parsing, so there is no longer a need for the caller to add it. This helps the keys match and work correctly.
[CL 19386795 by paul chipchase in ue5-main branch]
#rb Patrick.Laflamme
#jira UE-145721
#rnx
#preflight 622f0358752729a2ad448e16
- The problem we have is if a p4 depot path is used when requesting filestates (to see if the file exists in p4 for example) it will be treated as a networkpath due to it starting with // so calling the filesystem to check if it is a directory or not can get very expensive.
- Other operations just check the end of the path for '/' or '\' which is what this operation used to do, but that was changed at some point to check the file system, presumably to fix another problem and as we don't really know how 3rd party code is using this system we cannot just change the behavior without risking breaking existing code bases.
- This fix allows the caller to opt in to the old behavior of just checking the end of the path to determine if it is a directory or not.
- This fix is not very good, future work for a better fix will be tracked in the attached jira.
[CL 19371141 by paul chipchase in ue5-main branch]
#rb Luc.Eygasier, Sebastien.Lussier
#rnx
#preflight 622895d331133a23da78a4ca
### Misc
- FPerforceSourceControlRevision, FPerforceConnection and FPerforceSourceControlLabel now own a reference to their FPerforceSourceControlProvider and accesses the connection settings from that rather than via the module singleton.
- FPerforceSourceControlCommand now accesses the connection settings by finding the FPerforceSourceControlProvider that the given IPerforceSourceControlWorker references.
- FPerforceSourceControlProvider now owns it's own FPerforceSourceControlSettings rather than accessing a shared one owned by the module.
- Marked a number of derived classes as final.
### FPerforceConnection
- ::EnsureValidConnection can now work without a valid client (although most commands will require a client to run, things like p4 print does not).
-- Due to this change we might not catch cases where the the system cannot connect to source control as the user is not logged in as this would get caught when we checked the workspace by calling ::TestConnection.
-- To get around this ::EnsureValidConnection will now call "p4 login -s" via the newly added TestLoginConnection.
- Renamed ::TestConnection to ::TestClientConnection as we are really testing the connection to the specified workspace.
- There does seem to be some inconsistency in the logging, depending how a perforce source control provider first creates its connection as in some cases FPerforceConnection::EnsureValidConnection gets called first and in other cases FPerforceConnection::EstablishConnection is called. We should unify these paths but this change list is already so large I am opting to postpone this to it's own work item.
### FPerforceSourceControlModule
- Removed all of the singleton style accessors so that classes inside of the module can no longer assume that there is only one FPerforceSourceControlProvider. Updated the documentation to match.
- Since there is no reason for classes in the module to need to access FPerforceSourceControlModule, ::Get has been removed
- The responsibility to register all of the worker creation delegates has been moved to IPerforceSourceControlWorker so that all source control providers can share them.
- ::GetProvider has been removed, if a class inside the module needs access to a provider then a reference to it's provider will be given to it at construction time.
- ::SetLastError, ::AccessSettings and ::SaveSettings are now called directly on FPerforceSourceControlSettings
### FPerforceSourceControlSettings
- This class now takes the name of the system that owns it as a parameter in the constructor
-- The name will be used to identify where to store the settings in the config file, unless the name is blank in which case the pre-existing default location will be used.
- This will allow for multiple source control providers to store their connection info.
### SPerforceSourceControlSettings
- Moved FGetWorkspaces from SPerforceSourceControlSettings.h to its own header file to reduce the number of places that need to include the slate headers.
- Move the fwd declares to the top of the header base don recent coding standards discussions.
- Instead of accessing the source control provider via FPerforceSourceControlModule the dialog is now given a pointer to one when constructed.
-- This pointer is assumed to always be valid and should be a reference if the slate system supported it.
### PerforceSourceControlOperations
- IPerforceSourceControlWorker now takes a reference to the owning FPerforceSourceControlProvider in it's constructor. All derived workers have been updated to this.
- Moved the registering of worker creation delegates from the module cpp to a new static method IPerforceSourceControlWorker::RegisterWorkers
-- This should make it easier to find when adding a new worker and allows source control providers to share the same map of delegates rather than duplicating it.
-- This means we also need a new static method IPerforceSourceControlWorker::CreateWorker for instantiating new workers based on the given name.
### FSourceControlInitSettings
- This is a new class that is used to set up a newly created source control provider.
- The settings are expected to be given in the same way you would override settings via the cmdline. The existing perforce cmdline code has been updated to use this class.
- This still has the downside that the calling code needs to know the type of source control provider, but I don't really see a good way around this unless we refactor the entire API to be feature based.
[CL 19318801 by paul chipchase in ue5-main branch]
#Tests: PIE, switch between 2 branches with same config file, ensure that P4Config is correctly set
[REVIEW] Patrick.Laflamme
[FYI] Brooke.Hubert, Guillaume.Morreel
#ROBOMERGE-AUTHOR: charles.lefebvre
#ROBOMERGE-SOURCE: CL 19079616 via CL 19082287 via CL 19088908 via CL 19089025 via CL 19090086
#ROBOMERGE-BOT: UE5 (Release-Engine-Staging -> Main) (v921-19075845)
[CL 19131863 by charles lefebvre in ue5-main branch]
#rb scott.nelson
[FYI] luc.eygasier
#preflight 62055a9369224cb0d87518ca
#ROBOMERGE-OWNER: richard.malo
#ROBOMERGE-AUTHOR: richard.malo
#ROBOMERGE-SOURCE: CL 18943053 via CL 18944261 via CL 18944332 via CL 18944374 via CL 18946678 via CL 18946778
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v917-18934589)
[CL 18946785 by richard malo in ue5-main branch]
* Introduces a new 'GetFileList' operation
* Adds "keep checked out" option to other operations
#rb Luc.Eygasier
#preflight 620598920c64e1822f4849a8
[CL 18946159 by Mike Beach in ue5-main branch]
Fixes Reverting mark for add asset preventing to recreate an asset with the same name in the same editor session (Asset was still present in AssetRegistry).
#jira UE-140995, UEENGQA-62727
#rb Sebastien.Lussier
#tb Tim.Gautier
#preflight 62056230b84973a2bb61b450
#ROBOMERGE-OWNER: luc.eygasier
#ROBOMERGE-AUTHOR: luc.eygasier
#ROBOMERGE-SOURCE: CL 18943038 in //UE5/Release-5.0/... via CL 18943294 via CL 18943483
#ROBOMERGE-BOT: UE5 (Release-Engine-Test -> Main) (v917-18934589)
#ROBOMERGE-CONFLICT from-shelf
[CL 18943896 by luc eygasier in ue5-main branch]
