From 7da38113be8de6776ac06920992b05f4b81bbd01 Mon Sep 17 00:00:00 2001 From: fteppe Date: Thu, 13 Oct 2022 23:14:00 +0200 Subject: [PATCH] Changed more hardcoded RefId to be static to avoid multiple runtime creations adresses multiple review comments --- apps/openmw/mwbase/windowmanager.hpp | 2 +- apps/openmw/mwclass/clothing.cpp | 16 +++++----- apps/openmw/mwclass/creature.cpp | 3 +- apps/openmw/mwclass/door.cpp | 4 +-- apps/openmw/mwclass/misc.cpp | 29 ++++++++++--------- apps/openmw/mwclass/npc.cpp | 3 +- apps/openmw/mwdialogue/filter.cpp | 2 +- apps/openmw/mwdialogue/topic.cpp | 2 +- apps/openmw/mwdialogue/topic.hpp | 2 +- apps/openmw/mwgui/charactercreation.cpp | 2 +- apps/openmw/mwgui/statswindow.cpp | 2 +- apps/openmw/mwgui/travelwindow.cpp | 2 +- apps/openmw/mwmechanics/character.cpp | 3 +- apps/openmw/mwmechanics/combat.cpp | 3 +- .../mwmechanics/mechanicsmanagerimp.cpp | 3 +- apps/openmw/mwmechanics/recharge.cpp | 2 +- apps/openmw/mwscript/containerextensions.cpp | 12 ++++---- apps/openmw/mwworld/class.cpp | 3 +- apps/openmw/mwworld/containerstore.cpp | 3 +- apps/openmw/mwworld/store.cpp | 2 +- apps/openmw/mwworld/worldimp.cpp | 4 +-- components/esm/refid.cpp | 2 +- components/esm/refidhardcoded.cpp | 1 + components/esm/refidhardcoded.hpp | 2 +- components/misc/resourcehelpers.cpp | 8 +++-- 25 files changed, 65 insertions(+), 52 deletions(-) diff --git a/apps/openmw/mwbase/windowmanager.hpp b/apps/openmw/mwbase/windowmanager.hpp index 2f1de26441..e80f3c9d31 100644 --- a/apps/openmw/mwbase/windowmanager.hpp +++ b/apps/openmw/mwbase/windowmanager.hpp @@ -273,7 +273,7 @@ namespace MWBase * @param id Identifier for the GMST setting, e.g. "aName" * @param default Default value if the GMST setting cannot be used. */ - virtual std::string_view getGameSettingString(const std::string_view& id, std::string_view default_) = 0; + virtual std::string_view getGameSettingString(std::string_view id, std::string_view default_) = 0; virtual void processChangedSettings(const std::set>& changed) = 0; diff --git a/apps/openmw/mwclass/clothing.cpp b/apps/openmw/mwclass/clothing.cpp index 0a19ba6bec..119b3aaeca 100644 --- a/apps/openmw/mwclass/clothing.cpp +++ b/apps/openmw/mwclass/clothing.cpp @@ -121,25 +121,25 @@ namespace MWClass const ESM::RefId& Clothing::getUpSoundId(const MWWorld::ConstPtr& ptr) const { const MWWorld::LiveCellRef* ref = ptr.get(); - static ESM::RefId sound; + static const ESM::RefId ringUp = ESM::RefId::stringRefId("Item Ring Up"); + static const ESM::RefId clothsUp = ESM::RefId::stringRefId("Item Clothes Up"); if (ref->mBase->mData.mType == 8) { - sound = ESM::RefId::stringRefId("Item Ring Up"); + return ringUp; } - sound = ESM::RefId::stringRefId("Item Clothes Up"); - return sound; + return clothsUp; } const ESM::RefId& Clothing::getDownSoundId(const MWWorld::ConstPtr& ptr) const { const MWWorld::LiveCellRef* ref = ptr.get(); - static ESM::RefId sound; + static const ESM::RefId ringDown = ESM::RefId::stringRefId("Item Ring Down"); + static const ESM::RefId clothsDown = ESM::RefId::stringRefId("Item Clothes Down"); if (ref->mBase->mData.mType == 8) { - sound = ESM::RefId::stringRefId("Item Ring Down"); + return ringDown; } - sound = ESM::RefId::stringRefId("Item Clothes Down"); - return sound; + return clothsDown; } const std::string& Clothing::getInventoryIcon(const MWWorld::ConstPtr& ptr) const diff --git a/apps/openmw/mwclass/creature.cpp b/apps/openmw/mwclass/creature.cpp index 38aef45d85..11e7ff2af7 100644 --- a/apps/openmw/mwclass/creature.cpp +++ b/apps/openmw/mwclass/creature.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include "../mwmechanics/actorutil.hpp" #include "../mwmechanics/aisetting.hpp" @@ -448,7 +449,7 @@ namespace MWClass MWBase::Environment::get().getWorld()->spawnBloodEffect(ptr, hitPosition); } - MWBase::Environment::get().getSoundManager()->playSound3D(ptr, ESM::RefId::stringRefId("Health Damage"), 1.0f, 1.0f); + MWBase::Environment::get().getSoundManager()->playSound3D(ptr, ESM::sHealthDamageSoundId, 1.0f, 1.0f); MWMechanics::DynamicStat health(stats.getHealth()); health.setCurrent(health.getCurrent() - damage); diff --git a/apps/openmw/mwclass/door.cpp b/apps/openmw/mwclass/door.cpp index 5469bc95cd..e0c9a32f30 100644 --- a/apps/openmw/mwclass/door.cpp +++ b/apps/openmw/mwclass/door.cpp @@ -113,8 +113,8 @@ namespace MWClass const ESM::RefId& openSound = ref->mBase->mOpenSound; const ESM::RefId& closeSound = ref->mBase->mCloseSound; - const ESM::RefId lockedSound = ESM::RefId::stringRefId("LockedDoor"); - const ESM::RefId trapActivationSound = ESM::RefId::stringRefId("Disarm Trap Fail"); + static const ESM::RefId lockedSound = ESM::RefId::stringRefId("LockedDoor"); + static const ESM::RefId trapActivationSound = ESM::RefId::stringRefId("Disarm Trap Fail"); // FIXME: If NPC activate teleporting door, it can lead to crash due to iterator invalidation in the Actors // update. Make such activation a no-op for now, like how it is in the vanilla game. diff --git a/apps/openmw/mwclass/misc.cpp b/apps/openmw/mwclass/misc.cpp index df987d7632..7b530d379a 100644 --- a/apps/openmw/mwclass/misc.cpp +++ b/apps/openmw/mwclass/misc.cpp @@ -113,20 +113,21 @@ namespace MWClass const ESM::RefId& Miscellaneous::getUpSoundId(const MWWorld::ConstPtr& ptr) const { - static ESM::RefId sound; + static const ESM::RefId soundGold = ESM::RefId::stringRefId("Item Gold Up"); + static const ESM::RefId soundMisc = ESM::RefId::stringRefId("Item Misc Up"); if (isGold(ptr)) - sound = ESM::RefId::stringRefId("Item Gold Up"); - sound = ESM::RefId::stringRefId("Item Misc Up"); - return sound; + return soundGold; + + return soundMisc; } const ESM::RefId& Miscellaneous::getDownSoundId(const MWWorld::ConstPtr& ptr) const { - static ESM::RefId sound; + static const ESM::RefId soundGold = ESM::RefId::stringRefId("Item Gold Down"); + static const ESM::RefId soundMisc = ESM::RefId::stringRefId("Item Misc Down"); if (isGold(ptr)) - sound = ESM::RefId::stringRefId("Item Gold Down"); - sound = ESM::RefId::stringRefId("Item Misc Down"); - return sound; + return soundGold; + return soundMisc; } const std::string& Miscellaneous::getInventoryIcon(const MWWorld::ConstPtr& ptr) const @@ -184,19 +185,19 @@ namespace MWClass { int goldAmount = getValue(ptr) * count; - std::string_view base = "Gold_001"; + const ESM::RefId* base = &ESM::sGoldId001; if (goldAmount >= 100) - base = "Gold_100"; + base = &ESM::sGoldId100; else if (goldAmount >= 25) - base = "Gold_025"; + base = &ESM::sGoldId025; else if (goldAmount >= 10) - base = "Gold_010"; + base = &ESM::sGoldId010; else if (goldAmount >= 5) - base = "Gold_005"; + base = &ESM::sGoldId005; // Really, I have no idea why moving ref out of conditional // scope causes list::push_back throwing std::bad_alloc - MWWorld::ManualRef newRef(store, ESM::RefId::stringRefId(base)); + MWWorld::ManualRef newRef(store, *base); const MWWorld::LiveCellRef* ref = newRef.getPtr().get(); newPtr = MWWorld::Ptr(cell.insert(ref), &cell); diff --git a/apps/openmw/mwclass/npc.cpp b/apps/openmw/mwclass/npc.cpp index 11a5db3529..a2e750f4e2 100644 --- a/apps/openmw/mwclass/npc.cpp +++ b/apps/openmw/mwclass/npc.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include #include @@ -892,7 +893,7 @@ namespace MWClass if (damage > 0.0f) { - sndMgr->playSound3D(ptr, ESM::RefId::stringRefId("Health Damage"), 1.0f, 1.0f); + sndMgr->playSound3D(ptr, ESM::sHealthDamageSoundId, 1.0f, 1.0f); if (ptr == MWMechanics::getPlayer()) MWBase::Environment::get().getWindowManager()->activateHitOverlay(); if (!attacker.isEmpty()) diff --git a/apps/openmw/mwdialogue/filter.cpp b/apps/openmw/mwdialogue/filter.cpp index df04e6b30f..efe92d13fa 100644 --- a/apps/openmw/mwdialogue/filter.cpp +++ b/apps/openmw/mwdialogue/filter.cpp @@ -84,7 +84,7 @@ bool MWDialogue::Filter::testActor(const ESM::DialInfo& info) const // actor id if (!info.mActor.empty()) { - if (!(info.mActor == mActor.getCellRef().getRefId())) + if (info.mActor != mActor.getCellRef().getRefId()) return false; } else if (isCreature) diff --git a/apps/openmw/mwdialogue/topic.cpp b/apps/openmw/mwdialogue/topic.cpp index d9caa0a41b..7d16d948d0 100644 --- a/apps/openmw/mwdialogue/topic.cpp +++ b/apps/openmw/mwdialogue/topic.cpp @@ -38,7 +38,7 @@ namespace MWDialogue mEntries.push_back(entry); } - ESM::RefId Topic::getTopic() const + const ESM::RefId& Topic::getTopic() const { return mTopic; } diff --git a/apps/openmw/mwdialogue/topic.hpp b/apps/openmw/mwdialogue/topic.hpp index 79b59528fa..d95d2470c7 100644 --- a/apps/openmw/mwdialogue/topic.hpp +++ b/apps/openmw/mwdialogue/topic.hpp @@ -42,7 +42,7 @@ namespace MWDialogue ///< Add entry without checking for redundant entries or modifying the state of the /// topic otherwise - ESM::RefId getTopic() const; + const ESM::RefId& getTopic() const; virtual std::string_view getName() const; diff --git a/apps/openmw/mwgui/charactercreation.cpp b/apps/openmw/mwgui/charactercreation.cpp index 3b24c10b12..fed0d88ff6 100644 --- a/apps/openmw/mwgui/charactercreation.cpp +++ b/apps/openmw/mwgui/charactercreation.cpp @@ -551,7 +551,7 @@ namespace MWGui unsigned combat = mGenerateClassSpecializations[0]; unsigned magic = mGenerateClassSpecializations[1]; unsigned stealth = mGenerateClassSpecializations[2]; - std::string className; + std::string_view className; if (combat > 7) { className = "Warrior"; diff --git a/apps/openmw/mwgui/statswindow.cpp b/apps/openmw/mwgui/statswindow.cpp index 6f56f83427..c1ee2136a5 100644 --- a/apps/openmw/mwgui/statswindow.cpp +++ b/apps/openmw/mwgui/statswindow.cpp @@ -376,7 +376,7 @@ namespace MWGui setFactions(PCstats.getFactionRanks()); setExpelled(PCstats.getExpelled()); - auto signId = MWBase::Environment::get().getWorld()->getPlayer().getBirthSign(); + const auto& signId = MWBase::Environment::get().getWorld()->getPlayer().getBirthSign(); setBirthSign(signId); setReputation(PCstats.getReputation()); diff --git a/apps/openmw/mwgui/travelwindow.cpp b/apps/openmw/mwgui/travelwindow.cpp index 686cf8f465..902a5cde3d 100644 --- a/apps/openmw/mwgui/travelwindow.cpp +++ b/apps/openmw/mwgui/travelwindow.cpp @@ -90,7 +90,7 @@ namespace MWGui else toAdd->setUserString("interior", "n"); - std::string nameString = name.getRefIdString(); + const std::string& nameString = name.getRefIdString(); toAdd->setUserString("price", std::to_string(price)); toAdd->setCaptionWithReplacing("#{sCell=" + nameString + "} - " + MyGUI::utility::toString(price) + "#{sgp}"); toAdd->setSize(mDestinationsView->getWidth(), lineHeight); diff --git a/apps/openmw/mwmechanics/character.cpp b/apps/openmw/mwmechanics/character.cpp index 884de9b147..c9214d0b90 100644 --- a/apps/openmw/mwmechanics/character.cpp +++ b/apps/openmw/mwmechanics/character.cpp @@ -21,6 +21,7 @@ #include +#include #include #include #include @@ -2137,7 +2138,7 @@ namespace MWMechanics float realHealthLost = healthLost * (1.0f - 0.25f * fatigueTerm); health.setCurrent(health.getCurrent() - realHealthLost); cls.getCreatureStats(mPtr).setHealth(health); - sndMgr->playSound3D(mPtr, ESM::RefId::stringRefId("Health Damage"), 1.0f, 1.0f); + sndMgr->playSound3D(mPtr, ESM::sHealthDamageSoundId, 1.0f, 1.0f); if (isPlayer) MWBase::Environment::get().getWindowManager()->activateHitOverlay(); } diff --git a/apps/openmw/mwmechanics/combat.cpp b/apps/openmw/mwmechanics/combat.cpp index 755d3eb2b9..0e5418e32c 100644 --- a/apps/openmw/mwmechanics/combat.cpp +++ b/apps/openmw/mwmechanics/combat.cpp @@ -9,6 +9,7 @@ #include #include #include +#include #include "../mwbase/environment.hpp" #include "../mwbase/mechanicsmanager.hpp" @@ -382,7 +383,7 @@ namespace MWMechanics health.setCurrent(health.getCurrent() - x); attackerStats.setHealth(health); - MWBase::Environment::get().getSoundManager()->playSound3D(attacker, ESM::RefId::stringRefId("Health Damage"), 1.0f, 1.0f); + MWBase::Environment::get().getSoundManager()->playSound3D(attacker, ESM::sHealthDamageSoundId, 1.0f, 1.0f); } } diff --git a/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp b/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp index 9e4050ca8e..0ff88c7d61 100644 --- a/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp +++ b/apps/openmw/mwmechanics/mechanicsmanagerimp.cpp @@ -914,7 +914,8 @@ namespace MWMechanics return false; // A special case for evidence chest - we should not allow to take items even if it is technically permitted - return !(cellref.getRefId() == ESM::RefId::stringRefId("stolen_goods")); + static const ESM::RefId stolenGoods = ESM::RefId::stringRefId("stolen_goods"); + return !(cellref.getRefId() == stolenGoods); } bool MechanicsManager::sleepInBed(const MWWorld::Ptr& ptr, const MWWorld::Ptr& bed) diff --git a/apps/openmw/mwmechanics/recharge.cpp b/apps/openmw/mwmechanics/recharge.cpp index db5361ef69..6394a617e1 100644 --- a/apps/openmw/mwmechanics/recharge.cpp +++ b/apps/openmw/mwmechanics/recharge.cpp @@ -97,7 +97,7 @@ namespace MWMechanics MWBase::Environment::get().getWindowManager()->messageBox(message); - ESM::RefId soulGemAzura = ESM::RefId::stringRefId("Misc_SoulGem_Azura"); + static const ESM::RefId soulGemAzura = ESM::RefId::stringRefId("Misc_SoulGem_Azura"); // special case: readd Azura's Star if (gem.get()->mBase->mId == soulGemAzura) player.getClass().getContainerStore(player).add(soulGemAzura, 1, player); diff --git a/apps/openmw/mwscript/containerextensions.cpp b/apps/openmw/mwscript/containerextensions.cpp index d93b4acc6a..9c9e729078 100644 --- a/apps/openmw/mwscript/containerextensions.cpp +++ b/apps/openmw/mwscript/containerextensions.cpp @@ -191,9 +191,9 @@ namespace MWScript ESM::RefId item = ESM::RefId::stringRefId(runtime.getStringLiteral(runtime[0].mInteger)); runtime.pop(); - if (item == ESM::RefId::stringRefId("gold_005") ||item == ESM::RefId::stringRefId("gold_010") - || item == ESM::RefId::stringRefId("gold_025") ||item == ESM::RefId::stringRefId("gold_100")) - item = ESM::RefId::stringRefId("gold_001"); + if (item == ESM::sGoldId005 || item == ESM::sGoldId010 + || item == ESM::sGoldId025 || item == ESM::sGoldId100) + item = ESM::sGoldId001; MWWorld::ContainerStore& store = ptr.getClass().getContainerStore(ptr); @@ -222,9 +222,9 @@ namespace MWScript if (count == 0) return; - if (item == ESM::RefId::stringRefId("gold_005") || item == ESM::RefId::stringRefId("gold_010") - || item == ESM::RefId::stringRefId("gold_025") || item == ESM::RefId::stringRefId("gold_100")) - item = ESM::RefId::stringRefId("gold_001"); + if (item == ESM::sGoldId005 || item == ESM::sGoldId010 + || item == ESM::sGoldId025 || item == ESM::sGoldId100) + item = ESM::sGoldId001; // Explicit calls to non-unique actors affect the base record if (!R::implicit && ptr.getClass().isActor() diff --git a/apps/openmw/mwworld/class.cpp b/apps/openmw/mwworld/class.cpp index 5e0aa27a89..cf1f223b9a 100644 --- a/apps/openmw/mwworld/class.cpp +++ b/apps/openmw/mwworld/class.cpp @@ -293,7 +293,8 @@ namespace MWWorld // NOTE: Don't show WerewolfRobe objects in the inventory, or allow them to be taken. // Vanilla likely uses a hack like this since there's no other way to prevent it from // being shown or taken. - return (ptr.getCellRef().getRefId() != ESM::RefId::stringRefId("werewolfrobe")); + static const ESM::RefId werewolfrobe = ESM::RefId::stringRefId("werewolfrobe"); + return (ptr.getCellRef().getRefId() != werewolfrobe); } bool Class::hasToolTip(const ConstPtr& ptr) const diff --git a/apps/openmw/mwworld/containerstore.cpp b/apps/openmw/mwworld/containerstore.cpp index 33b8894a54..d6c884d4a6 100644 --- a/apps/openmw/mwworld/containerstore.cpp +++ b/apps/openmw/mwworld/containerstore.cpp @@ -6,6 +6,7 @@ #include #include #include +#include #include #include #include @@ -137,7 +138,7 @@ void MWWorld::ContainerStore::storeStates( } } -const ESM::RefId MWWorld::ContainerStore::sGoldId = ESM::RefId::stringRefId("gold_001"); +const ESM::RefId MWWorld::ContainerStore::sGoldId = ESM::sGoldId001; MWWorld::ContainerStore::ContainerStore() : mListener(nullptr) diff --git a/apps/openmw/mwworld/store.cpp b/apps/openmw/mwworld/store.cpp index 8f776bd861..7d93da7dc1 100644 --- a/apps/openmw/mwworld/store.cpp +++ b/apps/openmw/mwworld/store.cpp @@ -435,7 +435,7 @@ namespace MWWorld else mStatic.insert(it, std::move(land)); - return RecordId(ESM::RefId::stringRefId(""), isDeleted); + return RecordId(ESM::RefId::sEmpty, isDeleted); } void Store::setUp() { diff --git a/apps/openmw/mwworld/worldimp.cpp b/apps/openmw/mwworld/worldimp.cpp index bd6551f74d..cdebd414b1 100644 --- a/apps/openmw/mwworld/worldimp.cpp +++ b/apps/openmw/mwworld/worldimp.cpp @@ -2635,7 +2635,7 @@ namespace MWWorld if (actor == getPlayerPtr()) MWBase::Environment::get().getWindowManager()->activateHitOverlay(false); - auto healthDamage = ESM::RefId::stringRefId("Health Damage"); + auto healthDamage = ESM::sHealthDamageSoundId; if (!MWBase::Environment::get().getSoundManager()->getSoundPlaying(actor,healthDamage)) MWBase::Environment::get().getSoundManager()->playSound3D(actor, healthDamage, 1.0f, 1.0f); } @@ -2669,7 +2669,7 @@ namespace MWWorld if (actor == getPlayerPtr()) MWBase::Environment::get().getWindowManager()->activateHitOverlay(false); - auto healthDamage = ESM::RefId::stringRefId("Health Damage"); + auto healthDamage = ESM::sHealthDamageSoundId; if (!MWBase::Environment::get().getSoundManager()->getSoundPlaying(actor, healthDamage )) MWBase::Environment::get().getSoundManager()->playSound3D(actor, healthDamage, 1.0f, 1.0f); } diff --git a/components/esm/refid.cpp b/components/esm/refid.cpp index c7507182fa..5bac30f8a4 100644 --- a/components/esm/refid.cpp +++ b/components/esm/refid.cpp @@ -26,7 +26,7 @@ namespace ESM return newRefId; } - const RefId RefId::sEmpty = RefId::stringRefId(""); + const RefId RefId::sEmpty = {}; } diff --git a/components/esm/refidhardcoded.cpp b/components/esm/refidhardcoded.cpp index e311a0b5ba..964750d9ae 100644 --- a/components/esm/refidhardcoded.cpp +++ b/components/esm/refidhardcoded.cpp @@ -10,6 +10,7 @@ namespace ESM const RefId sGoldId010 = ESM::RefId::stringRefId("gold_010"); const RefId sGoldId025 = ESM::RefId::stringRefId("gold_025"); const RefId sGoldId100 = ESM::RefId::stringRefId("gold_100"); + const RefId sHealthDamageSoundId = ESM::RefId::stringRefId("Health Damage"); } diff --git a/components/esm/refidhardcoded.hpp b/components/esm/refidhardcoded.hpp index 2c9024a1f8..e15159328a 100644 --- a/components/esm/refidhardcoded.hpp +++ b/components/esm/refidhardcoded.hpp @@ -4,7 +4,7 @@ namespace ESM { - extern const RefId sPlayerId, sMenuClickSoundId, sBookPageSoundId; + extern const RefId sPlayerId, sMenuClickSoundId, sBookPageSoundId, sHealthDamageSoundId; extern const RefId sGoldId001, sGoldId005, sGoldId010, sGoldId025, sGoldId100; } diff --git a/components/misc/resourcehelpers.cpp b/components/misc/resourcehelpers.cpp index 059f7976b6..7d9567bab5 100644 --- a/components/misc/resourcehelpers.cpp +++ b/components/misc/resourcehelpers.cpp @@ -160,8 +160,12 @@ std::string Misc::ResourceHelpers::correctSoundPath(std::string_view resPath, co bool Misc::ResourceHelpers::isHiddenMarker(const ESM::RefId& id) { - return id == ESM::RefId::stringRefId("prisonmarker") ||id == ESM::RefId::stringRefId("divinemarker") - || id == ESM::RefId::stringRefId("templemarker") || id == ESM::RefId::stringRefId("northmarker"); + static const ESM::RefId prisonMarker = ESM::RefId::stringRefId("prisonmarker"); + static const ESM::RefId divineMarker = ESM::RefId::stringRefId("divinemarker"); + static const ESM::RefId templeMarker = ESM::RefId::stringRefId("templemarker"); + static const ESM::RefId northMarker = ESM::RefId::stringRefId("northmarker"); + + return id == prisonMarker || id == divineMarker || id == templeMarker || id == northMarker; } namespace