Skip to content

Commit 40cde77

Browse files
authored
Fixes the bug causing asset browser to be unable to delete/move/etc. (o3de#17568)
* Fixes o3de#15671 - [Bug Report] Asset Browser cannot delete asset * Adds missing functionality from the Linux ProcessWatcher to indicate the difference between a different problem launching program, and the program not being found. * Fixes the workflow of the Move/Delete/Rename operations in the Asset Browser to properly return the appropriate error when Source Control is not properly configured. * Adds a toggle for source control notification in AP, so that if you toggle the Editor to not use Source Control, the AP will not either. * Uses the same setting in AP as the Editor, to determine whether Source Control should be used or not, and sets it to false by default, just like the Editor. Signed-off-by: Nicholas Lawson <[email protected]>
1 parent f45ceb5 commit 40cde77

File tree

18 files changed

+440
-108
lines changed

18 files changed

+440
-108
lines changed

Code/Editor/MainStatusBar.cpp

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212
#include "MainStatusBar.h"
1313

1414
#include <AzCore/Utils/Utils.h>
15+
#include <AzFramework/Asset/AssetSystemBus.h>
16+
1517
// AzQtComponents
1618
#include <AzQtComponents/Components/Widgets/CheckBox.h>
1719
#include <AzQtComponents/Components/Style.h>
@@ -305,8 +307,21 @@ void SourceControlItem::UpdateAndShowMenu()
305307

306308
void SourceControlItem::ConnectivityStateChanged(const AzToolsFramework::SourceControlState state)
307309
{
310+
AzToolsFramework::SourceControlState oldState = m_SourceControlState;
308311
m_SourceControlState = state;
309312
UpdateMenuItems();
313+
314+
if (oldState != m_SourceControlState)
315+
{
316+
// if the user has turned the system on or off, signal the asset processor
317+
// we know the user has turned the system off if the old state was disabled
318+
// or if the new state is disabled (when the state changed)
319+
if ((oldState == AzToolsFramework::SourceControlState::Disabled) || (m_SourceControlState == AzToolsFramework::SourceControlState::Disabled))
320+
{
321+
bool enabled = m_SourceControlState != AzToolsFramework::SourceControlState::Disabled;
322+
AzFramework::AssetSystemRequestBus::Broadcast(&AzFramework::AssetSystem::AssetSystemRequests::UpdateSourceControlStatus, enabled);
323+
}
324+
}
310325
}
311326

312327
void SourceControlItem::InitMenu()
@@ -342,7 +357,19 @@ void SourceControlItem::InitMenu()
342357

343358
connect(m_settingsAction, &QAction::triggered, this, [&]()
344359
{
345-
GetIEditor()->GetSourceControl()->ShowSettings();
360+
// LEGACY note - GetIEditor and GetSourceControl are both legacy functions
361+
// but are currently the only way to actually show the source control settings.
362+
// They come from an editor plugin which should be moved to a gem eventually instead.
363+
// For now, the actual function of the p4 plugin (so for example, checking file status, checking out files, etc)
364+
// lives in AzToolsFramework, and is always available,
365+
// but the settings dialog lives in the plugin, which is not always available, on all platforms, and thus
366+
// this pointer could be null despite P4 still functioning. (For example, it can function via command line on linux,
367+
// and its settings can come from the P4 ENV or P4 Client env, without having to show the GUI). Therefore
368+
// it is still valid to have m_sourceControlAvailable be true yet GetIEditor()->GetSourceControl() be null.
369+
if (auto sourceControl = GetIEditor()->GetSourceControl())
370+
{
371+
sourceControl->ShowSettings();
372+
}
346373
});
347374

348375
connect(m_checkBox, &QCheckBox::stateChanged, this, [this](int state) {SetSourceControlEnabledState(state); });
@@ -367,6 +394,7 @@ void SourceControlItem::UpdateMenuItems()
367394
QString toolTip;
368395
bool disabled = false;
369396
bool errorIcon = false;
397+
bool invalidConfig = false;
370398

371399
switch (m_SourceControlState)
372400
{
@@ -377,6 +405,7 @@ void SourceControlItem::UpdateMenuItems()
377405
break;
378406
case AzToolsFramework::SourceControlState::ConfigurationInvalid:
379407
errorIcon = true;
408+
invalidConfig = true;
380409
toolTip = tr("Perforce configuration invalid");
381410
break;
382411
case AzToolsFramework::SourceControlState::Active:
@@ -386,7 +415,7 @@ void SourceControlItem::UpdateMenuItems()
386415

387416
m_settingsAction->setEnabled(!disabled);
388417
m_checkBox->setChecked(!disabled);
389-
m_checkBox->setText(disabled ? tr("Status: Offline") : tr("Status: Online"));
418+
m_checkBox->setText(disabled ? tr("Status: Offline") : invalidConfig ? tr("Status: Invalid Configuration - check the console log") : tr("Status: Online"));
390419
SetIcon(errorIcon ? disabled ? m_scIconDisabled : m_scIconWarning : m_scIconOk);
391420
SetToolTip(toolTip);
392421
}

Code/Editor/Plugins/PerforcePlugin/Platform/Linux/PAL_linux.cmake

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,4 +6,4 @@
66
#
77
#
88

9-
set(PAL_TRAIT_BUILD_P4PLUGIN_SUPPORTED FALSE)
9+
set(PAL_TRAIT_BUILD_P4PLUGIN_SUPPORTED TRUE)

Code/Framework/AzFramework/AzFramework/Asset/AssetProcessorMessages.cpp

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -621,6 +621,26 @@ namespace AzFramework
621621

622622
//---------------------------------------------------------------------
623623

624+
unsigned int UpdateSourceControlStatusRequest::GetMessageType() const
625+
{
626+
return MessageType;
627+
}
628+
629+
void UpdateSourceControlStatusRequest::Reflect(AZ::ReflectContext* context)
630+
{
631+
auto serialize = azrtti_cast<AZ::SerializeContext*>(context);
632+
if (serialize)
633+
{
634+
serialize->Class<UpdateSourceControlStatusRequest>()
635+
->Version(1)
636+
->Field("SourceControlEnabled", &UpdateSourceControlStatusRequest::m_sourceControlEnabled);
637+
}
638+
}
639+
640+
//---------------------------------------------------------------------
641+
642+
//---------------------------------------------------------------------
643+
624644
unsigned int ShowAssetInAssetProcessorRequest::GetMessageType() const
625645
{
626646
return MessageType;
@@ -1543,8 +1563,9 @@ namespace AzFramework
15431563
}
15441564
}
15451565

