Repository: RetroAchievements/RAIntegration
Pull request #688 (2021-03-03 04:08:45)

Prevent saving achievement that can't be deserialized
When a trigger cannot be parsed, the achievement can't be loaded. 

This prevents the user from saving an unparseable achievement. When a trigger becomes unparseable, an icon is displayed on the editor, and the user is no longer able to change to another achievement without fixing the error. Additionally, the "broken" achievement is automatically de-activated and cannot be re-activated until it is fixed. It can also not be saved or cloned while invalid. A tooltip on the icon indicates the error returned by the parser.

![image](https://user-images.githubusercontent.com/32680403/109750998-c24df100-7b9a-11eb-8011-c31c005f511e.png)



Patch:
diff --git a/src/RA_Resource.h b/src/RA_Resource.h
index 70be30d9..cc06f4a1 100644
--- a/src/RA_Resource.h
+++ b/src/RA_Resource.h
@@ -95,6 +95,7 @@
 #define IDC_RA_MOVE_COND_UP             1190
 #define IDC_RA_MOVE_COND_DOWN           1191
 #define IDC_RA_DISPLAY_CHALLENGE_INDICATOR 1192
+#define IDC_RA_ERROR_INDICATOR          1193
 #define IDD_RA_MEMORY                   1501
 #define IDD_RA_ACHIEVEMENTS             1502
 #define IDD_RA_ACHIEVEMENTEDITOR        1503
@@ -177,7 +178,7 @@
 #ifndef APSTUDIO_READONLY_SYMBOLS
 #define _APS_NEXT_RESOURCE_VALUE        122
 #define _APS_NEXT_COMMAND_VALUE         40001
-#define _APS_NEXT_CONTROL_VALUE         1193
+#define _APS_NEXT_CONTROL_VALUE         1194
 #define _APS_NEXT_SYMED_VALUE           101
 #endif
 #endif
diff --git a/src/RA_Shared.rc b/src/RA_Shared.rc
index 14e7c6b0..77751447 100644
--- a/src/RA_Shared.rc
+++ b/src/RA_Shared.rc
@@ -132,6 +132,7 @@ BEGIN
     PUSHBUTTON      "+",IDC_RA_ADD_GROUP,4,176,15,13
     PUSHBUTTON      "-",IDC_RA_DELETE_GROUP,31,176,15,13
     LTEXT           "Requirements:",IDC_STATIC,50,55,48,8
+    CONTROL         "",IDC_RA_ERROR_INDICATOR,"Static",SS_ICON | SS_CENTERIMAGE | SS_NOTIFY,97,53,12,12
     CONTROL         "",IDC_RA_LBX_CONDITIONS,"SysListView32",LVS_REPORT | LVS_SHOWSELALWAYS | LVS_ALIGNLEFT | LVS_NOSORTHEADER | WS_BORDER | WS_TABSTOP,50,66,391,106
     CONTROL         "Pause on Reset",IDC_RA_CHK_PAUSE_ON_RESET,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,307,53,65,13
     CONTROL         "Pause on Trigger",IDC_RA_CHK_PAUSE_ON_TRIGGER,"Button",BS_AUTOCHECKBOX | WS_TABSTOP,376,53,65,13
diff --git a/src/data/context/GameContext.cpp b/src/data/context/GameContext.cpp
index 82d7894e..2b36a3df 100644
--- a/src/data/context/GameContext.cpp
+++ b/src/data/context/GameContext.cpp
@@ -452,7 +452,8 @@ void GameContext::AwardAchievement(ra::AchievementID nAchievementId)
         bSubmit = false;
     }
 
-    if (pAchievement->IsModified())
+    if (pAchievement->IsModified() || // actual modifications
+        (bSubmit && pAchievement->GetChanges() != ra::data::models::AssetChanges::None)) // unpublished changes
     {
         auto sHeader = vmPopup->GetTitle();
         sHeader.insert(0, L"Modified ");
diff --git a/src/ui/viewmodels/AssetEditorViewModel.cpp b/src/ui/viewmodels/AssetEditorViewModel.cpp
index a88dd02e..e7e3775d 100644
--- a/src/ui/viewmodels/AssetEditorViewModel.cpp
+++ b/src/ui/viewmodels/AssetEditorViewModel.cpp
@@ -22,7 +22,9 @@ const StringModelProperty AssetEditorViewModel::BadgeProperty("AssetEditorViewMo
 const BoolModelProperty AssetEditorViewModel::PauseOnResetProperty("AssetEditorViewModel", "PauseOnReset", false);
 const BoolModelProperty AssetEditorViewModel::PauseOnTriggerProperty("AssetEditorViewModel", "PauseOnTrigger", false);
 const BoolModelProperty AssetEditorViewModel::DecimalPreferredProperty("AssetEditorViewModel", "DecimalPreferred", false);
-const BoolModelProperty AssetEditorViewModel::AssetLoadedProperty("AssetEditorViewModel", "AssetLoaded", false);
+const BoolModelProperty AssetEditorViewModel::IsAssetLoadedProperty("AssetEditorViewModel", "IsAssetLoaded", false);
+const BoolModelProperty AssetEditorViewModel::HasAssetValidationErrorProperty("AssetEditorViewModel", "HasAssetValidationError", false);
+const StringModelProperty AssetEditorViewModel::AssetValidationErrorProperty("AssetEditorViewModel", "AssetValidationError", L"");
 const StringModelProperty AssetEditorViewModel::WaitingLabelProperty("AssetEditorViewModel", "WaitingLabel", L"Active");
 
 AssetEditorViewModel::AssetEditorViewModel() noexcept
@@ -65,8 +67,24 @@ void AssetEditorViewModel::LoadAsset(ra::data::models::AssetModelBase* pAsset)
     if (m_pAsset == pAsset)
         return;
 
-    if (m_pAsset)
+    if (HasAssetValidationError())
+    {
+        if (ra::ui::viewmodels::MessageBoxViewModel::ShowWarningMessage(L"Discard changes?",
+            L"The currently loaded asset has an error that cannot be saved. If you switch to another asset, your changes will be lost.",
+            ra::ui::viewmodels::MessageBoxViewModel::Buttons::YesNo) == ra::ui::DialogResult::No)
+        {
+            return;
+        }
+
         m_pAsset->RemoveNotifyTarget(*this);
+        m_pAsset->RestoreLocalCheckpoint();
+
+        SetValue(AssetValidationErrorProperty, L"");
+    }
+    else if (m_pAsset)
+    {
+        m_pAsset->RemoveNotifyTarget(*this);
+    }
 
     // null out internal pointer while binding to prevent change triggers
     m_pAsset = nullptr;
@@ -111,11 +129,11 @@ void AssetEditorViewModel::LoadAsset(ra::data::models::AssetModelBase* pAsset)
         }
 
         m_pAsset = pAsset;
-        SetValue(AssetLoadedProperty, true);
+        SetValue(IsAssetLoadedProperty, true);
     }
     else
     {
-        SetValue(AssetLoadedProperty, false);
+        SetValue(IsAssetLoadedProperty, false);
 
         SetName(NameProperty.GetDefaultValue());
         SetValue(IDProperty, IDProperty.GetDefaultValue());
@@ -225,6 +243,9 @@ void AssetEditorViewModel::OnValueChanged(const StringModelProperty::ChangeArgs&
         }
     }
 
+    if (args.Property == AssetValidationErrorProperty)
+        SetValue(HasAssetValidationErrorProperty, !args.tNewValue.empty());
+
     WindowViewModelBase::OnValueChanged(args);
 }
 
