Skip to content

Commit bb1f37c

Browse files
JamirasJamiras
authored
prevent reset of removed fields when switching between leaderboard sections (#1292)
1 parent b799bd1 commit bb1f37c

4 files changed

Lines changed: 98 additions & 57 deletions

File tree

‎src/data/models/AchievementModel.cpp‎

Lines changed: 31 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#include "data\models\LocalBadgesModel.hh"
1010
#include "data\models\TriggerValidation.hh"
1111

12-
#include "services\IClock.hh"
1312
#include "services\ServiceLocator.hh"
1413

1514
#include <rcheevos\src\rcheevos\rc_internal.h>
@@ -405,6 +404,27 @@ void AchievementModel::SyncTriggerToRuntime()
405404
ParseTrigger();
406405
}
407406

407+
static void UpdateRuntimeLeaderboard(rc_client_game_info_t* pGame, rc_client_achievement_info_t* pAchievementInfo, rc_trigger_t* trigger) noexcept
408+
{
409+
const auto* pOldTrigger = pAchievementInfo->trigger;
410+
pAchievementInfo->trigger = trigger;
411+
412+
// update the runtime memory reference too
413+
auto* pRuntimeTrigger = pGame->runtime.triggers;
414+
const auto* pRuntimeTriggerStop = pRuntimeTrigger + pGame->runtime.trigger_count;
415+
for (; pRuntimeTrigger < pRuntimeTriggerStop; ++pRuntimeTrigger)
416+
{
417+
if (pRuntimeTrigger->trigger == pOldTrigger &&
418+
pRuntimeTrigger->id == pAchievementInfo->public_.id)
419+
{
420+
pRuntimeTrigger->trigger = pAchievementInfo->trigger;
421+
pRuntimeTrigger->serialized_size = 0;
422+
memcpy(pRuntimeTrigger->md5, pAchievementInfo->md5, sizeof(pAchievementInfo->md5));
423+
break;
424+
}
425+
}
426+
}
427+
408428
void AchievementModel::ParseTrigger() const
409429
{
410430
const auto& sTrigger = GetTrigger();
@@ -464,29 +484,22 @@ void AchievementModel::ParseTrigger() const
464484
rc_parse_trigger_internal(&trigger->trigger, &sMemaddr, &preparse.parse);
465485
trigger->trigger.has_memrefs = 1;
466486

467-
const auto* pOldTrigger = m_pAchievementInfo->trigger;
468-
m_pAchievementInfo->trigger = &trigger->trigger;
469-
470-
// update the runtime memory reference too
471-
auto* pRuntimeTrigger = pGame->runtime.triggers;
472-
const auto* pRuntimeTriggerStop = pRuntimeTrigger + pGame->runtime.trigger_count;
473-
for (; pRuntimeTrigger < pRuntimeTriggerStop; ++pRuntimeTrigger)
474-
{
475-
if (pRuntimeTrigger->trigger == pOldTrigger &&
476-
pRuntimeTrigger->id == m_pAchievementInfo->public_.id)
477-
{
478-
pRuntimeTrigger->trigger = m_pAchievementInfo->trigger;
479-
pRuntimeTrigger->serialized_size = 0;
480-
memcpy(pRuntimeTrigger->md5, md5, sizeof(md5));
481-
break;
482-
}
483-
}
487+
UpdateRuntimeLeaderboard(pGame, m_pAchievementInfo, &trigger->trigger);
484488

485489
m_pTriggerBuffer = std::move(trigger_buffer);
486490

487491
SyncStateToRuntime();
488492
}
489493
}
494+
else
495+
{
496+
// parse error - disable achievement
497+
m_pAchievementInfo->public_.state = RC_CLIENT_ACHIEVEMENT_STATE_DISABLED;
498+
UpdateRuntimeLeaderboard(pGame, m_pAchievementInfo, nullptr);
499+
500+
// release allocated memory
501+
m_pTriggerBuffer.reset();
502+
}
490503

491504
rc_mutex_unlock(&pClient->state.mutex);
492505

‎src/data/models/LeaderboardModel.cpp‎

Lines changed: 30 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,8 @@
22

33
#include "context\IRcClient.hh"
44

5-
#include "data\context\GameContext.hh"
6-
75
#include "data\models\TriggerValidation.hh"
86

9-
#include "services\AchievementRuntime.hh"
107
#include "services\ServiceLocator.hh"
118