1546-
AssetChangeReportResponse::AssetChangeReportResponse(AZStd::vector<AZStd::string> lines)
1566+
AssetChangeReportResponse::AssetChangeReportResponse(AZStd::vector<AZStd::string> lines, bool success)
15471567
: m_lines(lines)
1568+
, m_success(success)
15481569
{
15491570
}
15501571

@@ -1559,8 +1580,9 @@ namespace AzFramework
15591580
if (serialize)
15601581
{
15611582
serialize->Class<AssetChangeReportResponse, BaseAssetProcessorMessage>()
1562-
->Version(1)
1563-
->Field("Report", &AssetChangeReportResponse::m_lines);
1583+
->Version(2)
1584+
->Field("Report", &AssetChangeReportResponse::m_lines)
1585+
->Field("Success", &AssetChangeReportResponse::m_success);
15641586
}
15651587
}
15661588

Code/Framework/AzFramework/AzFramework/Asset/AssetProcessorMessages.h

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,6 +573,23 @@ namespace AzFramework
573573
unsigned int GetMessageType() const override;
574574
};
575575

576+
//! Sent from any tool to the AP, notifying it to toggle the state of source control to either on or off.
577+
//! This avoids the need for AP to restart.
578+
class UpdateSourceControlStatusRequest
579+
: public BaseAssetProcessorMessage
580+
{
581+
public:
582+
AZ_CLASS_ALLOCATOR(UpdateSourceControlStatusRequest, AZ::OSAllocator);
583+
AZ_RTTI(UpdateSourceControlStatusRequest, "{B313400A-3E5D-496F-BD91-09B9C10EBDF0}", BaseAssetProcessorMessage);
584+
static void Reflect(AZ::ReflectContext* context);
585+
static constexpr unsigned int MessageType = AZ_CRC("AssetSystem::UpdateSourceControlStatusRequest", 0x42f7a516);
586+
587+
UpdateSourceControlStatusRequest() = default;
588+
unsigned int GetMessageType() const override;
589+
590+
bool m_sourceControlEnabled = false;
591+
};
592+
576593
//////////////////////////////////////////////////////////////////////////
577594
//ShowAssetInAssetProcessorRequest
578595
class ShowAssetInAssetProcessorRequest
@@ -1310,10 +1327,11 @@ namespace AzFramework
13101327

13111328
// The default constructor is only required for the SerializeContext.
13121329
AssetChangeReportResponse() = default;
1313-
AssetChangeReportResponse(AZStd::vector<AZStd::string> lines);
1330+
AssetChangeReportResponse(AZStd::vector<AZStd::string> lines, bool success);
13141331
unsigned int GetMessageType() const override;
13151332

13161333
AZStd::vector<AZStd::string> m_lines;
1334+
bool m_success = false;
13171335
};
13181336

13191337
} // namespace AssetSystem

Code/Framework/AzFramework/AzFramework/Asset/AssetSystemBus.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -268,6 +268,7 @@ namespace AzFramework
268268

269269
//! Show the AssetProcessor App
270270
virtual void ShowAssetProcessor() = 0;
271+
virtual void UpdateSourceControlStatus(bool newStatus) = 0;
271272
//! Show an asset in the AssetProcessor App
272273
virtual void ShowInAssetProcessor(const AZStd::string& assetPath) = 0;
273274