@@ -285,14 +306,39 @@ void AssetEditorViewModel::OnTriggerChanged()
     if (pAchievement != nullptr)
     {
         const std::string& sTrigger = Trigger().Serialize();
-        pAchievement->SetTrigger(sTrigger);
 
-        // if trigger has actually changed, code should call back through UpdateTriggerBinding to update the UI
+        const int nSize = rc_trigger_size(sTrigger.c_str());
+        if (nSize < 0)
+        {
+            SetValue(AssetValidationErrorProperty, ra::Widen(rc_error_str(nSize)));
+
+            // if the achievement is active, we have to disable it
+            if (pAchievement->IsActive())
+                pAchievement->SetState(ra::data::models::AssetState::Inactive);
+
+            // we need to update the trigger to mark the asset as modified, and so any attempt to reset/revert
+            // the trigger will cause a PropertyChanged event. but we can't allow UpdateTriggerBinding to try
+            // to process it because the trigger is not valid.
+            m_bIgnoreTriggerUpdate = true;
+            pAchievement->SetTrigger(sTrigger);
+            m_bIgnoreTriggerUpdate = false;
+        }
+        else
+        {
+            SetValue(AssetValidationErrorProperty, L"");
+
+            pAchievement->SetTrigger(sTrigger);
+
+            // if trigger has actually changed, the code will call back through UpdateTriggerBinding to update the UI
+        }
     }
 }
 
 void AssetEditorViewModel::UpdateTriggerBinding()
 {
+    if (m_bIgnoreTriggerUpdate)
+        return;
+
     const auto* pAchievement = dynamic_cast<ra::data::models::AchievementModel*>(m_pAsset);
     if (pAchievement != nullptr)
     {
@@ -304,6 +350,8 @@ void AssetEditorViewModel::UpdateTriggerBinding()
             Trigger().UpdateFrom(*pTrigger);
         else
             Trigger().UpdateFrom(pAchievement->GetTrigger());
+
+        SetValue(AssetValidationErrorProperty, L"");
     }
 }
 
diff --git a/src/ui/viewmodels/AssetEditorViewModel.hh b/src/ui/viewmodels/AssetEditorViewModel.hh
index 73c1f42a..b520b6c1 100644
--- a/src/ui/viewmodels/AssetEditorViewModel.hh
+++ b/src/ui/viewmodels/AssetEditorViewModel.hh
@@ -28,14 +28,34 @@ public:
     AssetEditorViewModel& operator=(AssetEditorViewModel&&) noexcept = delete;
 
     /// <summary>
-    /// The <see cref="ModelProperty" /> for whether or not the emulator should be paused when the achievement triggers.
+    /// The <see cref="ModelProperty" /> for whether or not an asset is loaded in the editor.
     /// </summary>
-    static const BoolModelProperty AssetLoadedProperty;
+    static const BoolModelProperty IsAssetLoadedProperty;
 
     /// <summary>
-    /// Gets whether or not the emulator should be paused when the achievement triggers.
+    /// Gets whether or not an asset is loaded in the editor.
+    /// </summary>
+    bool IsAssetLoaded() const { return GetValue(IsAssetLoadedProperty); }
+
+    /// <summary>
+    /// The <see cref="ModelProperty" /> for whether or not an AssetValidationError is set.
+    /// </summary>
+    static const BoolModelProperty HasAssetValidationErrorProperty;
+
+    /// <summary>
+    /// Gets whether or not an AssetValidationError is set.
     /// </summary>
-    bool IsAssetLoaded() const { return GetValue(AssetLoadedProperty); }
+    bool HasAssetValidationError() const { return GetValue(HasAssetValidationErrorProperty); }
+
+    /// <summary>
+    /// The <see cref="ModelProperty" /> for the reason why the asset is not valid.
+    /// </summary>
+    static const StringModelProperty AssetValidationErrorProperty;
+
+    /// <summary>
+    /// Gets the reason why the asset is not valid.
+    /// </summary>
+    const std::wstring& GetAssetValidationError() const { return GetValue(AssetValidationErrorProperty); }
 
     // ===== Common =====
 
@@ -202,6 +222,7 @@ public:
     // ===== Functions =====
 
     void LoadAsset(ra::data::models::AssetModelBase* pAsset);
+    const ra::data::models::AssetModelBase* GetAsset() const noexcept { return m_pAsset; }
 
     void DoFrame();
 
@@ -221,14 +242,13 @@ protected:
     void OnValueChanged(const StringModelProperty::ChangeArgs& args) override;
     void OnValueChanged(const IntModelProperty::ChangeArgs& args) override;
 
-    void OnDataModelStateChanged(const IntModelProperty::ChangeArgs& args);
-
     void UpdateTriggerBinding();
     void OnTriggerChanged();
 
     TriggerViewModel m_vmTrigger;
 
     ra::data::models::AssetModelBase* m_pAsset = nullptr;
+    bool m_bIgnoreTriggerUpdate = false;
 };
 
 } // namespace viewmodels
diff --git a/src/ui/viewmodels/AssetListViewModel.cpp b/src/ui/viewmodels/AssetListViewModel.cpp
index 27b1d5f2..e0a88939 100644
--- a/src/ui/viewmodels/AssetListViewModel.cpp
+++ b/src/ui/viewmodels/AssetListViewModel.cpp
@@ -679,6 +679,28 @@ void AssetListViewModel::GetSelectedAssets(std::vector<ra::data::models::AssetMo
     }
 }
 
+bool AssetListViewModel::SelectionContainsInvalidAsset(const std::vector<ra::data::models::AssetModelBase*>& vSelectedAssets, _Out_ std::wstring& sErrorMessage) const
+{
+    auto& pWindowManager = ra::services::ServiceLocator::GetMutable<ra::ui::viewmodels::WindowManager>();
+    if (pWindowManager.AssetEditor.HasAssetValidationError())
+    {
+        const auto* vmInvalidAsset = pWindowManager.AssetEditor.GetAsset();
+        for (const auto* vmItem : vSelectedAssets)
+        {
+            if (vmItem == vmInvalidAsset)
+            {
+                sErrorMessage = ra::StringPrintf(L"The following errors must be corrected:\n* %s",
+                    pWindowManager.AssetEditor.GetAssetValidationError());
+
+                return true;
+            }
+        }
+    }
+
+    sErrorMessage.clear();
+    return false;
+}
+
 void AssetListViewModel::ActivateSelected()
 {
     std::vector<ra::data::models::AssetModelBase*> vSelectedAssets;
@@ -694,6 +716,13 @@ void AssetListViewModel::ActivateSelected()
     }
     else // Activate
     {
+        std::wstring sErrorMessage;
+        if (SelectionContainsInvalidAsset(vSelectedAssets, sErrorMessage))
+        {
+            ra::ui::viewmodels::MessageBoxViewModel::ShowErrorMessage(L"Unable to activate", sErrorMessage);
+            return;
+        }
+
         for (auto* vmItem : vSelectedAssets)
         {
             if (vmItem)
@@ -721,6 +750,13 @@ void AssetListViewModel::SaveSelected()
                     vSelectedAssets.push_back(pAsset);
             }
         }