129
#include <rcheevos\src\rc_client_internal.h>
@@ -331,6 +328,27 @@ void LeaderboardModel::SyncDefinitionToRuntime()
331328
ParseDefinition();
332329
}
333330

331+
static void UpdateRuntimeLeaderboard(rc_client_game_info_t* pGame, rc_client_leaderboard_info_t* pLeaderboardInfo, rc_lboard_t* lboard) noexcept
332+
{
333+
const auto* pOldLboard = pLeaderboardInfo->lboard;
334+
pLeaderboardInfo->lboard = lboard;
335+
336+
// update the runtime memory reference too
337+
auto* pRuntimeLboard = pGame->runtime.lboards;
338+
const auto* pRuntimeLboardStop = pRuntimeLboard + pGame->runtime.lboard_count;
339+
for (; pRuntimeLboard < pRuntimeLboardStop; ++pRuntimeLboard)
340+
{
341+
if (pRuntimeLboard->lboard == pOldLboard &&
342+
pRuntimeLboard->id == pLeaderboardInfo->public_.id)
343+
{
344+
pRuntimeLboard->lboard = lboard;
345+
pRuntimeLboard->serialized_size = 0;
346+
memcpy(pRuntimeLboard->md5, pLeaderboardInfo->md5, sizeof(pLeaderboardInfo->md5));
347+
break;
348+
}
349+
}
350+
}
351+
334352
void LeaderboardModel::ParseDefinition() const
335353
{
336354
const auto& sMemAddr = GetDefinition();
@@ -388,23 +406,7 @@ void LeaderboardModel::ParseDefinition() const
388406
rc_parse_lboard_internal(&lboard->lboard, sMemAddr.c_str(), &preparse.parse);
389407
lboard->lboard.has_memrefs = 1;
390408

391-
const auto* pOldLboard = m_pLeaderboardInfo->lboard;
392-
m_pLeaderboardInfo->lboard = &lboard->lboard;
393-
394-
// update the runtime memory reference too
395-
auto* pRuntimeLboard = pGame->runtime.lboards;
396-
const auto* pRuntimeLboardStop = pRuntimeLboard + pGame->runtime.lboard_count;
397-
for (; pRuntimeLboard < pRuntimeLboardStop; ++pRuntimeLboard)
398-
{
399-
if (pRuntimeLboard->lboard == pOldLboard &&
400-
pRuntimeLboard->id == m_pLeaderboardInfo->public_.id)
401-
{
402-
pRuntimeLboard->lboard = m_pLeaderboardInfo->lboard;
403-
pRuntimeLboard->serialized_size = 0;
404-
memcpy(pRuntimeLboard->md5, md5, sizeof(md5));
405-
break;
406-
}
407-
}
409+
UpdateRuntimeLeaderboard(pGame, m_pLeaderboardInfo, &lboard->lboard);
408410

409411
m_pLeaderboardBuffer = std::move(lboard_buffer);
410412

@@ -417,8 +419,15 @@ void LeaderboardModel::ParseDefinition() const
417419
}
418420
else
419421
{
420-
// parse error - discard old tracker
422+
// parse error - disable leaderboard
423+
m_pLeaderboardInfo->public_.state = RC_CLIENT_LEADERBOARD_STATE_DISABLED;
424+
UpdateRuntimeLeaderboard(pGame, m_pLeaderboardInfo, nullptr);
425+
426+
// discard old tracker
421427
ReleaseLeaderboardTracker(m_pLeaderboardInfo);
428+
429+
// release allocated memory
430+
m_pLeaderboardBuffer.reset();
422431
}
423432

424433
rc_mutex_unlock(&pClient->state.mutex);

