#rb Juan.Legaz
#jira UE-202626
- Move info messages (warnings in this case) containing "- warning: file not mapped in stream" to be errors when submitting content."
- Cleaned up use of RemoveRedundantInfo to pass in the default parameter.
[CL 30317027 by paul chipchase in ue5-main branch]
#rb Juan.Legaz
#jira UE-175581
- The command will already be marked as a failure, but as p4 returns the message about the file(s) being ignored as INFO we will not print any error messages to the user and the failure can be confusing.
- Add ::RemoveRedundantInfo to allow us to remove info messages and move them to be errors. This is basically the same logic as ::RemoveRedundantErrors but going in the opposite direction.
- Check for the info message mentioning that a file failed to add as it is ignored (ignored file can't be added) and move that message to be an error.
-- The errors will be logged by the calling code so there is no need for us to do that here. In addition if the original calling code holds on to the operation pointer it will have access to the FSourceControlResultsInfo structure containing the error message if the calling code wishes to do it's own logging.
[CL 30262250 by paul chipchase in ue5-main branch]
#rb juan.legaz
#jira UE-175581
### Perforce Connection
- FP4ClientUser now takes a FSourceControlResultInfo structure to store the warning/error messages generated during a perforce command, in addition we can now store info messages.
-- In almost all cases the array that was being passed in to store the warning/error messages came from a FSourceControlResultInfo so it doesn't change the overall flow.
- Rather than override ClientUser::OutputInfo to get the info messages we have overridden FP4ClientUser::Message instead as this is the preferred way in the cpp p4 api to capturing messages.
-- One difference between ::Message and the older capturing methods is that the older methods seemed to result in a newline character at the end of the message, which we do not see with the newer capture method. As some of our result parsing code relies on this behavior FP4ClientUser::Message will append this newline character for now until the parsing code can be fixed.
- For now FP4ClientUser still overrides ::OutputInfo, ::OutputError and ::HandleError but only to add a checkNoEntry() to each one. This is so that we can confirm that there are no remaining uses of it before removing it entirely.
- All classes derived from FP4ClientUser have been updated to work with the changes. In some cases this meant changing the override of ClientUser::OutputInfo to ClientUser::Message.
-- With a little bit of work we could probably remove these derived classes now that FP4ClientUser supports capturing the info messages which would help simplify the code base however this should be done in a future work item.
### Misc
- Remove ::FPerforceSourceControlProvider::GetWorkspaceList rather than update it, as it is not used.
- Add FSourceControlResultInfo::HasErrors to check if the struct contains errors or not.
-- Did not replace existing use of FPerforceSourceControlProvider::ErrorMessages.IsEmpty() that should be done as it's own work item.
[CL 30208418 by paul chipchase in ue5-main branch]
#rb Luc.Eygasier
#rnx
- FSourceControlAssetDataCache can launch work on the taskgraph system in the method ::FetchAssetData which in turn calls ISourceControlRevision::Get.
- The perforce implementation of this uses FMessageLog to report errors but this can only be used safely on the GameThread. When used on the taskgraph system we run the risk of either triggering an unsafe delegate assert or we end up trying to update a slate widget (if the FMessageLog window is open) which will assert when done outside of the GameThread.
- The short term fix is to use FTSMessageLog which is a quick and dirty wrapper in the perforce module that acts like FMessageLog but when called on a taskgraph thread it will just log the message rather than trying to route it to the FMessageLog system.
-- There is a work item to make FMessageLog thread safe but due to some API choices this is non-trivial.
- Note that this change does not address the original errors, but should prevent the crash.
[CL 29707502 by paul chipchase in ue5-main branch]
#rb j.baumgartner
#rnx
- The safest way to determine the client spec root is to just initialize a p4 connection and extract the info from that.
[CL 29596693 by paul chipchase in ue5-main branch]
#rb wouter.burgers
#rnx
### SourceControl
- Remove link to "InputCore" as it is not used
- Only include slate and rendering modules if the target we are building for uses slate.
- Define SOURCE_CONTROL_WITH_SLATE as 0 if the target we are building for does not use slate.
- When SOURCE_CONTROL_WITH_SLATE is zero the following will be applied:
-- FRevisionControlStyleManager will not be compiled as it only makes sense if slate is enabled.
-- The monitoring aspect of FSourceControlFileStatusMonitor will be disabled as we cannot be notified of user interactions.
-- FScopedSourceControlProgress will compile but essentially do nothing.
-- The ::GetIcon method of ISourceControlState will no longer be compiled as it returns a FSlateIcon by value.
[CL 29516345 by paul chipchase in ue5-main branch]
Results with a 40K actors changelist editor startup time:
- Before: FPerforce::AddShelvedFilesToChangelist took 2:35 min
- After: FPerforce::AddShelvedFilesToChangelist took 160.08 ms
#rb sebastien.lussier
[FYI] jeansebastien.guay
[CL 29047110 by jeanfrancois dube in ue5-main branch]
#rb PJ.Kack
#rnx
- Bug was intorduced in CL 26952096
- EConnectionOptions::WorkspaceOptional means we should not test if a workspace is valid but if one is provided it should still be used but in the above change EnsureValidConnection was changed and this resulted in the code discarding the workspace entirely if that option was used.
- If that option is used we just pass the input workspace name (if there is one) to the output structure and trust that the caller knows what they are doing.
[CL 27953007 by paul chipchase in ue5-main branch]
#rb Per.Larsson
### ISourceControlProvider
- Add a new ISourceControlProvider::Init overload that returns a struct (FInitResult) and takes a bitfield of flags (EInitFlags) so that we can more easily extend the initialization in future and return things to the caller.
- If a provider does not implement the new overload we fall-back to the original. This will be removed once all providers support the new path.
### PerforceSourceControlProvider
- Support the new init options via FPerforceConnection::EnsureValidConnection and update places that call it.
### FPerforceConnection
- Update ::AutoDetectWorkspace to return errors to the caller rather than handle its own logging.
- ::TestLoginConnection and ::TestClientConnection no longer calls FPerforceSourceControlProvider::SetLastErrors, this will be done at a higher level.
- ::AutoDetectWorkspace still does its own logging which will be fixed in a future update, it does however return its errors to the caller.
- Split the implementation of ::EnsureValidConnection to a stand alone internal function, the original function now handles and consolidates error handling.
-- Changed the logic to early out of failure rather than constantly checking a boolean all the way down.
-- Tried to remove the need for FPerforceSourceControlProvider to be passed in but a few places still require it.
-- Fixed our handling of the P4PORT value so that we can at least report problems if the address uses unicode characters. I have not yet tested if we can actually connect to such an address. We no longer use the servers unicode setting to decide the port strings encoding as it is not affected by the server settings.
-- Each error encountered should now record a localized error that we expect the user to be able to react to, then an array of errors from the API itself.
-- Once the internal code has run we will do a pass on all errors (if any) and strip any whitespace at the end (p4 tends to end error messages with newline characters) and then add a final entry to the additional errors detailing the settings at the point of failure. This was mostly being done before but is now enforced for every error.
-- No longer log the ticket, although the workflow to allow the user to supply one is not common we still probably don't want to save that to log files.
[CL 26952104 by paul chipchase in ue5-main branch]
Rewrite loop to be clear that the primary cursor is over changelists and the secondary cursor is over files.
#rb juan.legaz
[CL 26937944 by robert millar in ue5-main branch]
Speeds up query from 20+s to around 1.5s in an example workspace with 54 changelists.
#rb brooke.hubert
[CL 26294455 by robert millar in ue5-main branch]
#rb Wouter.Burgers
#rnx
### Bug Fix
- In CL 24941987 FPerforceGetFileWorker::Execute was modified to remove the perforce connection being created as that connection was not used and FPerforceSourceControlRevision::Get would create a second connection to be used.
- Removing this connection has an unintended knock on effect in that the command passed to the worker was never marked as having a successful server connection. So next tick the source control provider would check the now completed command, see that the connection was not a success (even though the command returned a success) and mark the connection to the server as broken, preventing future commands from working.
- Over the last year or so we have tried to move the code away from these sort of non-obvious side effects but there is a lot of work to do still.
- For now it is safest to add the connection back to FPerforceGetFileWorker::Execute but also add an overload to FPerforceSourceControlRevision::Get that allows us to pass in the connection to be used, to avoid the original problem of creating two connections as connection creation is super slow and can be costly on the server.
### Additional Changes
- There was no reason to allocate the FPerforceSourceControlRevision in FPerforceGetFileWorker::Execute on the heap as it is used immediately then deleted. We might as well just create it on the stack.
- Changed some code logic to early out rather than having long conditional scopes.
- The input parameters for FGetFile are still a bit odd (no revision == #0, revision -1 == #head etc) but leaving that alone for now until we merge FGetFile and FDownloadFiles to eliminate the duplicate code functionality.
[CL 25867524 by paul chipchase in ue5-main branch]
* Especially slow when large numbers of files are checked out (in the 25k+ range)
* Existing algorithm scales cubically
* Optimization: Changed to take advantage of the fact that filenames are already in a map, so a cheaper lookup to check if file in changelist is part of the to-be-deleted set
* Benchmark for this change is a map with ~32k empty actors in it. Source control enabled and using world partition (and thus One File Per Actor). It is important to check out all __ExternalActors__ files into default changelist and have the "View changes" window open.
- Delete N actors
- Save the level and measure time
Before:
100 actors : ~20s
200 actors : ~1m 20s
300 actors : ~2m 40s
500 actors : ~7m 20s
After:
100 actors : ~6s
200 actors : ~14s
500 actors : ~25s
There is plenty of room for more optimization here.
#jira UE-187093
#rb Brooke.Hubert
#preflight none
[CL 25652227 by logan buchy in ue5-main branch]
Added GetArrayHash() for hashing arrays by their contents.
Added a new pair of keyfuncs for TSets and TMaps which allow TCHAR* keys (or TCHAR* values passed to *ByHash functions) to be hashed by FCrc::Strihash_DEPRECATED().
#rb devin.doucette
#preflight 64708e7c296b2b37c6fd894a
[CL 25647216 by steve robb in ue5-main branch]