+
+        std::wstring sErrorMessage;
+        if (SelectionContainsInvalidAsset(vSelectedAssets, sErrorMessage))
+        {
+            ra::ui::viewmodels::MessageBoxViewModel::ShowErrorMessage(L"Unable to save", sErrorMessage);
+            return;
+        }
     }
     else if (sSaveButtonText.at(0) == 'P' && sSaveButtonText.at(1) == 'u') // "Publi&sh" / "Publi&sh All"
     {
@@ -1111,6 +1147,13 @@ void AssetListViewModel::CloneSelected()
     if (vSelectedAssets.empty())
         return;
 
+    std::wstring sErrorMessage;
+    if (SelectionContainsInvalidAsset(vSelectedAssets, sErrorMessage))
+    {
+        ra::ui::viewmodels::MessageBoxViewModel::ShowErrorMessage(L"Unable to clone", sErrorMessage);
+        return;
+    }
+
     auto& pGameContext = ra::services::ServiceLocator::GetMutable<ra::data::context::GameContext>();
 
     FilteredAssets().BeginUpdate();
diff --git a/src/ui/viewmodels/AssetListViewModel.hh b/src/ui/viewmodels/AssetListViewModel.hh
index 99099e3e..7f9d1adf 100644
--- a/src/ui/viewmodels/AssetListViewModel.hh
+++ b/src/ui/viewmodels/AssetListViewModel.hh
@@ -188,6 +188,7 @@ protected:
     virtual void Publish(std::vector<ra::data::models::AssetModelBase*>& vAssets);
     bool ValidateAssetsForCore(std::vector<ra::data::models::AssetModelBase*>& vAssets, bool bCoreOnly);
     virtual void ValidateAchievementForCore(std::wstring& sError, const ra::data::models::AchievementModel& pAchievement) const;
+    virtual bool SelectionContainsInvalidAsset(const std::vector<ra::data::models::AssetModelBase*>& vSelectedAssets, _Out_ std::wstring& sErrorMessage) const;
 
 private:
     // DataModelCollectionBase::NotifyTarget
diff --git a/src/ui/viewmodels/TriggerViewModel.cpp b/src/ui/viewmodels/TriggerViewModel.cpp
index 820ae47f..203398df 100644
--- a/src/ui/viewmodels/TriggerViewModel.cpp
+++ b/src/ui/viewmodels/TriggerViewModel.cpp
@@ -414,6 +414,7 @@ void TriggerViewModel::UpdateFrom(const rc_trigger_t& pTrigger)
     auto* vmCoreGroup = m_vGroups.GetItemAt(0);
     Expects(vmCoreGroup != nullptr);
     vmCoreGroup->m_pConditionSet = pTrigger.requirement;
+    vmCoreGroup->ResetSerialized();
 
     // update alt groups (add any new alt groups)
     gsl::index nIndex = 0;
@@ -422,9 +423,14 @@ void TriggerViewModel::UpdateFrom(const rc_trigger_t& pTrigger)
     {
         auto* vmAltGroup = m_vGroups.GetItemAt(++nIndex);
         if (vmAltGroup != nullptr)
+        {
             vmAltGroup->m_pConditionSet = pConditionSet;
+            vmAltGroup->ResetSerialized();
+        }
         else
+        {
             AddAltGroup(m_vGroups, gsl::narrow_cast<int>(nIndex), pConditionSet);
+        }
     }
 
     // remove any extra alt groups
diff --git a/src/ui/viewmodels/TriggerViewModel.hh b/src/ui/viewmodels/TriggerViewModel.hh
index 0ef47d76..97f66c26 100644
--- a/src/ui/viewmodels/TriggerViewModel.hh
+++ b/src/ui/viewmodels/TriggerViewModel.hh
@@ -49,6 +49,7 @@ public:
 
         bool UpdateSerialized(const ViewModelCollection<TriggerConditionViewModel>& vConditions);
         const std::string& GetSerialized() const;
+        void ResetSerialized() noexcept { m_sSerialized.clear(); }
 
     private:
         static void UpdateSerialized(std::string& sSerialized, const ViewModelCollection<TriggerConditionViewModel>& vConditions);
diff --git a/src/ui/win32/AssetEditorDialog.cpp b/src/ui/win32/AssetEditorDialog.cpp
index d2d38e55..66af4b03 100644
--- a/src/ui/win32/AssetEditorDialog.cpp
+++ b/src/ui/win32/AssetEditorDialog.cpp
@@ -395,9 +395,24 @@ void AssetEditorDialog::ActiveCheckBoxBinding::OnCommand()
     // check state has already been updated - update the backing data to match
     const auto bIsChecked = Button_GetCheck(m_hWnd);
     if (bIsChecked)
+    {
+        if (GetValue(ra::ui::viewmodels::AssetEditorViewModel::HasAssetValidationErrorProperty))
+        {
+            Button_SetCheck(m_hWnd, BST_UNCHECKED);
+
+            const auto sError = ra::StringPrintf(L"The following errors must be corrected:\n* %s",
+                GetValue(ra::ui::viewmodels::AssetEditorViewModel::AssetValidationErrorProperty));
+            ra::ui::viewmodels::MessageBoxViewModel::ShowErrorMessage(L"Unable to activate", sError);
+
+            return;
+        }
+
         SetValue(ra::ui::viewmodels::AssetEditorViewModel::StateProperty, ra::etoi(ra::data::models::AssetState::Waiting));
+    }
     else
+    {
         SetValue(ra::ui::viewmodels::AssetEditorViewModel::StateProperty, ra::etoi(ra::data::models::AssetState::Inactive));
+    }
 
     OnValueChanged();
 }
@@ -441,26 +456,27 @@ AssetEditorDialog::AssetEditorDialog(AssetEditorViewModel& vmAssetEditor)
     m_bindBadge.SetWrapAround(true);
     m_bindBadgeImage.BindImage(AssetEditorViewModel::BadgeProperty, ra::ui::ImageType::Badge);
     m_bindPoints.BindValue(AssetEditorViewModel::PointsProperty);
-    m_bindWindow.BindEnabled(IDC_RA_TITLE, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_DESCRIPTION, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_CHK_ACTIVE, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_POINTS, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_BADGENAME, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_BADGE_SPIN, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_UPLOAD_BADGE, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_CHK_PAUSE_ON_RESET, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_CHK_PAUSE_ON_TRIGGER, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_LBX_GROUPS, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_ADD_GROUP, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_DELETE_GROUP, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_LBX_CONDITIONS, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_ADD_COND, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_DELETE_COND, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_COPY_COND, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_PASTE_COND, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_MOVE_COND_UP, AssetEditorViewModel::AssetLoadedProperty);
-    m_bindWindow.BindEnabled(IDC_RA_MOVE_COND_DOWN, AssetEditorViewModel::AssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_TITLE, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_DESCRIPTION, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_CHK_ACTIVE, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_POINTS, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_BADGENAME, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_BADGE_SPIN, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_UPLOAD_BADGE, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_CHK_PAUSE_ON_RESET, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_CHK_PAUSE_ON_TRIGGER, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_LBX_GROUPS, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_ADD_GROUP, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_DELETE_GROUP, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_LBX_CONDITIONS, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_ADD_COND, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_DELETE_COND, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_COPY_COND, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_PASTE_COND, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_MOVE_COND_UP, AssetEditorViewModel::IsAssetLoadedProperty);
+    m_bindWindow.BindEnabled(IDC_RA_MOVE_COND_DOWN, AssetEditorViewModel::IsAssetLoadedProperty);
     m_bindWindow.BindLabel(IDC_RA_CHK_ACTIVE, AssetEditorViewModel::WaitingLabelProperty);
+    m_bindWindow.BindVisible(IDC_RA_ERROR_INDICATOR, AssetEditorViewModel::HasAssetValidationErrorProperty);
 
     m_bindPauseOnReset.BindCheck(AssetEditorViewModel::PauseOnResetProperty);
     m_bindPauseOnTrigger.BindCheck(AssetEditorViewModel::PauseOnTriggerProperty);
@@ -601,9 +617,93 @@ BOOL AssetEditorDialog::OnInitDialog()
     m_bindActive.SetControl(*this, IDC_RA_CHK_ACTIVE);
     m_bindDecimalPreferred.SetControl(*this, IDC_RA_CHK_SHOW_DECIMALS);
 
+    if (!m_hErrorIcon)
+    {
+        SHSTOCKICONINFO sii{};
+        sii.cbSize = sizeof(sii);
+        if (SUCCEEDED(::SHGetStockIconInfo(SIID_ERROR, SHGSI_ICON | SHGSI_SMALLICON, &sii)))
+            m_hErrorIcon = sii.hIcon;
+    }
+
+    const auto hErrorIconControl = GetDlgItem(GetHWND(), IDC_RA_ERROR_INDICATOR);
+    if (m_hErrorIcon)
+        GSL_SUPPRESS_TYPE1 ::SendMessage(hErrorIconControl, STM_SETICON, reinterpret_cast<WPARAM>(m_hErrorIcon), NULL);
+
+    m_hTooltip = CreateWindowEx(WS_EX_TOPMOST, TOOLTIPS_CLASS, nullptr,
+        WS_POPUP | TTS_ALWAYSTIP | TTS_NOPREFIX,
+        CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT, CW_USEDEFAULT,
+        GetHWND(), nullptr, GetModuleHandle(nullptr), nullptr);
+
+    if (m_hTooltip)
+    {
+        TOOLINFO toolInfo;
+        memset(&toolInfo, 0, sizeof(toolInfo));
+        GSL_SUPPRESS_ES47 toolInfo.cbSize = TTTOOLINFO_V1_SIZE;
+        toolInfo.uFlags = TTF_IDISHWND | TTF_SUBCLASS;
+        toolInfo.hwnd = GetHWND();
+        GSL_SUPPRESS_TYPE1 toolInfo.uId = reinterpret_cast<UINT_PTR>(hErrorIconControl);
+        toolInfo.lpszText = LPSTR_TEXTCALLBACK;
+        ::GetClientRect(hErrorIconControl, &toolInfo.rect);
+        GSL_SUPPRESS_TYPE1 SendMessage(m_hTooltip, TTM_ADDTOOL, 0, reinterpret_cast<LPARAM>(&toolInfo));
+        SendMessage(m_hTooltip, TTM_ACTIVATE, TRUE, 0);
+    }
+
     return DialogBase::OnInitDialog();
 }
 
