Removing calls that tested if a document was open and replacing them with other validity checks. Documents should be valid and usable if they are created without opening from another source. Some documents like material documents needed extra checks to prevent accessing uninitialized values.

Signed-off-by: gadams3 <[email protected]>

Gems/Atom/Tools/AtomToolsFramework/Code/Include/AtomToolsFramework/Document/AtomToolsAnyDocument.h

@@ -25,7 +25,7 @@ namespace AtomToolsFramework
         , public AtomToolsAnyDocumentRequestBus::Handler
     {
     public:
-        AZ_RTTI(AtomToolsAnyDocument, "{7DD73C7D-06BB-4AF1-907A-0F87AFDA54AF}", AtomToolsDocument);
+        AZ_RTTI(AtomToolsAnyDocument, "{EB339BF7-CC6B-4BC5-BD6C-7B5CAAFEE012}", AtomToolsDocument);
         AZ_CLASS_ALLOCATOR(AtomToolsAnyDocument, AZ::SystemAllocator, 0);
         AZ_DISABLE_COPY_MOVE(AtomToolsAnyDocument);
 

Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsAnyDocument.cpp

@@ -81,28 +81,22 @@ namespace AtomToolsFramework
 
     DocumentObjectInfoVector AtomToolsAnyDocument::GetObjectInfo() const
     {
-        if (!IsOpen())
-        {
-            AZ_Error("AtomToolsAnyDocument", false, "Document is not open.");
-            return {};
-        }
+        DocumentObjectInfoVector objects = AtomToolsDocument::GetObjectInfo();
 
-        if (m_content.empty())
+        if (!m_content.empty())
         {
-            return {};
+            // The reflected data stored within the document will be converted to a description of the object and its type info. This data
+            // will be used to populate the inspector.
+            DocumentObjectInfo objectInfo;
+            objectInfo.m_visible = true;
+            objectInfo.m_name = GetDocumentTypeInfo().m_documentTypeName;
+            objectInfo.m_displayName = GetDocumentTypeInfo().m_documentTypeName;
+            objectInfo.m_description = GetDocumentTypeInfo().m_documentTypeName;
+            objectInfo.m_objectType = m_content.type();
+            objectInfo.m_objectPtr = AZStd::any_cast<void>(const_cast<AZStd::any*>(&m_content));
+            objects.push_back(AZStd::move(objectInfo));
         }
 
-        // The reflected data stored within the document will be converted to a description of the object and its type info. This data will
-        // be used to populate the inspector and anything else concerned with RTTI.
-        DocumentObjectInfoVector objects;
-        DocumentObjectInfo objectInfo;
-        objectInfo.m_visible = true;
-        objectInfo.m_name = GetDocumentTypeInfo().m_documentTypeName;
-        objectInfo.m_displayName = GetDocumentTypeInfo().m_documentTypeName;
-        objectInfo.m_description = GetDocumentTypeInfo().m_documentTypeName;
-        objectInfo.m_objectType = m_content.type();
-        objectInfo.m_objectPtr = AZStd::any_cast<void>(const_cast<AZStd::any*>(&m_content));
-        objects.push_back(objectInfo);
         return objects;
     }
 

Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocument.cpp

@@ -132,12 +132,6 @@ namespace AtomToolsFramework
 
     bool AtomToolsDocument::Save()
     {
-        if (!IsOpen())
-        {
-            AZ_Error("AtomToolsDocument", false, "Document is not open to be saved: '%s'.", m_absolutePath.c_str());
-            return SaveFailed();
-        }
-
         if (!CanSave())
         {
             AZ_Error("AtomToolsDocument", false, "Document type can not be saved: '%s'.", m_absolutePath.c_str());
@@ -162,12 +156,6 @@ namespace AtomToolsFramework
 
     bool AtomToolsDocument::SaveAsCopy(const AZStd::string& savePath)
     {
-        if (!IsOpen())
-        {
-            AZ_Error("AtomToolsDocument", false, "Document is not open to be saved: '%s'.", m_absolutePath.c_str());
-            return SaveFailed();
-        }
-
         if (!CanSave())
         {
             AZ_Error("AtomToolsDocument", false, "Document type can not be saved: '%s'.", m_absolutePath.c_str());
@@ -192,12 +180,6 @@ namespace AtomToolsFramework
 
     bool AtomToolsDocument::SaveAsChild(const AZStd::string& savePath)
     {
-        if (!IsOpen())
-        {
-            AZ_Error("AtomToolsDocument", false, "Document is not open to be saved: '%s'.", m_absolutePath.c_str());
-            return SaveFailed();
-        }
-
         m_savePathNormalized = savePath;
         if (!ValidateDocumentPath(m_savePathNormalized))
         {
@@ -222,14 +204,7 @@ namespace AtomToolsFramework
 
     bool AtomToolsDocument::Close()
     {
-        if (!IsOpen())
-        {
-            AZ_Error("AtomToolsDocument", false, "Document is not open.");
-            return false;
-        }
-
         AZ_TracePrintf("AtomToolsDocument", "Document closed: '%s'.\n", m_absolutePath.c_str());
-
         AtomToolsDocumentNotificationBus::Event(m_toolId, &AtomToolsDocumentNotificationBus::Events::OnDocumentClosed, m_id);
 
         // Clearing after notification so paths are still available
@@ -252,7 +227,7 @@ namespace AtomToolsFramework
 
     bool AtomToolsDocument::IsOpen() const
     {
-        return !m_id.IsNull() && !m_absolutePath.empty();
+        return !m_id.IsNull();
     }
 
     bool AtomToolsDocument::IsModified() const
@@ -262,19 +237,19 @@ namespace AtomToolsFramework
 
     bool AtomToolsDocument::CanSave() const
     {
-        return IsOpen() && GetDocumentTypeInfo().IsSupportedExtensionToSave(m_absolutePath);
+        return GetDocumentTypeInfo().IsSupportedExtensionToSave(m_absolutePath);
     }
 
     bool AtomToolsDocument::CanUndo() const
     {
         // Undo will only be allowed if something has been recorded and we're not at the beginning of history
-        return IsOpen() && !m_undoHistory.empty() && m_undoHistoryIndex > 0;
+        return !m_undoHistory.empty() && m_undoHistoryIndex > 0;
     }
 
     bool AtomToolsDocument::CanRedo() const
     {
         // Redo will only be allowed if something has been recorded and we're not at the end of history
-        return IsOpen() && !m_undoHistory.empty() && m_undoHistoryIndex < m_undoHistory.size();
+        return !m_undoHistory.empty() && m_undoHistoryIndex < m_undoHistory.size();
     }
 
     bool AtomToolsDocument::Undo()

Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentInspector.cpp

@@ -126,34 +126,31 @@ namespace AtomToolsFramework
             m_documentId,
             [&](AtomToolsDocumentRequests* documentRequests)
             {
-                if (documentRequests->IsOpen())
+                // Create a unique settings prefix string per document using a CRC of the document path
+                const AZStd::string groupSettingsPrefix = m_documentSettingsPrefix +
+                    AZStd::string::format("/%08x/GroupSettings", aznumeric_cast<AZ::u32>(AZ::Crc32(documentRequests->GetAbsolutePath())));
+                SetGroupSettingsPrefix(groupSettingsPrefix);
+
+                // This will automatically expose all document contents to an inspector with a collapsible group per object.
+                // In the case of the material editor, this will be one inspector group per property group.
+                for (auto& objectInfo : documentRequests->GetObjectInfo())
                 {
-                    // Create a unique settings prefix string per document using a CRC of the document path
-                    const AZStd::string groupSettingsPrefix = m_documentSettingsPrefix +
-                        AZStd::string::format("/%08x/GroupSettings", aznumeric_cast<AZ::u32>(AZ::Crc32(documentRequests->GetAbsolutePath())));
-                    SetGroupSettingsPrefix(groupSettingsPrefix);
-
-                    // This will automatically expose all document contents to an inspector with a collapsible group per object.
-                    // In the case of the material editor, this will be one inspector group per property group.
-                    for (auto& objectInfo : documentRequests->GetObjectInfo())
-                    {
-                        // Passing in same main and comparison instance to enable custom value comparison
-                        const AZ::Crc32 groupSaveStateKey(
-                            AZStd::string::format("%s/%s", groupSettingsPrefix.c_str(), objectInfo.m_name.c_str()));
-                        auto propertyGroupWidget = new InspectorPropertyGroupWidget(
-                            objectInfo.m_objectPtr,
-                            objectInfo.m_objectPtr,
-                            objectInfo.m_objectType,
-                            this,
-                            this,
-                            groupSaveStateKey,
-                            {},
-                            objectInfo.m_nodeIndicatorFunction,
-                            0);
-
-                        AddGroup(objectInfo.m_name, objectInfo.m_displayName, objectInfo.m_description, propertyGroupWidget);
-                        SetGroupVisible(objectInfo.m_name, objectInfo.m_visible);
-                    }
+                    // Passing in same main and comparison instance to enable custom value comparison
+                    const AZ::Crc32 groupSaveStateKey(
+                        AZStd::string::format("%s/%s", groupSettingsPrefix.c_str(), objectInfo.m_name.c_str()));
+                    auto propertyGroupWidget = new InspectorPropertyGroupWidget(
+                        objectInfo.m_objectPtr,
+                        objectInfo.m_objectPtr,
+                        objectInfo.m_objectType,
+                        this,
+                        this,
+                        groupSaveStateKey,
+                        {},
+                        objectInfo.m_nodeIndicatorFunction,
+                        0);
+
+                    AddGroup(objectInfo.m_name, objectInfo.m_displayName, objectInfo.m_description, propertyGroupWidget);
+                    SetGroupVisible(objectInfo.m_name, objectInfo.m_visible);
                 }
 
                 InspectorRequestBus::Handler::BusConnect(m_documentId);

Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentMainWindow.cpp

@@ -573,8 +573,6 @@ namespace AtomToolsFramework
 
     void AtomToolsDocumentMainWindow::OnDocumentOpened(const AZ::Uuid& documentId)
     {
-        bool isOpen = false;
-        AtomToolsDocumentRequestBus::EventResult(isOpen, documentId, &AtomToolsDocumentRequestBus::Events::IsOpen);
         AZStd::string absolutePath;
         AtomToolsDocumentRequestBus::EventResult(absolutePath, documentId, &AtomToolsDocumentRequestBus::Events::GetAbsolutePath);
 
@@ -585,7 +583,7 @@ namespace AtomToolsFramework
         // Whenever a document is opened or selected select the corresponding tab
         m_tabWidget->setCurrentIndex(GetDocumentTabIndex(documentId));
 
-        if (isOpen && !absolutePath.empty())
+        if (!absolutePath.empty())
         {
             // Find and select the file path in the asset browser
             m_assetBrowser->SelectEntries(absolutePath);

Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/AtomToolsDocumentSystem.cpp

@@ -283,20 +283,12 @@ namespace AtomToolsFramework
 
     bool AtomToolsDocumentSystem::CloseDocument(const AZ::Uuid& documentId)
     {
-        bool isOpen = false;
-        AtomToolsDocumentRequestBus::EventResult(isOpen, documentId, &AtomToolsDocumentRequestBus::Events::IsOpen);
-        if (!isOpen)
-        {
-            // immediately destroy unopened documents
-            DestroyDocument(documentId);
-            return true;
-        }
-
         AZStd::string documentPath;
         AtomToolsDocumentRequestBus::EventResult(documentPath, documentId, &AtomToolsDocumentRequestBus::Events::GetAbsolutePath);
 
         bool isModified = false;
         AtomToolsDocumentRequestBus::EventResult(isModified, documentId, &AtomToolsDocumentRequestBus::Events::IsModified);
+
         if (isModified)
         {
             auto selection = QMessageBox::question(

Gems/Atom/Tools/AtomToolsFramework/Code/Source/Document/CreateDocumentDialog.cpp

@@ -98,8 +98,8 @@ namespace AtomToolsFramework
     CreateDocumentDialog::CreateDocumentDialog(const DocumentTypeInfo& documentType, const QString& initialPath, QWidget* parent)
         : CreateDocumentDialog(
               tr("Create %1 Document").arg(documentType.m_documentTypeName.c_str()),
-              tr("Select Type"),
-              tr("Select %1 Path").arg(documentType.m_documentTypeName.c_str()),
+              tr("Select source file, type, or template to create %1 document").arg(documentType.m_documentTypeName.c_str()),
+              tr("Select target path to save %1 document").arg(documentType.m_documentTypeName.c_str()),
               initialPath,
               { documentType.GetDefaultExtensionToSave().c_str() },
               documentType.m_defaultAssetIdToCreate,

Gems/Atom/Tools/MaterialCanvas/Code/Source/Document/MaterialCanvasDocument.cpp

@@ -69,16 +69,22 @@ namespace MaterialCanvas
         : AtomToolsFramework::AtomToolsDocument(toolId, documentTypeInfo)
         , m_graphContext(graphContext)
     {
+        AZ_Assert(m_graphContext, "Graph context must be valid in order to create a graph document.");
+
         // Creating the scene entity and graph for this document. This may end up moving to the view if we can have the document only store
         // minimal material graph specific data that can be transformed into a graph canvas graph in the view and back. That abstraction
         // would help maintain a separation between the serialized data and the UI for rendering and interacting with the graph. This would
         // also help establish a mediator pattern for other node based tools that want to visualize their data or documents as a graph. My
         // understanding is that graph model will help with this.
         m_graph = AZStd::make_shared<GraphModel::Graph>(m_graphContext);
+        AZ_Assert(m_graph, "Failed to create graph object.");
 
         GraphModelIntegration::GraphManagerRequestBus::BroadcastResult(
             m_sceneEntity, &GraphModelIntegration::GraphManagerRequests::CreateScene, m_graph, m_toolId);
+        AZ_Assert(m_sceneEntity, "Failed to create graph scene entity.");
+
         m_graphId = m_sceneEntity->GetId();
+        AZ_Assert(m_graphId.IsValid(), "Graph scene entity ID is not valid.");
 
         RecordGraphState();
 
@@ -150,14 +156,8 @@ namespace MaterialCanvas
 
     AtomToolsFramework::DocumentObjectInfoVector MaterialCanvasDocument::GetObjectInfo() const
     {
-        if (!IsOpen())
-        {
-            AZ_Error("MaterialCanvasDocument", false, "Document is not open.");
-            return {};
-        }
-
-        AtomToolsFramework::DocumentObjectInfoVector objects;
-        objects.reserve(m_groups.size());
+        AtomToolsFramework::DocumentObjectInfoVector objects = AtomToolsDocument::GetObjectInfo();
+        objects.reserve(objects.size() + m_groups.size());
 
         for (const auto& group : m_groups)
         {
@@ -175,7 +175,7 @@ namespace MaterialCanvas
                     // There are currently no indicators for material canvas nodes.
                     return nullptr;
                 };
-                objects.push_back(objectInfo);
+                objects.push_back(AZStd::move(objectInfo));
             }
         }
 
@@ -219,7 +219,8 @@ namespace MaterialCanvas
             return false;
         }
 
-        if (!AZ::JsonSerializationUtils::SaveObjectToFile(m_graph.get(), m_savePathNormalized))
+        AZ_Error("MaterialCanvasDocument", m_graph, "Attempting to save invalid graph objeTt. ");
+        if (!m_graph || !AZ::JsonSerializationUtils::SaveObjectToFile(m_graph.get(), m_savePathNormalized))
         {
             return SaveFailed();
         }
@@ -239,7 +240,8 @@ namespace MaterialCanvas
             return false;
         }
 
-        if (!AZ::JsonSerializationUtils::SaveObjectToFile(m_graph.get(), m_savePathNormalized))
+        AZ_Error("MaterialCanvasDocument", m_graph, "Attempting to save invalid graph objeTt. ");
+        if (!m_graph || !AZ::JsonSerializationUtils::SaveObjectToFile(m_graph.get(), m_savePathNormalized))
         {
             return SaveFailed();
         }
@@ -259,7 +261,8 @@ namespace MaterialCanvas
             return false;
         }
 
-        if (!AZ::JsonSerializationUtils::SaveObjectToFile(m_graph.get(), m_savePathNormalized))
+        AZ_Error("MaterialCanvasDocument", m_graph, "Attempting to save invalid graph object. ");
+        if (!m_graph || !AZ::JsonSerializationUtils::SaveObjectToFile(m_graph.get(), m_savePathNormalized))
         {
             return SaveFailed();
         }
@@ -270,11 +273,6 @@ namespace MaterialCanvas
         return SaveSucceeded();
     }
 
-    bool MaterialCanvasDocument::IsOpen() const
-    {
-        return AtomToolsDocument::IsOpen() && m_graph && m_graphId.IsValid();
-    }
-
     bool MaterialCanvasDocument::IsModified() const
     {
         return m_modified;
@@ -328,16 +326,6 @@ namespace MaterialCanvas
         AtomToolsFramework::AtomToolsDocument::Clear();
     }
 
-    bool MaterialCanvasDocument::ReopenRecordState()
-    {
-        return AtomToolsDocument::ReopenRecordState();
-    }
-
-    bool MaterialCanvasDocument::ReopenRestoreState()
-    {
-        return AtomToolsDocument::ReopenRestoreState();
-    }
-
     void MaterialCanvasDocument::OnGraphModelRequestUndoPoint()
     {
         // Undo and redo is being handled differently for edits received directly from graph model and graph canvas. By the time this is
@@ -411,14 +399,18 @@ namespace MaterialCanvas
     {
         DestroyGraph();
 
-        m_graph = graph;
-        m_graph->PostLoadSetup(m_graphContext);
-        BuildEditablePropertyGroups();
-        RecordGraphState();
+        if (graph)
+        {
+            m_graph = graph;
+            m_graph->PostLoadSetup(m_graphContext);
 
-        // The graph controller will create all of the scene items on construction.
-        GraphModelIntegration::GraphManagerRequestBus::Broadcast(
-            &GraphModelIntegration::GraphManagerRequests::CreateGraphController, m_graphId, m_graph);
+            BuildEditablePropertyGroups();
+            RecordGraphState();
+
+            // The graph controller will create all of the scene items on construction.
+            GraphModelIntegration::GraphManagerRequestBus::Broadcast(
+                &GraphModelIntegration::GraphManagerRequests::CreateGraphController, m_graphId, m_graph);
+        }
     }
 
     void MaterialCanvasDocument::DestroyGraph()
@@ -656,6 +648,8 @@ namespace MaterialCanvas
     AZStd::vector<GraphModel::ConstNodePtr> MaterialCanvasDocument::GetInstructionNodesInExecutionOrder(
         GraphModel::ConstNodePtr outputNode, const AZStd::vector<AZStd::string>& inputSlotNames) const
     {
+        AZ_Assert(m_graph, "Attempting to generate data from invalid graph object.");
+
         AZStd::vector<GraphModel::ConstNodePtr> sortedNodes;
         sortedNodes.reserve(m_graph->GetNodes().size());
 
@@ -772,6 +766,8 @@ namespace MaterialCanvas
 
     AZStd::vector<AZStd::string> MaterialCanvasDocument::GetMaterialInputsFromNodes() const
     {
+        AZ_Assert(m_graph, "Attempting to generate data from invalid graph object.");
+
         AZStd::vector<AZStd::string> materialInputs;
 
         for (const auto& inputNodePair : m_graph->GetNodes())
@@ -869,6 +865,9 @@ namespace MaterialCanvas
     bool MaterialCanvasDocument::BuildMaterialTypeFromTemplate(
         GraphModel::ConstNodePtr templateNode, const AZStd::string& templateInputPath, const AZStd::string& templateOutputPath) const
     {
+        AZ_Assert(m_graph, "Attempting to generate data from invalid graph object.");
+        AZ_Assert(templateNode, "Attempting to generate data from invalid template node.");
+
         // Load the material type template file, which is the same format as MaterialTypeSourceData with a different extension
         auto materialTypeOutcome = AZ::RPI::MaterialUtils::LoadMaterialTypeSourceData(templateInputPath);
         if (!materialTypeOutcome.IsSuccess())
@@ -969,7 +968,7 @@ namespace MaterialCanvas
         m_compileGraphQueued = false;
         m_generatedFiles.clear();
 
-        if (!IsOpen())
+        if (!m_graph)
         {
             return false;
         }
@@ -1127,7 +1126,7 @@ namespace MaterialCanvas
 
     void MaterialCanvasDocument::QueueCompileGraph() const
     {
-        if (IsOpen() && !m_compileGraphQueued)
+        if (m_graph && !m_compileGraphQueued)
         {
             m_compileGraphQueued = true;
             AZ::SystemTickBus::QueueFunction([id = m_id](){

Gems/Atom/Tools/MaterialCanvas/Code/Source/Document/MaterialCanvasDocument.h

@@ -22,7 +22,8 @@
 
 namespace MaterialCanvas
 {
-    //! MaterialCanvasDocument
+    //! MaterialCanvasDocument implements support for creating, loading, saving, and manipulating graph model and canvas graphs that
+    //! represent and will be transformed into shader and material data.
     class MaterialCanvasDocument
         : public AtomToolsFramework::AtomToolsDocument
         , public MaterialCanvasDocumentRequestBus::Handler
@@ -30,7 +31,7 @@ namespace MaterialCanvas
         , public GraphCanvas::SceneNotificationBus::Handler
     {
     public:
-        AZ_RTTI(MaterialCanvasDocument, "{90299628-AD02-4FEB-9527-7278FA2817AD}", AtomToolsFramework::AtomToolsDocument);
+        AZ_RTTI(MaterialCanvasDocument, "{C60CA7B2-D9FA-4E20-AD7D-D8A661902A7D}", AtomToolsFramework::AtomToolsDocument);
         AZ_CLASS_ALLOCATOR(MaterialCanvasDocument, AZ::SystemAllocator, 0);
         AZ_DISABLE_COPY_MOVE(MaterialCanvasDocument);
 
@@ -50,7 +51,6 @@ namespace MaterialCanvas
         bool Save() override;
         bool SaveAsCopy(const AZStd::string& savePath) override;
         bool SaveAsChild(const AZStd::string& savePath) override;
-        bool IsOpen() const override;
         bool IsModified() const override;
         bool BeginEdit() override;
         bool EndEdit() override;
@@ -66,8 +66,6 @@ namespace MaterialCanvas
     private:
         // AtomToolsFramework::AtomToolsDocument overrides...
         void Clear() override;
-        bool ReopenRecordState() override;
-        bool ReopenRestoreState() override;
 
         // GraphModelIntegration::GraphControllerNotificationBus::Handler overrides...
         void OnGraphModelRequestUndoPoint() override;

Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.cpp

@@ -80,12 +80,6 @@ namespace MaterialEditor
 
     void MaterialDocument::SetPropertyValue(const AZStd::string& propertyId, const AZStd::any& value)
     {
-        if (!IsOpen())
-        {
-            AZ_Error("MaterialDocument", false, "Document is not open.");
-            return;
-        }
-
         const AZ::Name propertyName(propertyId);
 
         AtomToolsFramework::DynamicProperty* foundProperty = {};
@@ -96,20 +90,23 @@ namespace MaterialEditor
                 {
                     foundProperty = &property;
 
-                    // This first converts to an acceptable runtime type in case the value came from script
-                    const AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(value);
+                    if (m_materialInstance)
+                    {
+                        // This first converts to an acceptable runtime type in case the value came from script
+                        const AZ::RPI::MaterialPropertyValue propertyValue = AtomToolsFramework::ConvertToRuntimeType(value);
 
-                    property.SetValue(AtomToolsFramework::ConvertToEditableType(propertyValue));
+                        property.SetValue(AtomToolsFramework::ConvertToEditableType(propertyValue));
 
-                    const auto propertyIndex = m_materialInstance->FindPropertyIndex(propertyName);
-                    if (!propertyIndex.IsNull())
-                    {
-                        if (m_materialInstance->SetPropertyValue(propertyIndex, propertyValue))
+                        const auto propertyIndex = m_materialInstance->FindPropertyIndex(propertyName);
+                        if (!propertyIndex.IsNull())
                         {
-                            AZ::RPI::MaterialPropertyFlags dirtyFlags = m_materialInstance->GetPropertyDirtyFlags();
+                            if (m_materialInstance->SetPropertyValue(propertyIndex, propertyValue))
+                            {
+                                AZ::RPI::MaterialPropertyFlags dirtyFlags = m_materialInstance->GetPropertyDirtyFlags();
 
-                            Recompile();
-                            RunEditorMaterialFunctors(dirtyFlags);
+                                Recompile();
+                                RunEditorMaterialFunctors(dirtyFlags);
+                            }
                         }
                     }
 
@@ -133,12 +130,6 @@ namespace MaterialEditor
 
     const AZStd::any& MaterialDocument::GetPropertyValue(const AZStd::string& propertyId) const
     {
-        if (!IsOpen())
-        {
-            AZ_Error("MaterialDocument", false, "Document is not open.");
-            return m_invalidValue;
-        }
-
         auto property = FindProperty(AZ::Name(propertyId));
         if (!property)
         {
@@ -169,19 +160,12 @@ namespace MaterialEditor
 
     AtomToolsFramework::DocumentObjectInfoVector MaterialDocument::GetObjectInfo() const
     {
-        if (!IsOpen())
-        {
-            AZ_Error("MaterialDocument", false, "Document is not open.");
-            return {};
-        }
-
-        AtomToolsFramework::DocumentObjectInfoVector objects;
-        objects.reserve(m_groups.size());
+        AtomToolsFramework::DocumentObjectInfoVector objects = AtomToolsDocument::GetObjectInfo();
+        objects.reserve(objects.size() + m_groups.size());
 
-        AtomToolsFramework::DocumentObjectInfo objectInfo;
         for (const auto& group : m_groups)
         {
-            objects.push_back(GetObjectInfoFromDynamicPropertyGroup(group.get()));
+            objects.push_back(AZStd::move(GetObjectInfoFromDynamicPropertyGroup(group.get())));
         }
 
         return objects;
@@ -198,7 +182,11 @@ namespace MaterialEditor
 
         // populate sourceData with modified or overridden properties and save object
         AZ::RPI::MaterialSourceData sourceData;
-        sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion();
+        if (m_materialAsset.IsReady() &&
+            m_materialAsset->GetMaterialTypeAsset().IsReady())
+        {
+            sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion();
+        }
         sourceData.m_materialType = AtomToolsFramework::GetPathToExteralReference(m_absolutePath, m_materialSourceData.m_materialType);
         sourceData.m_parentMaterial = AtomToolsFramework::GetPathToExteralReference(m_absolutePath, m_materialSourceData.m_parentMaterial);
         auto propertyFilter = [](const AtomToolsFramework::DynamicProperty& property) {
@@ -234,7 +222,11 @@ namespace MaterialEditor
 
         // populate sourceData with modified or overridden properties and save object
         AZ::RPI::MaterialSourceData sourceData;
-        sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion();
+        if (m_materialAsset.IsReady() &&
+            m_materialAsset->GetMaterialTypeAsset().IsReady())
+        {
+            sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion();
+        }
         sourceData.m_materialType = AtomToolsFramework::GetPathToExteralReference(m_savePathNormalized, m_materialSourceData.m_materialType);
         sourceData.m_parentMaterial = AtomToolsFramework::GetPathToExteralReference(m_savePathNormalized, m_materialSourceData.m_parentMaterial);
         auto propertyFilter = [](const AtomToolsFramework::DynamicProperty& property) {
@@ -266,7 +258,11 @@ namespace MaterialEditor
 
         // populate sourceData with modified or overridden properties and save object
         AZ::RPI::MaterialSourceData sourceData;
-        sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion();
+        if (m_materialAsset.IsReady() &&
+            m_materialAsset->GetMaterialTypeAsset().IsReady())
+        {
+            sourceData.m_materialTypeVersion = m_materialAsset->GetMaterialTypeAsset()->GetVersion();
+        }
         sourceData.m_materialType = AtomToolsFramework::GetPathToExteralReference(m_savePathNormalized, m_materialSourceData.m_materialType);
 
         // Only assign a parent path if the source was a .material
@@ -293,11 +289,6 @@ namespace MaterialEditor
         return SaveSucceeded();
     }
 
-    bool MaterialDocument::IsOpen() const
-    {
-        return AtomToolsDocument::IsOpen() && m_materialAsset.IsReady() && m_materialInstance;
-    }
-
     bool MaterialDocument::IsModified() const
     {
         bool result = false;
@@ -362,7 +353,7 @@ namespace MaterialEditor
     {
         if (m_compilePending)
         {
-            if (m_materialInstance->Compile())
+            if (m_materialInstance && m_materialInstance->Compile())
             {
                 m_compilePending = false;
                 AZ::SystemTickBus::Handler::BusDisconnect();
@@ -884,6 +875,11 @@ namespace MaterialEditor
 
     void MaterialDocument::RunEditorMaterialFunctors(AZ::RPI::MaterialPropertyFlags dirtyFlags)
     {
+        if (!m_materialInstance)
+        {
+            return;
+        }
+
         AZStd::unordered_map<AZ::Name, AZ::RPI::MaterialPropertyDynamicMetadata> propertyDynamicMetadata;
         AZStd::unordered_map<AZ::Name, AZ::RPI::MaterialPropertyGroupDynamicMetadata> propertyGroupDynamicMetadata;
 

Gems/Atom/Tools/MaterialEditor/Code/Source/Document/MaterialDocument.h

@@ -46,7 +46,6 @@ namespace MaterialEditor
         bool Save() override;
         bool SaveAsCopy(const AZStd::string& savePath) override;
         bool SaveAsChild(const AZStd::string& savePath) override;
-        bool IsOpen() const override;
         bool IsModified() const override;
         bool BeginEdit() override;
         bool EndEdit() override;

Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.cpp

@@ -83,7 +83,7 @@ namespace ShaderManagementConsole
 
     size_t ShaderManagementConsoleDocument::GetShaderOptionDescriptorCount() const
     {
-        if (IsOpen())
+        if (m_shaderAsset.IsReady())
         {
             const auto& layout = m_shaderAsset->GetShaderOptionGroupLayout();
             const auto& shaderOptionDescriptors = layout->GetShaderOptions();
@@ -94,7 +94,7 @@ namespace ShaderManagementConsole
 
     const AZ::RPI::ShaderOptionDescriptor& ShaderManagementConsoleDocument::GetShaderOptionDescriptor(size_t index) const
     {
-        if (IsOpen())
+        if (m_shaderAsset.IsReady())
         {
             const auto& layout = m_shaderAsset->GetShaderOptionGroupLayout();
             const auto& shaderOptionDescriptors = layout->GetShaderOptions();
@@ -109,9 +109,6 @@ namespace ShaderManagementConsole
         documentType.m_documentTypeName = "Shader Variant List";
         documentType.m_documentFactoryCallback = [](const AZ::Crc32& toolId, const AtomToolsFramework::DocumentTypeInfo& documentTypeInfo) {
             return aznew ShaderManagementConsoleDocument(toolId, documentTypeInfo); };
-        documentType.m_supportedExtensionsToCreate.push_back({ "Shader", AZ::RPI::ShaderSourceData::Extension });
-        documentType.m_supportedExtensionsToCreate.push_back({ "Shader Variant List", AZ::RPI::ShaderVariantListSourceData::Extension });
-        documentType.m_supportedExtensionsToOpen.push_back({ "Shader", AZ::RPI::ShaderSourceData::Extension });
         documentType.m_supportedExtensionsToOpen.push_back({ "Shader Variant List", AZ::RPI::ShaderVariantListSourceData::Extension });
         documentType.m_supportedExtensionsToSave.push_back({ "Shader Variant List", AZ::RPI::ShaderVariantListSourceData::Extension });
         documentType.m_supportedAssetTypesToCreate.insert(azrtti_typeid<AZ::RPI::ShaderAsset>());
@@ -120,13 +117,7 @@ namespace ShaderManagementConsole
 
     AtomToolsFramework::DocumentObjectInfoVector ShaderManagementConsoleDocument::GetObjectInfo() const
     {
-        if (!IsOpen())
-        {
-            AZ_Error("ShaderManagementConsoleDocument", false, "Document is not open.");
-            return {};
-        }
-
-        AtomToolsFramework::DocumentObjectInfoVector objects;
+        AtomToolsFramework::DocumentObjectInfoVector objects = AtomToolsDocument::GetObjectInfo();
 
         AtomToolsFramework::DocumentObjectInfo objectInfo;
         objectInfo.m_visible = true;
@@ -199,11 +190,6 @@ namespace ShaderManagementConsole
         return SaveSourceData();
     }
 
-    bool ShaderManagementConsoleDocument::IsOpen() const
-    {
-        return AtomToolsDocument::IsOpen() && m_shaderAsset.IsReady();
-    }
-
     bool ShaderManagementConsoleDocument::IsModified() const
     {
         return m_modified;
@@ -370,7 +356,7 @@ namespace ShaderManagementConsole
         SetShaderVariantListSourceData(shaderVariantListSourceData);
         m_modified = {};
 
-        return IsOpen() ? OpenSucceeded() : OpenFailed();
+        return OpenSucceeded();
     }
 
     AZStd::vector<AZ::Data::AssetId> ShaderManagementConsoleDocument::FindMaterialAssetsUsingShader(const AZStd::string& shaderFilePath)

Gems/Atom/Tools/ShaderManagementConsole/Code/Source/Document/ShaderManagementConsoleDocument.h

@@ -42,7 +42,6 @@ namespace ShaderManagementConsole
         bool Save() override;
         bool SaveAsCopy(const AZStd::string& savePath) override;
         bool SaveAsChild(const AZStd::string& savePath) override;
-        bool IsOpen() const override;
         bool IsModified() const override;
         bool BeginEdit() override;
         bool EndEdit() override;