From 4b78d51a854cd2794b6d4e80c17692611232e14b Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 15 Oct 2019 07:08:45 -0400 Subject: [PATCH 1/9] CLogBookScreen: Use emplace_back where applicable Allows simplifying code and constructing elements in place instead of copying them (which is what would occur with the defautl move constructor). --- Runtime/MP1/CLogBookScreen.cpp | 32 ++++++++++++++++++-------------- 1 file changed, 18 insertions(+), 14 deletions(-) diff --git a/Runtime/MP1/CLogBookScreen.cpp b/Runtime/MP1/CLogBookScreen.cpp index e653fc805..087faff80 100644 --- a/Runtime/MP1/CLogBookScreen.cpp +++ b/Runtime/MP1/CLogBookScreen.cpp @@ -34,15 +34,18 @@ bool CLogBookScreen::IsScanComplete(CSaveWorld::EScanCategory category, CAssetId } void CLogBookScreen::InitializeLogBook() { - for (int i = 0; i < 5; ++i) + for (int i = 0; i < 5; ++i) { x19c_scanCompletes[i].reserve(g_MemoryCardSys->GetScanCategoryCount(CSaveWorld::EScanCategory(i + 1))); + } CPlayerState& playerState = *x4_mgr.GetPlayerState(); - for (const std::pair& scanState : g_MemoryCardSys->GetScanStates()) { - if (scanState.second == CSaveWorld::EScanCategory::None) + for (const auto& [scanId, scanCategory] : g_MemoryCardSys->GetScanStates()) { + if (scanCategory == CSaveWorld::EScanCategory::None) { continue; - bool complete = IsScanComplete(scanState.second, scanState.first, playerState); - x19c_scanCompletes[int(scanState.second) - 1].push_back(std::make_pair(scanState.first, complete)); + } + + const bool complete = IsScanComplete(scanCategory, scanId, playerState); + x19c_scanCompletes[int(scanCategory) - 1].emplace_back(scanId, complete); } std::sort(x19c_scanCompletes[4].begin(), x19c_scanCompletes[4].end(), @@ -52,13 +55,14 @@ void CLogBookScreen::InitializeLogBook() { }); auto viewIt = x200_viewScans.begin(); - for (std::vector>& category : x19c_scanCompletes) { - std::vector, TLockedToken>>& viewScans = *viewIt++; - size_t viewScanCount = std::min(category.size(), size_t(5)); + for (const std::vector>& category : x19c_scanCompletes) { + const size_t viewScanCount = std::min(category.size(), size_t(5)); + auto& viewScans = *viewIt++; viewScans.reserve(viewScanCount); - for (size_t i = 0; i < viewScanCount; ++i) - viewScans.push_back( - std::make_pair(g_SimplePool->GetObj({FOURCC('SCAN'), category[i].first}), TLockedToken{})); + + for (size_t i = 0; i < viewScanCount; ++i) { + viewScans.emplace_back(g_SimplePool->GetObj({FOURCC('SCAN'), category[i].first}), TLockedToken{}); + } } } @@ -379,9 +383,9 @@ void CLogBookScreen::UpdateRightTable() { x1f0_curViewScans.clear(); std::vector>& category = x19c_scanCompletes[x70_tablegroup_leftlog->GetUserSelection()]; x1f0_curViewScans.reserve(category.size()); - for (std::pair& scan : category) - x1f0_curViewScans.push_back( - std::make_pair(g_SimplePool->GetObj({FOURCC('SCAN'), scan.first}), TLockedToken{})); + for (const std::pair& scan : category) { + x1f0_curViewScans.emplace_back(g_SimplePool->GetObj({FOURCC('SCAN'), scan.first}), TLockedToken{}); + } PumpArticleLoad(); UpdateRightTitles(); From 3951a07bfa86c7976806cbcecb7cf16fc389c113 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 15 Oct 2019 09:12:58 -0400 Subject: [PATCH 2/9] CLogBookScreen: Dehardcode constants where applicable We can just query the containers for the iteration value --- Runtime/MP1/CLogBookScreen.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Runtime/MP1/CLogBookScreen.cpp b/Runtime/MP1/CLogBookScreen.cpp index 087faff80..b15635248 100644 --- a/Runtime/MP1/CLogBookScreen.cpp +++ b/Runtime/MP1/CLogBookScreen.cpp @@ -34,7 +34,7 @@ bool CLogBookScreen::IsScanComplete(CSaveWorld::EScanCategory category, CAssetId } void CLogBookScreen::InitializeLogBook() { - for (int i = 0; i < 5; ++i) { + for (size_t i = 0; i < x19c_scanCompletes.size(); ++i) { x19c_scanCompletes[i].reserve(g_MemoryCardSys->GetScanCategoryCount(CSaveWorld::EScanCategory(i + 1))); } @@ -345,7 +345,7 @@ void CLogBookScreen::Draw(float transInterp, float totalAlpha, float yOff) { bool CLogBookScreen::VReady() const { return true; } void CLogBookScreen::VActivate() { - for (int i = 0; i < 5; ++i) { + for (int i = 0; i < int(xa8_textpane_categories.size()); ++i) { if (IsScanCategoryReady(CSaveWorld::EScanCategory(i + 1))) { xa8_textpane_categories[i]->TextSupport().SetText(xc_pauseStrg.GetString(i + 1)); } else { From f21ee0786a7277f9ef3c8d526b1af46f0bbd0af3 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 15 Oct 2019 09:36:07 -0400 Subject: [PATCH 3/9] CLogBookScreen: Make signed/unsigned/FP conversions explicit Silences warnings relating to signed/unsigned comparisons and conversions to floating-point types. --- Runtime/MP1/CLogBookScreen.cpp | 57 ++++++++++++++++++++++------------ 1 file changed, 37 insertions(+), 20 deletions(-) diff --git a/Runtime/MP1/CLogBookScreen.cpp b/Runtime/MP1/CLogBookScreen.cpp index b15635248..00eefbe38 100644 --- a/Runtime/MP1/CLogBookScreen.cpp +++ b/Runtime/MP1/CLogBookScreen.cpp @@ -67,33 +67,38 @@ void CLogBookScreen::InitializeLogBook() { } void CLogBookScreen::UpdateRightTitles() { - std::vector>& category = x19c_scanCompletes[x70_tablegroup_leftlog->GetUserSelection()]; + const std::vector>& category = x19c_scanCompletes[x70_tablegroup_leftlog->GetUserSelection()]; for (size_t i = 0; i < xd8_textpane_titles.size(); ++i) { std::u16string string; - size_t scanIndex = x18_firstViewRightSel + i; + const auto scanIndex = size_t(x18_firstViewRightSel) + i; + if (scanIndex < x1f0_curViewScans.size()) { - std::pair, TCachedToken>& scan = x1f0_curViewScans[scanIndex]; + const auto& scan = x1f0_curViewScans[scanIndex]; + if (scan.second && scan.second.IsLoaded()) { if (category[scanIndex].second) { - if (scan.second->GetStringCount() > 1) + if (scan.second->GetStringCount() > 1) { string = scan.second->GetString(1); - else + } else { string = u"No Title!"; + } } else { string = u"??????"; } } - if (string.empty()) + if (string.empty()) { string = u"........"; + } } + xd8_textpane_titles[i]->TextSupport().SetText(string); } - int rightSelMod = x18_firstViewRightSel % 5; - int rightSelRem = 5 - rightSelMod; + const int rightSelMod = x18_firstViewRightSel % 5; + const int rightSelRem = 5 - rightSelMod; for (size_t i = 0; i < x144_model_titles.size(); ++i) { - float zOff = ((i >= rightSelMod) ? rightSelRem - 5 : rightSelRem) * x38_highlightPitch; + const float zOff = float(((int(i) >= rightSelMod) ? rightSelRem - 5 : rightSelRem)) * x38_highlightPitch; x144_model_titles[i]->SetLocalTransform(zeus::CTransform::Translate(0.f, 0.f, zOff) * x144_model_titles[i]->GetTransform()); } @@ -212,36 +217,48 @@ void CLogBookScreen::UpdateBodyImagesAndText() { int CLogBookScreen::NextSurroundingArticleIndex(int cur) const { if (cur < x18_firstViewRightSel) { - int tmp = x18_firstViewRightSel + (x18_firstViewRightSel - cur + 6); - if (tmp >= x1f0_curViewScans.size()) + const int tmp = x18_firstViewRightSel + (x18_firstViewRightSel - cur + 6); + + if (tmp >= int(x1f0_curViewScans.size())) { return cur - 1; - else - return tmp; + } + + return tmp; } if (cur < x18_firstViewRightSel + 6) { - if (cur + 1 < x1f0_curViewScans.size()) + if (cur + 1 < int(x1f0_curViewScans.size())) { return cur + 1; - if (x18_firstViewRightSel == 0) + } + + if (x18_firstViewRightSel == 0) { return -1; + } + return x18_firstViewRightSel - 1; } - int tmp = x18_firstViewRightSel - (cur - (x18_firstViewRightSel + 5)); - if (tmp >= 0) + const int tmp = x18_firstViewRightSel - (cur - (x18_firstViewRightSel + 5)); + if (tmp >= 0) { return tmp; + } - if (cur >= x1f0_curViewScans.size() - 1) + if (cur >= int(x1f0_curViewScans.size()) - 1) { return -1; + } + return cur + 1; } bool CLogBookScreen::IsArtifactCategorySelected() const { return x70_tablegroup_leftlog->GetUserSelection() == 4; } int CLogBookScreen::GetSelectedArtifactHeadScanIndex() const { - auto& category = x19c_scanCompletes[x70_tablegroup_leftlog->GetUserSelection()]; - if (x1c_rightSel < category.size()) + const auto& category = x19c_scanCompletes[x70_tablegroup_leftlog->GetUserSelection()]; + + if (x1c_rightSel < int(category.size())) { return CArtifactDoll::GetArtifactHeadScanIndex(category[x1c_rightSel].first); + } + return -1; } From 251a2a77239f555a811cb24c92e317633a7df83d Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 15 Oct 2019 09:40:05 -0400 Subject: [PATCH 4/9] CLogBookScreen: Make use of structured bindings where applicable Allows decomposing long pair names into their constituent elements --- Runtime/MP1/CLogBookScreen.cpp | 46 ++++++++++++++++++---------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/Runtime/MP1/CLogBookScreen.cpp b/Runtime/MP1/CLogBookScreen.cpp index 00eefbe38..cab126582 100644 --- a/Runtime/MP1/CLogBookScreen.cpp +++ b/Runtime/MP1/CLogBookScreen.cpp @@ -106,12 +106,11 @@ void CLogBookScreen::UpdateRightTitles() { void CLogBookScreen::PumpArticleLoad() { x260_24_loaded = true; - for (std::vector, TLockedToken>>& category : - x200_viewScans) { - for (std::pair, TLockedToken>& scan : category) { - if (scan.first.IsLoaded()) { - if (!scan.second) { - scan.second = g_SimplePool->GetObj({FOURCC('STRG'), scan.first->GetStringTableId()}); + for (auto& category : x200_viewScans) { + for (auto& [scanInfo, stringTable] : category) { + if (scanInfo.IsLoaded()) { + if (!stringTable) { + stringTable = g_SimplePool->GetObj({FOURCC('STRG'), scanInfo->GetStringTableId()}); x260_24_loaded = false; } } else { @@ -121,14 +120,14 @@ void CLogBookScreen::PumpArticleLoad() { } int rem = 6; - for (std::pair, TCachedToken>& scan : x1f0_curViewScans) { - if (scan.first.IsLoaded()) { - if (!scan.second) { - scan.second = g_SimplePool->GetObj({FOURCC('STRG'), scan.first->GetStringTableId()}); - scan.second.Lock(); + for (auto& [scanInfo, stringTable] : x1f0_curViewScans) { + if (scanInfo.IsLoaded()) { + if (!stringTable) { + stringTable = g_SimplePool->GetObj({FOURCC('STRG'), scanInfo->GetStringTableId()}); + stringTable.Lock(); --rem; } - } else if (scan.first.IsLocked()) { + } else if (scanInfo.IsLocked()) { --rem; } if (rem == 0) @@ -146,13 +145,16 @@ void CLogBookScreen::PumpArticleLoad() { } } - for (std::pair, TCachedToken>& scan : x1f0_curViewScans) { - if (scan.first.IsLoaded()) { - if (scan.second && scan.second.IsLoaded()) { - UpdateRightTitles(); - UpdateBodyText(); - } + for (const auto& [scanInfo, stringTable] : x1f0_curViewScans) { + if (!scanInfo.IsLoaded()) { + continue; } + if (!stringTable || !stringTable.IsLoaded()) { + continue; + } + + UpdateRightTitles(); + UpdateBodyText(); } } @@ -397,8 +399,9 @@ void CLogBookScreen::ChangedMode(EMode oldMode) { void CLogBookScreen::UpdateRightTable() { CPauseScreenBase::UpdateRightTable(); + + const auto& category = x19c_scanCompletes[x70_tablegroup_leftlog->GetUserSelection()]; x1f0_curViewScans.clear(); - std::vector>& category = x19c_scanCompletes[x70_tablegroup_leftlog->GetUserSelection()]; x1f0_curViewScans.reserve(category.size()); for (const std::pair& scan : category) { x1f0_curViewScans.emplace_back(g_SimplePool->GetObj({FOURCC('SCAN'), scan.first}), TLockedToken{}); @@ -415,9 +418,8 @@ bool CLogBookScreen::ShouldLeftTableAdvance() const { } bool CLogBookScreen::ShouldRightTableAdvance() const { - const std::pair, TLockedToken>& scan = - x1f0_curViewScans[x1c_rightSel]; - return scan.first.IsLoaded() && scan.second.IsLoaded(); + const auto& [info, stringTable] = x1f0_curViewScans[x1c_rightSel]; + return info.IsLoaded() && stringTable.IsLoaded(); } u32 CLogBookScreen::GetRightTableCount() const { return x1f0_curViewScans.size(); } From d64824d3e5616c9a91a087691f613b6e96228f9f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 15 Oct 2019 10:00:10 -0400 Subject: [PATCH 5/9] CLogBookScreen: Make use of std::any_of within IsScanCategoryReady() Same thing but collapses into a single return. --- Runtime/MP1/CLogBookScreen.cpp | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/Runtime/MP1/CLogBookScreen.cpp b/Runtime/MP1/CLogBookScreen.cpp index cab126582..2437294c9 100644 --- a/Runtime/MP1/CLogBookScreen.cpp +++ b/Runtime/MP1/CLogBookScreen.cpp @@ -159,16 +159,19 @@ void CLogBookScreen::PumpArticleLoad() { } bool CLogBookScreen::IsScanCategoryReady(CSaveWorld::EScanCategory category) const { - CPlayerState& playerState = *x4_mgr.GetPlayerState(); - for (const std::pair& scanState : g_MemoryCardSys->GetScanStates()) { - if (scanState.second != category) - continue; - if (IsScanComplete(scanState.second, scanState.first, playerState)) - return true; - } - return false; + const CPlayerState& playerState = *x4_mgr.GetPlayerState(); + const auto& scanState = g_MemoryCardSys->GetScanStates(); + + return std::any_of(scanState.cbegin(), scanState.cend(), [category, &playerState](const auto& state) { + if (state.second != category) { + return false; + } + + return IsScanComplete(state.second, state.first, playerState); + }); } + void CLogBookScreen::UpdateBodyText() { if (x10_mode != EMode::TextScroll) { x174_textpane_body->TextSupport().SetText(u""); From b93f7a4ceb24904763e02ef9c4b2eefdf29f08fb Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 15 Oct 2019 10:03:52 -0400 Subject: [PATCH 6/9] CLogBookScreen: Use std::u16string's append() member instead of operator+ Appends to the existing buffer instead of constructing a superfluous temporary. --- Runtime/MP1/CLogBookScreen.cpp | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Runtime/MP1/CLogBookScreen.cpp b/Runtime/MP1/CLogBookScreen.cpp index 2437294c9..b0565c5a0 100644 --- a/Runtime/MP1/CLogBookScreen.cpp +++ b/Runtime/MP1/CLogBookScreen.cpp @@ -187,9 +187,10 @@ void CLogBookScreen::UpdateBodyText() { } if (IsArtifactCategorySelected()) { - int headIdx = GetSelectedArtifactHeadScanIndex(); - if (headIdx >= 0 && g_GameState->GetPlayerState()->HasPowerUp(CPlayerState::EItemType(headIdx + 29))) - accumStr = std::u16string(u"\n\n\n\n\n\n") + g_MainStringTable->GetString(105); + const int headIdx = GetSelectedArtifactHeadScanIndex(); + if (headIdx >= 0 && g_GameState->GetPlayerState()->HasPowerUp(CPlayerState::EItemType(headIdx + 29))) { + accumStr = std::u16string(u"\n\n\n\n\n\n").append(g_MainStringTable->GetString(105)); + } } x174_textpane_body->TextSupport().SetText(accumStr, true); From 4db0e49851362ea70600af0bc885cd1ec464004f Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 15 Oct 2019 10:05:50 -0400 Subject: [PATCH 7/9] CLogBookScreen: Invert conditional within UpdateBodyText() Allows unindenting the contained code by one level. --- Runtime/MP1/CLogBookScreen.cpp | 34 ++++++++++++++++++---------------- 1 file changed, 18 insertions(+), 16 deletions(-) diff --git a/Runtime/MP1/CLogBookScreen.cpp b/Runtime/MP1/CLogBookScreen.cpp index b0565c5a0..2165d4ff2 100644 --- a/Runtime/MP1/CLogBookScreen.cpp +++ b/Runtime/MP1/CLogBookScreen.cpp @@ -178,23 +178,25 @@ void CLogBookScreen::UpdateBodyText() { return; } - TCachedToken& str = x1f0_curViewScans[x1c_rightSel].second; - if (str && str.IsLoaded()) { - std::u16string accumStr = str->GetString(0); - if (str->GetStringCount() > 2) { - accumStr += u"\n\n"; - accumStr += str->GetString(2); - } - - if (IsArtifactCategorySelected()) { - const int headIdx = GetSelectedArtifactHeadScanIndex(); - if (headIdx >= 0 && g_GameState->GetPlayerState()->HasPowerUp(CPlayerState::EItemType(headIdx + 29))) { - accumStr = std::u16string(u"\n\n\n\n\n\n").append(g_MainStringTable->GetString(105)); - } - } - - x174_textpane_body->TextSupport().SetText(accumStr, true); + const TCachedToken& str = x1f0_curViewScans[x1c_rightSel].second; + if (!str || !str.IsLoaded()) { + return; } + + std::u16string accumStr = str->GetString(0); + if (str->GetStringCount() > 2) { + accumStr += u"\n\n"; + accumStr += str->GetString(2); + } + + if (IsArtifactCategorySelected()) { + const int headIdx = GetSelectedArtifactHeadScanIndex(); + if (headIdx >= 0 && g_GameState->GetPlayerState()->HasPowerUp(CPlayerState::EItemType(headIdx + 29))) { + accumStr = std::u16string(u"\n\n\n\n\n\n").append(g_MainStringTable->GetString(105)); + } + } + + x174_textpane_body->TextSupport().SetText(accumStr, true); } void CLogBookScreen::UpdateBodyImagesAndText() { From 5b8f3f6693c09b56c22978c96709cc024b659f16 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 15 Oct 2019 10:13:27 -0400 Subject: [PATCH 8/9] CLogBookScreen: Organize cpp includes Tidies them up to be consistent with its header. --- Runtime/MP1/CLogBookScreen.cpp | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/Runtime/MP1/CLogBookScreen.cpp b/Runtime/MP1/CLogBookScreen.cpp index 2165d4ff2..904a74c6d 100644 --- a/Runtime/MP1/CLogBookScreen.cpp +++ b/Runtime/MP1/CLogBookScreen.cpp @@ -1,9 +1,12 @@ -#include "CLogBookScreen.hpp" -#include "GuiSys/CGuiModel.hpp" -#include "GuiSys/CGuiTableGroup.hpp" -#include "GuiSys/CGuiTextPane.hpp" -#include "GuiSys/CAuiImagePane.hpp" -#include "MP1.hpp" +#include "Runtime/MP1/CLogBookScreen.hpp" + +#include + +#include "Runtime/GuiSys/CAuiImagePane.hpp" +#include "Runtime/GuiSys/CGuiModel.hpp" +#include "Runtime/GuiSys/CGuiTableGroup.hpp" +#include "Runtime/GuiSys/CGuiTextPane.hpp" +#include "Runtime/MP1/MP1.hpp" namespace urde::MP1 { From 3a0daa3ab9f56264abdb1de9071dbb2463b1ade2 Mon Sep 17 00:00:00 2001 From: Lioncash Date: Tue, 15 Oct 2019 10:22:47 -0400 Subject: [PATCH 9/9] CLogBookScreen: Use forward declarations where applicable Allows for avoiding the over exposing of types through the header when included in other translation units or headers. --- Runtime/MP1/CLogBookScreen.cpp | 3 +++ Runtime/MP1/CLogBookScreen.hpp | 11 +++++++++-- 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/Runtime/MP1/CLogBookScreen.cpp b/Runtime/MP1/CLogBookScreen.cpp index 904a74c6d..a459d9c1a 100644 --- a/Runtime/MP1/CLogBookScreen.cpp +++ b/Runtime/MP1/CLogBookScreen.cpp @@ -2,10 +2,13 @@ #include +#include "Runtime/CPlayerState.hpp" +#include "Runtime/CStateManager.hpp" #include "Runtime/GuiSys/CAuiImagePane.hpp" #include "Runtime/GuiSys/CGuiModel.hpp" #include "Runtime/GuiSys/CGuiTableGroup.hpp" #include "Runtime/GuiSys/CGuiTextPane.hpp" +#include "Runtime/MP1/CArtifactDoll.hpp" #include "Runtime/MP1/MP1.hpp" namespace urde::MP1 { diff --git a/Runtime/MP1/CLogBookScreen.hpp b/Runtime/MP1/CLogBookScreen.hpp index c6d2a930d..8b2892b7b 100644 --- a/Runtime/MP1/CLogBookScreen.hpp +++ b/Runtime/MP1/CLogBookScreen.hpp @@ -4,12 +4,19 @@ #include #include +#include "Runtime/CSaveWorld.hpp" +#include "Runtime/CToken.hpp" #include "Runtime/rstl.hpp" -#include "Runtime/MP1/CArtifactDoll.hpp" -#include "Runtime/MP1/CInGameGuiManager.hpp" #include "Runtime/MP1/CPauseScreenBase.hpp" +namespace urde { +class CPlayerState; +class CScannableObjectInfo; +class CStringTable; +} + namespace urde::MP1 { +class CArtifactDoll; class CLogBookScreen : public CPauseScreenBase { rstl::reserved_vector>, 5> x19c_scanCompletes;