+INT_PTR CALLBACK AssetEditorDialog::DialogProc(HWND hDlg, UINT uMsg, WPARAM wParam, LPARAM lParam)
+{
+    switch (uMsg)
+    {
+        case WM_NOTIFY:
+        {
+            LPNMHDR pnmHdr;
+            GSL_SUPPRESS_TYPE1{ pnmHdr = reinterpret_cast<LPNMHDR>(lParam); }
+            switch (pnmHdr->code)
+            {
+                case TTN_GETDISPINFO:
+                {
+                    if (pnmHdr->hwndFrom == m_hTooltip)
+                    {
+                        const auto* vmAssetEditor = dynamic_cast<AssetEditorViewModel*>(&m_vmWindow);
+                        if (vmAssetEditor)
+                        {
+                            NMTTDISPINFO* pnmTtDispInfo;
+                            GSL_SUPPRESS_TYPE1{ pnmTtDispInfo = reinterpret_cast<NMTTDISPINFO*>(lParam); }
+                            Expects(pnmTtDispInfo != nullptr);
+                            GSL_SUPPRESS_TYPE3 pnmTtDispInfo->lpszText = const_cast<LPTSTR>(vmAssetEditor->GetAssetValidationError().c_str());
+                            return 0;
+                        }
+                    }
+
+                    break;
+                }
+            }
+
+            break;
+        }
+    }
+
+    return DialogBase::DialogProc(hDlg, uMsg, wParam, lParam);
+}
+
+void AssetEditorDialog::OnDestroy()
+{
+    if (m_hErrorIcon)
+    {
+        ::DestroyIcon(m_hErrorIcon);
+        m_hErrorIcon = nullptr;
+    }
+
+    if (m_hTooltip)
+    {
+        ::DestroyWindow(m_hTooltip);
+        m_hTooltip = nullptr;
+    }
+
+    return DialogBase::OnDestroy();
+}
+
 BOOL AssetEditorDialog::OnCommand(WORD nCommand)
 {
     switch (nCommand)
diff --git a/src/ui/win32/AssetEditorDialog.hh b/src/ui/win32/AssetEditorDialog.hh
index e8cb940c..fe412b41 100644
--- a/src/ui/win32/AssetEditorDialog.hh
+++ b/src/ui/win32/AssetEditorDialog.hh
@@ -42,7 +42,9 @@ public:
 
 protected:
     BOOL OnInitDialog() override;
+    INT_PTR CALLBACK DialogProc(HWND, UINT, WPARAM, LPARAM) override;
     BOOL OnCommand(WORD nCommand) override;
+    void OnDestroy() override;
 
 private:
     class BadgeNameBinding : public ra::ui::win32::bindings::NumericUpDownBinding
@@ -97,6 +99,9 @@ private:
 
     ra::ui::win32::bindings::GridBinding m_bindGroups;
     ra::ui::win32::bindings::GridBinding m_bindConditions;
+
+    HICON m_hErrorIcon = nullptr;
+    HWND m_hTooltip = nullptr;
 };
 
 } // namespace win32


Test patch:
diff --git a/tests/RA_Integration.Tests.vcxproj b/tests/RA_Integration.Tests.vcxproj
index 802911d3..059186eb 100644
--- a/tests/RA_Integration.Tests.vcxproj
+++ b/tests/RA_Integration.Tests.vcxproj
@@ -413,6 +413,8 @@
     <ClCompile Include="ui\viewmodels\UnknownGameViewModel_Tests.cpp" />
     <ClCompile Include="ui\WindowViewModelBase_Tests.cpp" />
     <ClInclude Include="..\src\pch.h" />
+    <ClInclude Include="api\ApiAsserts.hh" />
+    <ClInclude Include="data\DataAsserts.hh" />
     <ClInclude Include="mocks\MockAchievementRuntime.hh" />
     <ClInclude Include="mocks\MockAudioSystem.hh" />
     <ClInclude Include="mocks\MockClipboard.hh" />
@@ -435,12 +437,11 @@
     <ClInclude Include="mocks\MockThreadPool.hh" />
     <ClInclude Include="mocks\MockUserContext.hh" />
     <ClInclude Include="mocks\MockWindowManager.hh" />
-    <ClInclude Include="RA_UnitTestApiHelpers.h" />
     <ClInclude Include="RA_UnitTestHelpers.h" />
     <ClCompile Include="RA_UnitTestHelpers.cpp" />
-    <ClInclude Include="RA_UnitTestDataHelpers.h" />
-    <ClInclude Include="RA_UnitTestServicesHelpers.h" />
-    <ClInclude Include="RA_UnitTestUIHelpers.h" />
+    <ClInclude Include="services\ServicesAsserts.hh" />
+    <ClInclude Include="ui\UIAsserts.hh" />
+    <ClInclude Include="ui\viewmodels\TriggerConditionAsserts.hh" />
   </ItemGroup>
   <Import Project="$(VCTargetsPath)\Microsoft.Cpp.targets" />
   <ImportGroup Label="ExtensionTargets">
diff --git a/tests/RA_Integration.Tests.vcxproj.filters b/tests/RA_Integration.Tests.vcxproj.filters
index f71a728e..f6922fa3 100644
--- a/tests/RA_Integration.Tests.vcxproj.filters
+++ b/tests/RA_Integration.Tests.vcxproj.filters
@@ -491,17 +491,20 @@
     <ClInclude Include="mocks\MockFrameEventQueue.hh">
       <Filter>Mocks</Filter>
     </ClInclude>
-    <ClInclude Include="RA_UnitTestUIHelpers.h">
-      <Filter>Tests</Filter>
+    <ClInclude Include="ui\UIAsserts.hh">
+      <Filter>Tests\UI</Filter>
     </ClInclude>
-    <ClInclude Include="RA_UnitTestDataHelpers.h">
-      <Filter>Tests</Filter>
+    <ClInclude Include="services\ServicesAsserts.hh">
+      <Filter>Tests\Services</Filter>
     </ClInclude>
-    <ClInclude Include="RA_UnitTestApiHelpers.h">
-      <Filter>Tests</Filter>
+    <ClInclude Include="data\DataAsserts.hh">
+      <Filter>Tests\Data</Filter>
     </ClInclude>
-    <ClInclude Include="RA_UnitTestServicesHelpers.h">
-      <Filter>Tests</Filter>
+    <ClInclude Include="api\ApiAsserts.hh">
+      <Filter>Tests\API</Filter>
+    </ClInclude>
+    <ClInclude Include="ui\viewmodels\TriggerConditionAsserts.hh">
+      <Filter>Tests\UI\ViewModels</Filter>
     </ClInclude>
   </ItemGroup>
 </Project>
\ No newline at end of file
diff --git a/tests/data/context/GameContext_Tests.cpp b/tests/data/context/GameContext_Tests.cpp
index fcbba392..c517c392 100644
--- a/tests/data/context/GameContext_Tests.cpp
+++ b/tests/data/context/GameContext_Tests.cpp
@@ -1175,6 +1175,32 @@ public:
         game.mockThreadPool.ExecuteNextTask();
     }
 