‎src/ui/viewmodels/AssetEditorViewModel.cpp‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -683,9 +683,7 @@ void AssetEditorViewModel::HandleStateChanged(ra::data::models::AssetState nOldS
683683

684684
static void AppendTrigger(std::string& sDefinition, const std::string& sTrigger)
685685
{
686-
if (sTrigger.empty())
687-
sDefinition += "0=1";
688-
else
686+
if (!sTrigger.empty())
689687
sDefinition += sTrigger;
690688
}
691689

‎tests/ui/viewmodels/AssetEditorViewModel_Tests.cpp‎

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -768,22 +768,14 @@ TEST_CLASS(AssetEditorViewModel_Tests)
768768
Assert::IsTrue(editor.IsLeaderboard());
769769
Assert::IsTrue(editor.IsTrigger());
770770
Assert::IsTrue(editor.HasMeasured());
771-
Assert::IsFalse(editor.HasAssetValidationError());
772-
Assert::IsTrue(editor.HasAssetValidationWarning());
773-
Assert::AreEqual(std::wstring(L"No Start condition"), editor.GetAssetValidationWarning());
771+
Assert::IsTrue(editor.HasAssetValidationError());
772+
Assert::AreEqual(std::wstring(L"Missing start condition"), editor.GetAssetValidationError());
774773
Assert::AreEqual(std::wstring(L"Groups:"), editor.GetGroupsHeaderLabel());
775774

776775
ra::ui::viewmodels::mocks::MockWindowManager mockWindowManager; // to copy address from memory viewer
777776
editor.Trigger().NewCondition();
778-
Assert::IsFalse(editor.HasAssetValidationError());
779-
Assert::IsTrue(editor.HasAssetValidationWarning());
780-
Assert::AreEqual(std::wstring(L"No Start condition"), editor.GetAssetValidationWarning());
781-
782-
// warning is only updated when record is saved to prevent spam when constructing a record
783-
leaderboard.Validate();
784-
Assert::IsFalse(editor.HasAssetValidationError());
785-
Assert::IsTrue(editor.HasAssetValidationWarning());
786-
Assert::AreEqual(std::wstring(L"No Cancel condition"), editor.GetAssetValidationWarning());
777+
Assert::IsTrue(editor.HasAssetValidationError());
778+
Assert::AreEqual(std::wstring(L"Missing cancel condition"), editor.GetAssetValidationError());
787779
}
788780

789781
TEST_METHOD(TestActivateNewLeaderboard)
@@ -798,9 +790,8 @@ TEST_CLASS(AssetEditorViewModel_Tests)
798790

799791
editor.LoadAsset(leaderboard);
800792

801-
Assert::IsFalse(editor.HasAssetValidationError());
802-
Assert::IsTrue(editor.HasAssetValidationWarning());
803-
Assert::AreEqual(std::wstring(L"No Start condition"), editor.GetAssetValidationWarning());
793+
Assert::IsTrue(editor.HasAssetValidationError());
794+
Assert::AreEqual(std::wstring(L"Missing start condition"), editor.GetAssetValidationError());
804795

805796
// attempt to start incomplete leaderboard should fail
806797
bool bDialogSeen = false;
@@ -2462,6 +2453,36 @@ TEST_CLASS(AssetEditorViewModel_Tests)
24622453
Assert::AreEqual(2, editor.Trigger().GetSelectedGroupIndex());
24632454
Assert::AreEqual(nDefaultColor, editor.Trigger().Conditions().GetItemAt(0)->GetRowColor().ARGB);
24642455
}
2456+
2457+
TEST_METHOD(TestClearOutLeaderboardSubmit)
2458+
{
2459+
AssetEditorViewModelHarness editor;
2460+
editor.mockDesktop.ExpectWindow<ra::ui::viewmodels::MessageBoxViewModel>(
2461+
[](ra::ui::viewmodels::MessageBoxViewModel&)
2462+
{
2463+
return DialogResult::Yes;
2464+
});
2465+
2466+
LeaderboardModel leaderboard;
2467+
leaderboard.SetDefinition("STA:0x1234=1::CAN:0x1234=2::SUB:0x1234=3::VAL=0x1235");
2468+
leaderboard.SetValueFormat(ra::data::ValueFormat::Score);
2469+
leaderboard.CreateServerCheckpoint();
2470+
leaderboard.CreateLocalCheckpoint();
2471+
2472+
editor.LoadAsset(&leaderboard);
2473+
editor.SetSelectedLeaderboardPart(AssetEditorViewModel::LeaderboardPart::Submit);
2474+
2475+
Assert::AreEqual({ 1 }, editor.Trigger().Conditions().Count());
2476+
editor.Trigger().SelectRange(0, 0, true);
2477+
editor.Trigger().RemoveSelectedConditions();
2478+
Assert::AreEqual({ 0 }, editor.Trigger().Conditions().Count());
2479+
2480+
editor.SetSelectedLeaderboardPart(AssetEditorViewModel::LeaderboardPart::Cancel);
2481+
Assert::AreEqual({ 1 }, editor.Trigger().Conditions().Count());
2482+
2483+
editor.SetSelectedLeaderboardPart(AssetEditorViewModel::LeaderboardPart::Submit);
2484+
Assert::AreEqual({ 0 }, editor.Trigger().Conditions().Count());
2485+
}
24652486
};
24662487

24672488
} // namespace tests

0 commit comments

Comments
 (0)