Code/Framework/AzFramework/AzFramework/Asset/AssetSystemComponent.cpp

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -264,6 +264,7 @@ namespace AzFramework
264264
RegisterSourceAssetRequest::Reflect(context);
265265
UnregisterSourceAssetRequest::Reflect(context);
266266
ShowAssetProcessorRequest::Reflect(context);
267+
UpdateSourceControlStatusRequest::Reflect(context);
267268
ShowAssetInAssetProcessorRequest::Reflect(context);
268269
FileOpenRequest::Reflect(context);
269270
FileCloseRequest::Reflect(context);

Code/Framework/AzFramework/AzFramework/Asset/AssetSystemComponent.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,6 +147,7 @@ namespace AzFramework
147147
bool EscalateAssetBySearchTerm(AZStd::string_view searchTerm) override;
148148

149149
void ShowAssetProcessor() override;
150+
void UpdateSourceControlStatus(bool newStatus) override;
150151
void ShowInAssetProcessor(const AZStd::string& assetPath) override;
151152

152153
void GetUnresolvedProductReferences(AZ::Data::AssetId assetId, AZ::u32& unresolvedAssetIdReferences, AZ::u32& unresolvedPathReferences) override;

Code/Framework/AzFramework/AzFramework/Asset/AssetSystemComponentHelper.cpp

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,13 @@ namespace AzFramework
4343
SendRequest(request);
4444
}
4545

46+
void AssetSystemComponent::UpdateSourceControlStatus(bool newStatus)
47+
{
48+
UpdateSourceControlStatusRequest request;
49+
request.m_sourceControlEnabled = newStatus;
50+
SendRequest(request);
51+
}
52+
4653
void AssetSystemComponent::ShowInAssetProcessor(const AZStd::string& assetPath)
4754
{
4855
AllowAssetProcessorToForeground();

Code/Framework/AzFramework/Platform/Linux/AzFramework/Process/ProcessWatcher_Linux.cpp

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,10 @@ namespace AzFramework
368368
{
369369
AZ_TracePrintf("Process Watcher", "ProcessLauncher::LaunchProcess: Unable to launch process %s : errno = %s\n", commandAndArgs[0], strerror(errorCodeFromChild));
370370
processData.m_childProcessIsDone = true;
371+
if (errorCodeFromChild == ENOENT)
372+
{
373+
processLaunchInfo.m_launchResult = PLR_MissingFile; // Note for future maintainers, m_launchResult is mutable
374+
}
371375
child_pid = -1;
372376
}
373377
}
@@ -382,7 +386,13 @@ namespace AzFramework
382386
delete [] commandAndArgs;
383387

384388
// If an error occurs, exit the application.
385-
return child_pid >= 0;
389+
if (child_pid >= 0)
390+
{
391+
processLaunchInfo.m_launchResult = PLR_Success; // Note for future maintainers, m_launchResult is mutable
392+
return true;
393+
}
394+
395+
return false;
386396
}
387397

388398
StdProcessCommunicator* ProcessWatcher::CreateStdCommunicator()

Code/Framework/AzQtComponents/AzQtComponents/Components/Widgets/AssetFolderThumbnailView.cpp

Lines changed: 14 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -275,22 +275,24 @@ namespace AzQtComponents
275275
QLineEdit* lineEdit = qobject_cast<QLineEdit*>(widget);
276276
if (lineEdit)
277277
{
278-
connect(
279-
lineEdit,
280-
&QLineEdit::editingFinished,
281-
this,
282-
[this]()
283-
{
284-
auto sendingLineEdit = qobject_cast<QLineEdit*>(sender());
285-
if (sendingLineEdit)
286-
{
287-
emit RenameThumbnail(sendingLineEdit->text());
288-
}
289-
});
278+
connect(lineEdit, &QLineEdit::editingFinished, this, &AssetFolderThumbnailViewDelegate::editingFinished);
290279
}
291280
return widget;
292281
}
293282

283+
void AssetFolderThumbnailViewDelegate::editingFinished()
284+
{
285+
if (auto sendingLineEdit = qobject_cast<QLineEdit*>(sender()); sendingLineEdit)
286+
{
287+
// debounce this call by disconnecting from the signal immediately.
288+
// This is because editingFinished is triggered repeatedly, for example,
289+
// when the user presses enter but also when the line edit loses focus.
290+
// If the user presses enter, both happen, causing a double send if we don't debounce like this.
291+
QObject::disconnect(sendingLineEdit, &QLineEdit::editingFinished, this, &AssetFolderThumbnailViewDelegate::editingFinished);
292+
emit RenameThumbnail(sendingLineEdit->text());
293+
}
294+
}
295+
294296
void AssetFolderThumbnailViewDelegate::updateEditorGeometry(QWidget* editor, const QStyleOptionViewItem& option, const QModelIndex& index) const
295297
{
296298
if (QLineEdit* lineEdit = qobject_cast<QLineEdit*>(editor))

0 commit comments

Comments
 (0)