+    TEST_METHOD(TestAwardAchievementUnpublished)
+    {
+        GameContextHarness game;
+        game.mockConfiguration.SetPopupLocation(ra::ui::viewmodels::Popup::AchievementTriggered, ra::ui::viewmodels::PopupLocation::BottomLeft);
+        game.mockServer.ExpectUncalled<ra::api::AwardAchievement>();
+
+        auto& pAch = game.MockAchievement();
+        pAch.SetPoints(99);
+        pAch.UpdateLocalCheckpoint();
+        Assert::IsFalse(pAch.IsModified());
+        Assert::AreEqual(ra::data::models::AssetChanges::Unpublished, pAch.GetChanges());
+        game.AwardAchievement(1U);
+
+        Assert::IsTrue(game.mockAudioSystem.WasAudioFilePlayed(L"Overlay\\acherror.wav"));
+        const auto* pPopup = game.mockOverlayManager.GetMessage(1);
+        Expects(pPopup != nullptr);
+        Assert::IsNotNull(pPopup);
+        Assert::AreEqual(std::wstring(L"Modified Achievement NOT Unlocked"), pPopup->GetTitle());
+        Assert::AreEqual(std::wstring(L"AchievementTitle (99)"), pPopup->GetDescription());
+        Assert::AreEqual(std::wstring(L"AchievementDescription"), pPopup->GetDetail());
+        Assert::AreEqual(std::string("12345"), pPopup->GetImage().Name());
+
+        // AwardAchievement API call is async, try to execute it - expect no tasks queued
+        game.mockThreadPool.ExecuteNextTask();
+    }
+
     TEST_METHOD(TestAwardAchievementMemoryModified)
     {
         GameContextHarness game;
diff --git a/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp b/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp
index d2f4e0bd..362f8151 100644
--- a/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp
@@ -1,14 +1,17 @@
 #include "CppUnitTest.h"
 
 #include "ui\viewmodels\AssetEditorViewModel.hh"
+#include "ui\viewmodels\MessageBoxViewModel.hh"
 
 #include "tests\RA_UnitTestHelpers.h"
 #include "tests\data\DataAsserts.hh"
+#include "tests\ui\viewmodels\TriggerConditionAsserts.hh"
 
 #include "tests\mocks\MockAchievementRuntime.hh"
 #include "tests\mocks\MockClock.hh"
 #include "tests\mocks\MockConfiguration.hh"
 #include "tests\mocks\MockGameContext.hh"
+#include "tests\mocks\MockDesktop.hh"
 
 using namespace Microsoft::VisualStudio::CppUnitTestFramework;
 
@@ -50,11 +53,17 @@ private:
         ra::services::mocks::MockClock mockClock;
         ra::services::mocks::MockConfiguration mockConfiguration;
         ra::data::context::mocks::MockGameContext mockGameContext;
+        ra::ui::mocks::MockDesktop mockDesktop;
 
         const std::wstring& GetWaitingLabel() const
         {
             return GetValue(WaitingLabelProperty);
         }
+
+        void SetAssetValidationError(const std::wstring& sValue)
+        {
+            SetValue(AssetValidationErrorProperty, sValue);
+        }
     };
 
 
@@ -134,6 +143,110 @@ public:
         Assert::IsFalse(achievement.IsModified());
     }
 
+    TEST_METHOD(TestLoadAchievementWhileInvalid)
+    {
+        AssetEditorViewModelHarness editor;
+        AchievementModel achievement;
+        achievement.SetName(L"Test Achievement");
+        achievement.SetID(1234U);
+        achievement.SetState(AssetState::Active);
+        achievement.SetDescription(L"Do something cool");
+        achievement.SetCategory(AssetCategory::Unofficial);
+        achievement.SetPoints(10);
+        achievement.SetBadge(L"58329");
+        achievement.CreateServerCheckpoint();
+        achievement.CreateLocalCheckpoint();
+        achievement.SetDescription(L"Do something modified");
+        Assert::AreEqual(AssetChanges::Modified, achievement.GetChanges());
+
+        editor.LoadAsset(&achievement);
+
+        editor.SetAssetValidationError(L"I am error.");
+        bool bMessageSeen = false;
+        editor.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([&bMessageSeen](ra::ui::viewmodels::MessageBoxViewModel& vmMessage)
+        {
+            bMessageSeen = true;
+            Assert::AreEqual(std::wstring(L"Discard changes?"), vmMessage.GetHeader());
+            Assert::AreEqual(std::wstring(L"The currently loaded asset has an error that cannot be saved. If you switch to another asset, your changes will be lost."), vmMessage.GetMessage());
+
+            return DialogResult::No;
+        });
+        Assert::IsTrue(editor.HasAssetValidationError());
+
+        // attempt to reload current asset does not show warning
+        editor.LoadAsset(&achievement);
+        Assert::IsFalse(bMessageSeen);
+        Assert::AreEqual(1234U, editor.GetID());
+
+        AchievementModel achievement2;
+        achievement2.SetName(L"Test Achievement 2");
+        achievement2.SetID(2345U);
+        achievement2.SetState(AssetState::Waiting);
+        achievement2.SetDescription(L"Do something else");
+        achievement2.SetCategory(AssetCategory::Unofficial);
+        achievement2.SetPoints(5);
+        achievement2.SetBadge(L"83295");
+        achievement2.CreateServerCheckpoint();
+        achievement2.CreateLocalCheckpoint();
+
+        // attempt to load alternate asset shows warning (set up to abort)
+        editor.LoadAsset(&achievement2);
+        Assert::IsTrue(bMessageSeen);
+        bMessageSeen = false;
+
+        Assert::AreEqual(1234U, editor.GetID());
+        Assert::AreEqual(std::wstring(L"Test Achievement"), editor.GetName());
+        Assert::AreEqual(std::wstring(L"Do something modified"), editor.GetDescription());
+        Assert::AreEqual(AssetCategory::Unofficial, editor.GetCategory());
+        Assert::AreEqual(AssetState::Active, editor.GetState());
+        Assert::AreEqual(10, editor.GetPoints());
+        Assert::AreEqual(std::wstring(L"58329"), editor.GetBadge());
+        Assert::IsTrue(editor.IsAssetLoaded());
+        Assert::IsTrue(editor.HasAssetValidationError());
+        Assert::AreEqual(AssetChanges::Modified, achievement.GetChanges());
+
+        // attempt to load no asset shows warning (set up to abort)
+        editor.LoadAsset(nullptr);
+        Assert::IsTrue(bMessageSeen);
+        bMessageSeen = false;
+
+        Assert::AreEqual(1234U, editor.GetID());
+        Assert::AreEqual(std::wstring(L"Test Achievement"), editor.GetName());
+        Assert::AreEqual(std::wstring(L"Do something modified"), editor.GetDescription());
+        Assert::AreEqual(AssetCategory::Unofficial, editor.GetCategory());
+        Assert::AreEqual(AssetState::Active, editor.GetState());
+        Assert::AreEqual(10, editor.GetPoints());
+        Assert::AreEqual(std::wstring(L"58329"), editor.GetBadge());
+        Assert::IsTrue(editor.IsAssetLoaded());
+        Assert::IsTrue(editor.HasAssetValidationError());
+        Assert::AreEqual(AssetChanges::Modified, achievement.GetChanges());
+
+        // change behavior to not abort
+        editor.mockDesktop.ResetExpectedWindows();
+        editor.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([&bMessageSeen](ra::ui::viewmodels::MessageBoxViewModel&)
+        {
+            bMessageSeen = true;
+            return DialogResult::Yes;
+        });
+
+        editor.LoadAsset(&achievement2);
+        Assert::IsTrue(bMessageSeen);
+
+        Assert::AreEqual(2345U, editor.GetID());
+        Assert::AreEqual(std::wstring(L"Test Achievement 2"), editor.GetName());
+        Assert::AreEqual(std::wstring(L"Do something else"), editor.GetDescription());
+        Assert::AreEqual(AssetCategory::Unofficial, editor.GetCategory());
+        Assert::AreEqual(AssetState::Waiting, editor.GetState());
+        Assert::AreEqual(5, editor.GetPoints());
+        Assert::AreEqual(std::wstring(L"83295"), editor.GetBadge());
+        Assert::IsTrue(editor.IsAssetLoaded());
+        Assert::IsFalse(editor.HasAssetValidationError());
+
+        // changes should have been discarded
+        Assert::AreEqual(AssetChanges::None, achievement.GetChanges());
+        Assert::AreEqual(std::wstring(L"Do something cool"), achievement.GetDescription());
+    }
+
     TEST_METHOD(TestSyncId)
     {
         AssetEditorViewModelHarness editor;
@@ -572,6 +685,68 @@ public:
         Assert::AreEqual(2U, pCondition->GetTargetValue());
     }
 
+    TEST_METHOD(TestTriggerUpdatedInvalid)
+    {
+        AssetEditorViewModelHarness editor;
+        AchievementModel achievement;
+        achievement.SetID(1234U);
+        achievement.SetTrigger("M:0xH1234=1.1._0xH2345=1.2.");
+        achievement.CreateServerCheckpoint();
+        achievement.CreateLocalCheckpoint();
+        achievement.Activate();
+
+        editor.LoadAsset(&achievement);
+
+        // make sure the record got loaded into the runtime
+        const auto* pTrigger = editor.mockRuntime.GetAchievementTrigger(1234U);
+        Expects(pTrigger != nullptr);
+
+        Assert::AreEqual({ 2U }, editor.Trigger().Conditions().Count());
+        editor.Trigger().Conditions().GetItemAt(1)->SetType(ra::ui::viewmodels::TriggerConditionType::Measured);
+
+        // make sure the trigger definition got updated
+        Assert::AreEqual(std::string("M:0xH1234=1.1._M:0xH2345=1.2."), achievement.GetTrigger());
+
+        // make sure the error got set
+        Assert::AreEqual(std::wstring(L"Multiple measured targets"), editor.GetAssetValidationError());
+        Assert::IsTrue(editor.HasAssetValidationError());
+
+        // make sure the achievement was disabled
+        Assert::AreEqual(AssetState::Inactive, achievement.GetState());
+        pTrigger = editor.mockRuntime.GetAchievementTrigger(1234U);
+        Assert::IsNull(pTrigger);
+
+        // make sure the UI kept the change
+        Assert::AreEqual({ 2U }, editor.Trigger().Conditions().Count());
+        auto* pCondition = editor.Trigger().Conditions().GetItemAt(1);
+        Expects(pCondition != nullptr);
+        Assert::AreEqual(TriggerConditionType::Measured, pCondition->GetType());
+
+        // fix the error
+        editor.Trigger().Conditions().GetItemAt(0)->SetType(ra::ui::viewmodels::TriggerConditionType::Standard);
+
+        // make sure the trigger definition got updated
+        Assert::AreEqual(std::string("0xH1234=1.1._M:0xH2345=1.2."), achievement.GetTrigger());
+
+        // make sure the error got cleared
+        Assert::AreEqual(std::wstring(L""), editor.GetAssetValidationError());
+        Assert::IsFalse(editor.HasAssetValidationError());
+
+        // the achievement should have remained disabled
+        Assert::AreEqual(AssetState::Inactive, achievement.GetState());
+        pTrigger = editor.mockRuntime.GetAchievementTrigger(1234U);
+        Assert::IsNull(pTrigger);
+
+        // make sure the UI kept the change
+        Assert::AreEqual({ 2U }, editor.Trigger().Conditions().Count());
+        pCondition = editor.Trigger().Conditions().GetItemAt(0);
+        Expects(pCondition != nullptr);
+        Assert::AreEqual(TriggerConditionType::Standard, pCondition->GetType());
+        pCondition = editor.Trigger().Conditions().GetItemAt(1);
+        Expects(pCondition != nullptr);
+        Assert::AreEqual(TriggerConditionType::Measured, pCondition->GetType());
+    }
+
     TEST_METHOD(TestDoFrameUpdatesHitsFromActiveAchievement)
     {
         AssetEditorViewModelHarness editor;
diff --git a/tests/ui/viewmodels/AssetListViewModel_Tests.cpp b/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
index b531d9c3..d223e1e6 100644
--- a/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/AssetListViewModel_Tests.cpp
@@ -368,6 +368,23 @@ private:
                 sError.append(ra::StringPrintf(L"\n* %s: %s", pAchievement.GetName(), pIter->second));
         }
 
+        bool SelectionContainsInvalidAsset(const std::vector<ra::data::models::AssetModelBase*>& vSelectedAssets, _Out_ std::wstring& sErrorMessage) const override
+        {
+            for (const auto* pAsset : vSelectedAssets)
+            {
+                Expects(pAsset != nullptr);
+                const auto pIter = m_mPublishErrors.find(pAsset->GetID());
+                if (pIter != m_mPublishErrors.end())
+                {
+                    sErrorMessage = pIter->second;
+                    return true;
+                }
+            }
+
+            sErrorMessage.clear();
+            return false;
+        }
+
     private:
         std::map<ra::AchievementID, std::wstring> m_mPublishErrors;
     };
@@ -1320,6 +1337,34 @@ public:
         Assert::IsTrue(vmAssetList.CanActivate());
     }
 
+    TEST_METHOD(TestActivateSelectedInvalid)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.SetGameId(1U);
+        vmAssetList.SetFilterCategory(AssetCategory::Core);
+        vmAssetList.AddThreeAchievements();
+
+        Assert::AreEqual({ 2U }, vmAssetList.FilteredAssets().Count());
+        vmAssetList.FilteredAssets().GetItemAt(1)->SetSelected(true);
+        vmAssetList.SetPublishError(vmAssetList.FilteredAssets().GetItemAt(1)->GetId(), L"Error message goes here.");
+        vmAssetList.ForceUpdateButtons();
+
+        bool bMessageSeen = false;
+        vmAssetList.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([&bMessageSeen](ra::ui::viewmodels::MessageBoxViewModel& vmMessage)
+        {
+            bMessageSeen = true;
+            Assert::AreEqual(std::wstring(L"Unable to activate"), vmMessage.GetHeader());
+            Assert::AreEqual(std::wstring(L"Error message goes here."), vmMessage.GetMessage());
+
+            return DialogResult::OK;
+        });
+
+        Assert::AreEqual(AssetState::Inactive, vmAssetList.FilteredAssets().GetItemAt(1)->GetState());
+        vmAssetList.ActivateSelected();
+        Assert::IsTrue(bMessageSeen);
+        Assert::AreEqual(AssetState::Inactive, vmAssetList.FilteredAssets().GetItemAt(1)->GetState());
+    }
+
     TEST_METHOD(TestSaveSelectedLocalUnmodified)
     {
         AssetListViewModelHarness vmAssetList;
@@ -1477,6 +1522,36 @@ public:
         Assert::AreEqual(AssetChanges::None, pItem->GetChanges());
     }
 
+    TEST_METHOD(TestSaveSelectedInvalid)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.MockGameId(1U);
+        vmAssetList.AddAchievement(AssetCategory::Core, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.AddAchievement(AssetCategory::Core, 7, L"Test2", L"Desc2", L"11111", "0xH1111=1");
+
+        auto* pItem = dynamic_cast<ra::data::models::AchievementModel*>(vmAssetList.mockGameContext.Assets().GetItemAt(0));
+        Expects(pItem != nullptr);
+        pItem->SetName(L"Test1b");
+        vmAssetList.FilteredAssets().GetItemAt(0)->SetSelected(true);
+        vmAssetList.SetPublishError(pItem->GetID(), L"Error message goes here.");
+        vmAssetList.ForceUpdateButtons();
+
+        bool bMessageSeen = false;
+        vmAssetList.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([&bMessageSeen](ra::ui::viewmodels::MessageBoxViewModel& vmMessage)
+        {
+            bMessageSeen = true;
+            Assert::AreEqual(std::wstring(L"Unable to save"), vmMessage.GetHeader());
+            Assert::AreEqual(std::wstring(L"Error message goes here."), vmMessage.GetMessage());
+
+            return DialogResult::OK;
+        });
+
+        Assert::AreEqual(AssetChanges::Modified, pItem->GetChanges());
+        vmAssetList.SaveSelected();
+        Assert::IsTrue(bMessageSeen);
+        Assert::AreEqual(AssetChanges::Modified, pItem->GetChanges());
+    }
+
     TEST_METHOD(TestSaveSelectedPublishCoreModified)
     {
         AssetListViewModelHarness vmAssetList;
@@ -2155,6 +2230,38 @@ public:
         Assert::IsTrue(vmAssetList.FilteredAssets().GetItemAt(1)->IsSelected());
     }
 
+    TEST_METHOD(TestCloneInvalid)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.SetGameId(22U);
+        vmAssetList.AddAchievement(AssetCategory::Core, 5, L"Test1", L"Desc1", L"12345", "0xH1234=1");
+        vmAssetList.AddAchievement(AssetCategory::Core, 7, L"Test2", L"Desc2", L"11111", "0xH1111=1");
+
+        Assert::AreEqual({ 2U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::AreEqual({ 2U }, vmAssetList.FilteredAssets().Count());
+        Assert::AreEqual(AssetCategory::Core, vmAssetList.GetFilterCategory());
+
+        vmAssetList.FilteredAssets().GetItemAt(1)->SetSelected(true);
+        vmAssetList.SetPublishError(vmAssetList.FilteredAssets().GetItemAt(1)->GetId(), L"Error message goes here.");
+        vmAssetList.ForceUpdateButtons();
+
+        bool bMessageSeen = false;
+        vmAssetList.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>([&bMessageSeen](ra::ui::viewmodels::MessageBoxViewModel& vmMessage)
+        {
+            bMessageSeen = true;
+            Assert::AreEqual(std::wstring(L"Unable to clone"), vmMessage.GetHeader());
+            Assert::AreEqual(std::wstring(L"Error message goes here."), vmMessage.GetMessage());
+
+            return DialogResult::OK;
+        });
+
+        vmAssetList.CloneSelected();
+
+        // achievement should not be created
+        Assert::AreEqual({ 2U }, vmAssetList.mockGameContext.Assets().Count());
+        Assert::IsTrue(bMessageSeen);
+    }
+
     TEST_METHOD(TestResetSelectedAllUnmodified)
     {
         AssetListViewModelHarness vmAssetList;
@@ -2308,6 +2415,42 @@ public:
         Assert::AreEqual(AssetChanges::Modified, pAsset->GetChanges());
     }
 
+    TEST_METHOD(TestResetSelectedModifiedInvalid)
+    {
+        AssetListViewModelHarness vmAssetList;
+        vmAssetList.MockGameId(22U);
+        vmAssetList.AddThreeAchievements();
+        vmAssetList.FilteredAssets().GetItemAt(0)->SetSelected(true);
+        vmAssetList.SetPublishError(vmAssetList.FilteredAssets().GetItemAt(0)->GetId(), L"Error message goes here.");
+        vmAssetList.ForceUpdateButtons();
+        vmAssetList.AssertButtonState(ResetButtonState::Reset);
+
+        auto* pAsset = vmAssetList.mockGameContext.Assets().FindAchievement({ 1U });
+        Assert::IsNotNull(pAsset);
+        Ensures(pAsset != nullptr);
+        pAsset->SetTrigger("0x2345=1");
+        Assert::AreEqual(AssetChanges::Modified, pAsset->GetChanges());
+
+        vmAssetList.MockUserFileContents("1:\"0xH1234=0\":Test:::::User:0:0:0:::00000\n");
+
+        int nDialogShown = 0;
+        vmAssetList.mockDesktop.ExpectWindow<MessageBoxViewModel>([&nDialogShown](MessageBoxViewModel& vmMessage)
+        {
+            Assert::AreEqual(std::wstring(L"Reload from disk?"), vmMessage.GetHeader());
+            ++nDialogShown;
+            return DialogResult::Yes;
+        });
+
+        vmAssetList.ResetSelected();
+
+        // invalid asset should be allowed to be reset, with only one confirmation dialog
+        Assert::AreEqual(1, nDialogShown);
+
+        Assert::AreEqual(std::string("0xH1234=0"), pAsset->GetTrigger());
+        Assert::AreEqual(AssetCategory::Core, pAsset->GetCategory());
+        Assert::AreEqual(AssetChanges::Unpublished, pAsset->GetChanges());
+    }
+
     TEST_METHOD(TestResetSelectedNew)
     {
         AssetListViewModelHarness vmAssetList;
diff --git a/tests/ui/viewmodels/TriggerConditionAsserts.hh b/tests/ui/viewmodels/TriggerConditionAsserts.hh
new file mode 100644
index 00000000..d56344bb
--- /dev/null
+++ b/tests/ui/viewmodels/TriggerConditionAsserts.hh
@@ -0,0 +1,111 @@
+#ifndef RA_UNITTEST_TRIGGERCONDITIONASSERTS_H
+#define RA_UNITTEST_TRIGGERCONDITIONASSERTS_H
+#pragma once
+
+#include "ui\viewmodels\TriggerConditionViewModel.hh"
+
+namespace Microsoft {
+namespace VisualStudio {
+namespace CppUnitTestFramework {
+
+// converters for asserting enum values
+
+#pragma warning(push)
+#pragma warning(disable : 4505) // unreferenced inline functions, they are referenced. Must be a bug.
+
+template<>
+std::wstring ToString<ra::ui::viewmodels::TriggerConditionType>(
+    const ra::ui::viewmodels::TriggerConditionType& nConditionType)
+{
+    switch (nConditionType)
+    {
+        case ra::ui::viewmodels::TriggerConditionType::Standard:
+            return L"Standard";
+        case ra::ui::viewmodels::TriggerConditionType::PauseIf:
+            return L"PauseIf";
+        case ra::ui::viewmodels::TriggerConditionType::ResetIf:
+            return L"ResetIf";
+        case ra::ui::viewmodels::TriggerConditionType::AddSource:
+            return L"AddSource";
+        case ra::ui::viewmodels::TriggerConditionType::SubSource:
+            return L"SubSource";
+        case ra::ui::viewmodels::TriggerConditionType::AddHits:
+            return L"AddHits";
+        case ra::ui::viewmodels::TriggerConditionType::AndNext:
+            return L"AndNext";
+        case ra::ui::viewmodels::TriggerConditionType::Measured:
+            return L"Measured";
+        case ra::ui::viewmodels::TriggerConditionType::AddAddress:
+            return L"AddAddress";
+        case ra::ui::viewmodels::TriggerConditionType::OrNext:
+            return L"OrNext";
+        case ra::ui::viewmodels::TriggerConditionType::Trigger:
+            return L"Trigger";
+        case ra::ui::viewmodels::TriggerConditionType::MeasuredIf:
+            return L"MeasuredIf";
+        case ra::ui::viewmodels::TriggerConditionType::ResetNextIf:
+            return L"ResetNextIf";
+        default:
+            return std::to_wstring(static_cast<int>(nConditionType));
+    }
+}
+
+template<>
+std::wstring ToString<ra::ui::viewmodels::TriggerOperandType>(
+    const ra::ui::viewmodels::TriggerOperandType& nOperandType)
+{
+    switch (nOperandType)
+    {
+        case ra::ui::viewmodels::TriggerOperandType::Address:
+            return L"Address";
+        case ra::ui::viewmodels::TriggerOperandType::Delta:
+            return L"Delta";
+        case ra::ui::viewmodels::TriggerOperandType::Value:
+            return L"Value";
+        case ra::ui::viewmodels::TriggerOperandType::Prior:
+            return L"Prior";
+        case ra::ui::viewmodels::TriggerOperandType::BCD:
+            return L"BCD";
+        default:
+            return std::to_wstring(static_cast<int>(nOperandType));
+    }
+}
+
+template<>
+std::wstring ToString<ra::ui::viewmodels::TriggerOperatorType>(
+    const ra::ui::viewmodels::TriggerOperatorType& nConditionType)
+{
+    switch (nConditionType)
+    {
+        case ra::ui::viewmodels::TriggerOperatorType::Equals:
+            return L"Equals";
+        case ra::ui::viewmodels::TriggerOperatorType::LessThan:
+            return L"LessThan";
+        case ra::ui::viewmodels::TriggerOperatorType::LessThanOrEqual:
+            return L"LessThanOrEqual";
+        case ra::ui::viewmodels::TriggerOperatorType::GreaterThan:
+            return L"GreaterThan";
+        case ra::ui::viewmodels::TriggerOperatorType::GreaterThanOrEqual:
+            return L"GreaterThanOrEqual";
+        case ra::ui::viewmodels::TriggerOperatorType::NotEquals:
+            return L"NotEquals";
+        case ra::ui::viewmodels::TriggerOperatorType::None:
+            return L"None";
+        case ra::ui::viewmodels::TriggerOperatorType::Multiply:
+            return L"Multiply";
+        case ra::ui::viewmodels::TriggerOperatorType::Divide:
+            return L"Divide";
+        case ra::ui::viewmodels::TriggerOperatorType::BitwiseAnd:
+            return L"BitwiseAnd";
+        default:
+            return std::to_wstring(static_cast<int>(nConditionType));
+    }
+}
+
+#pragma warning(pop)
+
+} // namespace CppUnitTestFramework
+} // namespace VisualStudio
+} // namespace Microsoft
+
+#endif /* !RA_UNITTEST_TRIGGERCONDITIONASSERTS_H */
diff --git a/tests/ui/viewmodels/TriggerConditionViewModel_Tests.cpp b/tests/ui/viewmodels/TriggerConditionViewModel_Tests.cpp
index b86bedd0..202319b3 100644
--- a/tests/ui/viewmodels/TriggerConditionViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/TriggerConditionViewModel_Tests.cpp
@@ -5,107 +5,10 @@
 #include "tests\RA_UnitTestHelpers.h"
 #include "tests\mocks\MockConfiguration.hh"
 #include "tests\mocks\MockGameContext.hh"
+#include "tests\ui\viewmodels\TriggerConditionAsserts.hh"
 
 using namespace Microsoft::VisualStudio::CppUnitTestFramework;
 
-
-namespace Microsoft {
-namespace VisualStudio {
-namespace CppUnitTestFramework {
-
-template<>
-std::wstring ToString<ra::ui::viewmodels::TriggerConditionType>(
-    const ra::ui::viewmodels::TriggerConditionType& nConditionType)
-{
-    switch (nConditionType)
-    {
-        case ra::ui::viewmodels::TriggerConditionType::Standard:
-            return L"Standard";
-        case ra::ui::viewmodels::TriggerConditionType::PauseIf:
-            return L"PauseIf";
-        case ra::ui::viewmodels::TriggerConditionType::ResetIf:
-            return L"ResetIf";
-        case ra::ui::viewmodels::TriggerConditionType::AddSource:
-            return L"AddSource";
-        case ra::ui::viewmodels::TriggerConditionType::SubSource:
-            return L"SubSource";
-        case ra::ui::viewmodels::TriggerConditionType::AddHits:
-            return L"AddHits";
-        case ra::ui::viewmodels::TriggerConditionType::AndNext:
-            return L"AndNext";
-        case ra::ui::viewmodels::TriggerConditionType::Measured:
-            return L"Measured";
-        case ra::ui::viewmodels::TriggerConditionType::AddAddress:
-            return L"AddAddress";
-        case ra::ui::viewmodels::TriggerConditionType::OrNext:
-            return L"OrNext";
-        case ra::ui::viewmodels::TriggerConditionType::Trigger:
-            return L"Trigger";
-        case ra::ui::viewmodels::TriggerConditionType::MeasuredIf:
-            return L"MeasuredIf";
-        case ra::ui::viewmodels::TriggerConditionType::ResetNextIf:
-            return L"ResetNextIf";
-        default:
-            return std::to_wstring(static_cast<int>(nConditionType));
-    }
-}
-
-template<>
-std::wstring ToString<ra::ui::viewmodels::TriggerOperandType>(
-    const ra::ui::viewmodels::TriggerOperandType& nOperandType)
-{
-    switch (nOperandType)
-    {
-        case ra::ui::viewmodels::TriggerOperandType::Address:
-            return L"Address";
-        case ra::ui::viewmodels::TriggerOperandType::Delta:
-            return L"Delta";
-        case ra::ui::viewmodels::TriggerOperandType::Value:
-            return L"Value";
-        case ra::ui::viewmodels::TriggerOperandType::Prior:
-            return L"Prior";
-        case ra::ui::viewmodels::TriggerOperandType::BCD:
-            return L"BCD";
-        default:
-            return std::to_wstring(static_cast<int>(nOperandType));
-    }
-}
-
-template<>
-std::wstring ToString<ra::ui::viewmodels::TriggerOperatorType>(
-    const ra::ui::viewmodels::TriggerOperatorType& nConditionType)
-{
-    switch (nConditionType)
-    {
-        case ra::ui::viewmodels::TriggerOperatorType::Equals:
-            return L"Equals";
-        case ra::ui::viewmodels::TriggerOperatorType::LessThan:
-            return L"LessThan";
-        case ra::ui::viewmodels::TriggerOperatorType::LessThanOrEqual:
-            return L"LessThanOrEqual";
-        case ra::ui::viewmodels::TriggerOperatorType::GreaterThan:
-            return L"GreaterThan";
-        case ra::ui::viewmodels::TriggerOperatorType::GreaterThanOrEqual:
-            return L"GreaterThanOrEqual";
-        case ra::ui::viewmodels::TriggerOperatorType::NotEquals:
-            return L"NotEquals";
-        case ra::ui::viewmodels::TriggerOperatorType::None:
-            return L"None";
-        case ra::ui::viewmodels::TriggerOperatorType::Multiply:
-            return L"Multiply";
-        case ra::ui::viewmodels::TriggerOperatorType::Divide:
-            return L"Divide";
-        case ra::ui::viewmodels::TriggerOperatorType::BitwiseAnd:
-            return L"BitwiseAnd";
-        default:
-            return std::to_wstring(static_cast<int>(nConditionType));
-    }
-}
-
-} // namespace CppUnitTestFramework
-} // namespace VisualStudio
-} // namespace Microsoft
-
 namespace ra {
 namespace ui {
 namespace viewmodels {
diff --git a/tests/ui/viewmodels/TriggerViewModel_Tests.cpp b/tests/ui/viewmodels/TriggerViewModel_Tests.cpp
index bcb24c5b..3c2829e7 100644
--- a/tests/ui/viewmodels/TriggerViewModel_Tests.cpp
+++ b/tests/ui/viewmodels/TriggerViewModel_Tests.cpp
@@ -257,6 +257,26 @@ public:
         Assert::AreEqual(std::string("0xH1234=0xH2345_0xH04d2=0S0xX5555=1"), pMonitor.sNewTrigger);
     }
 
+    TEST_METHOD(TestTriggerUpdateConditionsResetAndReapplied)
+    {
+        TriggerViewModel vmTrigger;
+        Parse(vmTrigger, "0xH1234=1");
+
+        TriggerMonitor pMonitor(vmTrigger);
+        vmTrigger.SetSelectedGroupIndex(0);
+        vmTrigger.Conditions().Add();
+        Assert::AreEqual(std::string("0xH1234=1_0xH0000=0"), pMonitor.sNewTrigger);
+        Assert::AreEqual(std::string("0xH1234=1_0xH0000=0"), vmTrigger.Serialize());
+
+        vmTrigger.UpdateFrom("0xH1234=1");
+        Assert::AreEqual({ 1 }, vmTrigger.Conditions().Count());
+        Assert::AreEqual(std::string("0xH1234=1"), vmTrigger.Serialize());
+
+        vmTrigger.Conditions().Add();
+        Assert::AreEqual(std::string("0xH1234=1_0xH0000=0"), pMonitor.sNewTrigger);
+        Assert::AreEqual(std::string("0xH1234=1_0xH0000=0"), vmTrigger.Serialize());
+    }
+
     TEST_METHOD(TestCopySelectedConditionsToClipboard)
     {
         TriggerViewModelHarness vmTrigger;