From 67de61e1fb3bf14ef0b5f6bb1cecc3b6b4d3d363 Mon Sep 17 00:00:00 2001 From: Capostrophic Date: Wed, 24 Oct 2018 18:51:34 +0300 Subject: [PATCH 1/3] Avoid item condition and charge zero divisions --- apps/openmw/mwclass/armor.cpp | 2 +- apps/openmw/mwclass/npc.cpp | 11 +++++- apps/openmw/mwclass/weapon.cpp | 2 +- apps/openmw/mwgui/merchantrepair.cpp | 2 +- apps/openmw/mwgui/sortfilteritemmodel.cpp | 48 +++++++++++++++++++++-- apps/openmw/mwgui/tradewindow.cpp | 11 +++++- apps/openmw/mwgui/windowmanagerimp.cpp | 29 ++++++++++++-- apps/openmw/mwmechanics/combat.cpp | 11 +++++- 8 files changed, 100 insertions(+), 16 deletions(-) diff --git a/apps/openmw/mwclass/armor.cpp b/apps/openmw/mwclass/armor.cpp index b90c1ec58..ad64ad0d1 100644 --- a/apps/openmw/mwclass/armor.cpp +++ b/apps/openmw/mwclass/armor.cpp @@ -297,7 +297,7 @@ namespace MWClass { const MWWorld::InventoryStore& invStore = npc.getClass().getInventoryStore(npc); - if (ptr.getCellRef().getCharge() == 0) + if (getItemHealth(ptr) == 0) return std::make_pair(0, "#{sInventoryMessage1}"); // slots that this item can be equipped in diff --git a/apps/openmw/mwclass/npc.cpp b/apps/openmw/mwclass/npc.cpp index d6dafd2a2..f7172ac0b 100644 --- a/apps/openmw/mwclass/npc.cpp +++ b/apps/openmw/mwclass/npc.cpp @@ -1148,9 +1148,16 @@ namespace MWClass const bool hasHealth = it->getClass().hasItemHealth(*it); if (hasHealth) { - int armorHealth = it->getClass().getItemHealth(*it); int armorMaxHealth = it->getClass().getItemMaxHealth(*it); - ratings[i] *= (float(armorHealth) / armorMaxHealth); + if (armorMaxHealth == 0) + { + ratings[i] = 0; + } + else + { + int armorHealth = it->getClass().getItemHealth(*it); + ratings[i] *= (float(armorHealth) / armorMaxHealth); + } } } } diff --git a/apps/openmw/mwclass/weapon.cpp b/apps/openmw/mwclass/weapon.cpp index 78678f461..7d28c8983 100644 --- a/apps/openmw/mwclass/weapon.cpp +++ b/apps/openmw/mwclass/weapon.cpp @@ -383,7 +383,7 @@ namespace MWClass std::pair Weapon::canBeEquipped(const MWWorld::ConstPtr &ptr, const MWWorld::Ptr &npc) const { - if (hasItemHealth(ptr) && ptr.getCellRef().getCharge() == 0) + if (hasItemHealth(ptr) && getItemHealth(ptr) == 0) return std::make_pair(0, "#{sInventoryMessage1}"); // Do not allow equip weapons from inventory during attack diff --git a/apps/openmw/mwgui/merchantrepair.cpp b/apps/openmw/mwgui/merchantrepair.cpp index b9d4c80f4..282c0e4ea 100644 --- a/apps/openmw/mwgui/merchantrepair.cpp +++ b/apps/openmw/mwgui/merchantrepair.cpp @@ -51,7 +51,7 @@ void MerchantRepair::setPtr(const MWWorld::Ptr &actor) { int maxDurability = iter->getClass().getItemMaxHealth(*iter); int durability = iter->getClass().getItemHealth(*iter); - if (maxDurability == durability) + if (maxDurability == durability || maxDurability == 0) continue; int basePrice = iter->getClass().getValue(*iter); diff --git a/apps/openmw/mwgui/sortfilteritemmodel.cpp b/apps/openmw/mwgui/sortfilteritemmodel.cpp index fe7f61952..5172c85ee 100644 --- a/apps/openmw/mwgui/sortfilteritemmodel.cpp +++ b/apps/openmw/mwgui/sortfilteritemmodel.cpp @@ -86,26 +86,66 @@ namespace if (!leftName.empty()) { const ESM::Enchantment* ench = MWBase::Environment::get().getWorld()->getStore().get().search(leftName); + if (ench) { if (ench->mData.mType == ESM::Enchantment::ConstantEffect) + { leftChargePercent = 101; + } else - leftChargePercent = (left.mBase.getCellRef().getEnchantmentCharge() == -1) ? 100 - : static_cast(left.mBase.getCellRef().getEnchantmentCharge() / static_cast(ench->mData.mCharge) * 100); + { + int maxEnchCharge = ench->mData.mCharge; + if (maxEnchCharge == 0) + { + leftChargePercent = 0; + } + else + { + float enchCharge = left.mBase.getCellRef().getEnchantmentCharge(); + if (enchCharge == -1) + { + leftChargePercent = 100; + } + else + { + leftChargePercent = static_cast(enchCharge / static_cast(maxEnchCharge) * 100); + } + } + } } } if (!rightName.empty()) { const ESM::Enchantment* ench = MWBase::Environment::get().getWorld()->getStore().get().search(rightName); + if (ench) { if (ench->mData.mType == ESM::Enchantment::ConstantEffect) + { rightChargePercent = 101; + } else - rightChargePercent = (right.mBase.getCellRef().getEnchantmentCharge() == -1) ? 100 - : static_cast(right.mBase.getCellRef().getEnchantmentCharge() / static_cast(ench->mData.mCharge) * 100); + { + int maxEnchCharge = ench->mData.mCharge; + if (maxEnchCharge == 0) + { + rightChargePercent = 0; + } + else + { + float enchCharge = right.mBase.getCellRef().getEnchantmentCharge(); + if (enchCharge == -1) + { + rightChargePercent = 100; + } + else + { + rightChargePercent = static_cast(enchCharge / static_cast(maxEnchCharge) * 100); + } + } + } } } diff --git a/apps/openmw/mwgui/tradewindow.cpp b/apps/openmw/mwgui/tradewindow.cpp index 4404b2b1a..2fd91fd4a 100644 --- a/apps/openmw/mwgui/tradewindow.cpp +++ b/apps/openmw/mwgui/tradewindow.cpp @@ -35,8 +35,15 @@ namespace float price = static_cast(item.getClass().getValue(item)); if (item.getClass().hasItemHealth(item)) { - price *= item.getClass().getItemHealth(item); - price /= item.getClass().getItemMaxHealth(item); + if (item.getClass().getItemMaxHealth(item) == 0) + { + price = 0; + } + else + { + price *= item.getClass().getItemHealth(item); + price /= item.getClass().getItemMaxHealth(item); + } } return static_cast(price * count); } diff --git a/apps/openmw/mwgui/windowmanagerimp.cpp b/apps/openmw/mwgui/windowmanagerimp.cpp index e4515fdc3..3b26edecb 100644 --- a/apps/openmw/mwgui/windowmanagerimp.cpp +++ b/apps/openmw/mwgui/windowmanagerimp.cpp @@ -1369,8 +1369,22 @@ namespace MWGui const ESM::Enchantment* ench = mStore->get() .find(item.getClass().getEnchantment(item)); - int chargePercent = (item.getCellRef().getEnchantmentCharge() == -1) ? 100 - : static_cast(item.getCellRef().getEnchantmentCharge() / static_cast(ench->mData.mCharge) * 100); + int chargePercent = 100; + + int maxEnchCharge = ench->mData.mCharge; + if (maxEnchCharge == 0) + { + chargePercent = 0; + } + else + { + float enchCharge = item.getCellRef().getEnchantmentCharge(); + if (enchCharge != -1) + { + chargePercent = static_cast(enchCharge / static_cast(maxEnchCharge) * 100); + } + } + mHud->setSelectedEnchantItem(item, chargePercent); mSpellWindow->setTitle(item.getClass().getName(item)); } @@ -1386,7 +1400,16 @@ namespace MWGui int durabilityPercent = 100; if (item.getClass().hasItemHealth(item)) { - durabilityPercent = static_cast(item.getClass().getItemHealth(item) / static_cast(item.getClass().getItemMaxHealth(item)) * 100); + int weapmaxhealth = item.getClass().getItemMaxHealth(item); + if (weapmaxhealth == 0) + { + durabilityPercent = 0; + } + else + { + int weaphealth = item.getClass().getItemHealth(item); + durabilityPercent = static_cast(weaphealth / static_cast(weapmaxhealth) * 100); + } } mHud->setSelectedWeapon(item, durabilityPercent); mInventoryWindow->setTitle(item.getClass().getName(item)); diff --git a/apps/openmw/mwmechanics/combat.cpp b/apps/openmw/mwmechanics/combat.cpp index be55b681f..2a9ff5e84 100644 --- a/apps/openmw/mwmechanics/combat.cpp +++ b/apps/openmw/mwmechanics/combat.cpp @@ -371,10 +371,17 @@ namespace MWMechanics return; const bool weaphashealth = weapon.getClass().hasItemHealth(weapon); - if(weaphashealth) + if (weaphashealth) { - int weaphealth = weapon.getClass().getItemHealth(weapon); int weapmaxhealth = weapon.getClass().getItemMaxHealth(weapon); + + if (weapmaxhealth == 0) + { + damage = 0; + return; + } + + int weaphealth = weapon.getClass().getItemHealth(weapon); damage *= (float(weaphealth) / weapmaxhealth); } From 54bd7b2dcff287451c2eea0b318b098ad5e90a35 Mon Sep 17 00:00:00 2001 From: Capostrophic Date: Thu, 25 Oct 2018 15:45:31 +0300 Subject: [PATCH 2/3] Implement getItemNormalizedHealth() method and use it --- apps/openmw/mwclass/npc.cpp | 11 +---------- apps/openmw/mwgui/tradewindow.cpp | 13 ++----------- apps/openmw/mwgui/windowmanagerimp.cpp | 12 ++---------- apps/openmw/mwmechanics/combat.cpp | 11 +---------- apps/openmw/mwworld/class.cpp | 12 ++++++++++++ apps/openmw/mwworld/class.hpp | 3 +++ 6 files changed, 21 insertions(+), 41 deletions(-) diff --git a/apps/openmw/mwclass/npc.cpp b/apps/openmw/mwclass/npc.cpp index f7172ac0b..b8b1e9600 100644 --- a/apps/openmw/mwclass/npc.cpp +++ b/apps/openmw/mwclass/npc.cpp @@ -1148,16 +1148,7 @@ namespace MWClass const bool hasHealth = it->getClass().hasItemHealth(*it); if (hasHealth) { - int armorMaxHealth = it->getClass().getItemMaxHealth(*it); - if (armorMaxHealth == 0) - { - ratings[i] = 0; - } - else - { - int armorHealth = it->getClass().getItemHealth(*it); - ratings[i] *= (float(armorHealth) / armorMaxHealth); - } + ratings[i] *= it->getClass().getItemNormalizedHealth(*it); } } } diff --git a/apps/openmw/mwgui/tradewindow.cpp b/apps/openmw/mwgui/tradewindow.cpp index 2fd91fd4a..ce8193da2 100644 --- a/apps/openmw/mwgui/tradewindow.cpp +++ b/apps/openmw/mwgui/tradewindow.cpp @@ -34,17 +34,8 @@ namespace { float price = static_cast(item.getClass().getValue(item)); if (item.getClass().hasItemHealth(item)) - { - if (item.getClass().getItemMaxHealth(item) == 0) - { - price = 0; - } - else - { - price *= item.getClass().getItemHealth(item); - price /= item.getClass().getItemMaxHealth(item); - } - } + price *= item.getClass().getItemNormalizedHealth(item); + return static_cast(price * count); } diff --git a/apps/openmw/mwgui/windowmanagerimp.cpp b/apps/openmw/mwgui/windowmanagerimp.cpp index 3b26edecb..27ea534a7 100644 --- a/apps/openmw/mwgui/windowmanagerimp.cpp +++ b/apps/openmw/mwgui/windowmanagerimp.cpp @@ -1400,17 +1400,9 @@ namespace MWGui int durabilityPercent = 100; if (item.getClass().hasItemHealth(item)) { - int weapmaxhealth = item.getClass().getItemMaxHealth(item); - if (weapmaxhealth == 0) - { - durabilityPercent = 0; - } - else - { - int weaphealth = item.getClass().getItemHealth(item); - durabilityPercent = static_cast(weaphealth / static_cast(weapmaxhealth) * 100); - } + durabilityPercent = static_cast(item.getClass().getItemNormalizedHealth(item)); } + mHud->setSelectedWeapon(item, durabilityPercent); mInventoryWindow->setTitle(item.getClass().getName(item)); } diff --git a/apps/openmw/mwmechanics/combat.cpp b/apps/openmw/mwmechanics/combat.cpp index 2a9ff5e84..41e2485ce 100644 --- a/apps/openmw/mwmechanics/combat.cpp +++ b/apps/openmw/mwmechanics/combat.cpp @@ -373,16 +373,7 @@ namespace MWMechanics const bool weaphashealth = weapon.getClass().hasItemHealth(weapon); if (weaphashealth) { - int weapmaxhealth = weapon.getClass().getItemMaxHealth(weapon); - - if (weapmaxhealth == 0) - { - damage = 0; - return; - } - - int weaphealth = weapon.getClass().getItemHealth(weapon); - damage *= (float(weaphealth) / weapmaxhealth); + damage *= weapon.getClass().getItemNormalizedHealth(weapon); } static const float fDamageStrengthBase = MWBase::Environment::get().getWorld()->getStore().get() diff --git a/apps/openmw/mwworld/class.cpp b/apps/openmw/mwworld/class.cpp index 68d5998ac..70fde33cf 100644 --- a/apps/openmw/mwworld/class.cpp +++ b/apps/openmw/mwworld/class.cpp @@ -83,6 +83,18 @@ namespace MWWorld return ptr.getCellRef().getCharge(); } + float Class::getItemNormalizedHealth (const ConstPtr& ptr) const + { + if (getItemMaxHealth(ptr) == 0) + { + return 0.f; + } + else + { + return getItemHealth(ptr) / static_cast(getItemMaxHealth(ptr)); + } + } + int Class::getItemMaxHealth (const ConstPtr& ptr) const { throw std::runtime_error ("class does not have item health"); diff --git a/apps/openmw/mwworld/class.hpp b/apps/openmw/mwworld/class.hpp index 132b7e9e8..af88b0dcc 100644 --- a/apps/openmw/mwworld/class.hpp +++ b/apps/openmw/mwworld/class.hpp @@ -112,6 +112,9 @@ namespace MWWorld virtual int getItemHealth (const ConstPtr& ptr) const; ///< Return current item health or throw an exception if class does not have item health + virtual float getItemNormalizedHealth (const ConstPtr& ptr) const; + ///< Return current item health re-scaled to maximum health + virtual int getItemMaxHealth (const ConstPtr& ptr) const; ///< Return item max health or throw an exception, if class does not have item health /// (default implementation: throw an exception) From c3e8d536cdbfedd8dccdfac24b9fd10299376f1b Mon Sep 17 00:00:00 2001 From: Capostrophic Date: Thu, 25 Oct 2018 16:09:07 +0300 Subject: [PATCH 3/3] Implement getNormalizedEnchantmentCharge() method and use it --- apps/openmw/mwgui/sortfilteritemmodel.cpp | 46 +---------------------- apps/openmw/mwgui/tradewindow.cpp | 3 +- apps/openmw/mwgui/windowmanagerimp.cpp | 18 +-------- apps/openmw/mwworld/cellref.cpp | 16 ++++++++ apps/openmw/mwworld/cellref.hpp | 3 ++ 5 files changed, 24 insertions(+), 62 deletions(-) diff --git a/apps/openmw/mwgui/sortfilteritemmodel.cpp b/apps/openmw/mwgui/sortfilteritemmodel.cpp index 5172c85ee..23f8a121b 100644 --- a/apps/openmw/mwgui/sortfilteritemmodel.cpp +++ b/apps/openmw/mwgui/sortfilteritemmodel.cpp @@ -86,66 +86,24 @@ namespace if (!leftName.empty()) { const ESM::Enchantment* ench = MWBase::Environment::get().getWorld()->getStore().get().search(leftName); - if (ench) { if (ench->mData.mType == ESM::Enchantment::ConstantEffect) - { leftChargePercent = 101; - } else - { - int maxEnchCharge = ench->mData.mCharge; - if (maxEnchCharge == 0) - { - leftChargePercent = 0; - } - else - { - float enchCharge = left.mBase.getCellRef().getEnchantmentCharge(); - if (enchCharge == -1) - { - leftChargePercent = 100; - } - else - { - leftChargePercent = static_cast(enchCharge / static_cast(maxEnchCharge) * 100); - } - } - } + leftChargePercent = static_cast(left.mBase.getCellRef().getNormalizedEnchantmentCharge(ench->mData.mCharge) * 100); } } if (!rightName.empty()) { const ESM::Enchantment* ench = MWBase::Environment::get().getWorld()->getStore().get().search(rightName); - if (ench) { if (ench->mData.mType == ESM::Enchantment::ConstantEffect) - { rightChargePercent = 101; - } else - { - int maxEnchCharge = ench->mData.mCharge; - if (maxEnchCharge == 0) - { - rightChargePercent = 0; - } - else - { - float enchCharge = right.mBase.getCellRef().getEnchantmentCharge(); - if (enchCharge == -1) - { - rightChargePercent = 100; - } - else - { - rightChargePercent = static_cast(enchCharge / static_cast(maxEnchCharge) * 100); - } - } - } + rightChargePercent = static_cast(right.mBase.getCellRef().getNormalizedEnchantmentCharge(ench->mData.mCharge) * 100); } } diff --git a/apps/openmw/mwgui/tradewindow.cpp b/apps/openmw/mwgui/tradewindow.cpp index ce8193da2..90698dfc4 100644 --- a/apps/openmw/mwgui/tradewindow.cpp +++ b/apps/openmw/mwgui/tradewindow.cpp @@ -34,8 +34,9 @@ namespace { float price = static_cast(item.getClass().getValue(item)); if (item.getClass().hasItemHealth(item)) + { price *= item.getClass().getItemNormalizedHealth(item); - + } return static_cast(price * count); } diff --git a/apps/openmw/mwgui/windowmanagerimp.cpp b/apps/openmw/mwgui/windowmanagerimp.cpp index 27ea534a7..5ffed4815 100644 --- a/apps/openmw/mwgui/windowmanagerimp.cpp +++ b/apps/openmw/mwgui/windowmanagerimp.cpp @@ -1369,22 +1369,7 @@ namespace MWGui const ESM::Enchantment* ench = mStore->get() .find(item.getClass().getEnchantment(item)); - int chargePercent = 100; - - int maxEnchCharge = ench->mData.mCharge; - if (maxEnchCharge == 0) - { - chargePercent = 0; - } - else - { - float enchCharge = item.getCellRef().getEnchantmentCharge(); - if (enchCharge != -1) - { - chargePercent = static_cast(enchCharge / static_cast(maxEnchCharge) * 100); - } - } - + int chargePercent = static_cast(item.getCellRef().getNormalizedEnchantmentCharge(ench->mData.mCharge) * 100); mHud->setSelectedEnchantItem(item, chargePercent); mSpellWindow->setTitle(item.getClass().getName(item)); } @@ -1402,7 +1387,6 @@ namespace MWGui { durabilityPercent = static_cast(item.getClass().getItemNormalizedHealth(item)); } - mHud->setSelectedWeapon(item, durabilityPercent); mInventoryWindow->setTitle(item.getClass().getName(item)); } diff --git a/apps/openmw/mwworld/cellref.cpp b/apps/openmw/mwworld/cellref.cpp index 72ee56e6a..094669bdd 100644 --- a/apps/openmw/mwworld/cellref.cpp +++ b/apps/openmw/mwworld/cellref.cpp @@ -70,6 +70,22 @@ namespace MWWorld return mCellRef.mEnchantmentCharge; } + float CellRef::getNormalizedEnchantmentCharge(int maxCharge) const + { + if (maxCharge == 0) + { + return 0; + } + else if (mCellRef.mEnchantmentCharge == -1) + { + return 1; + } + else + { + return mCellRef.mEnchantmentCharge / static_cast(maxCharge); + } + } + void CellRef::setEnchantmentCharge(float charge) { if (charge != mCellRef.mEnchantmentCharge) diff --git a/apps/openmw/mwworld/cellref.hpp b/apps/openmw/mwworld/cellref.hpp index 7e27e6ef3..5646bafb0 100644 --- a/apps/openmw/mwworld/cellref.hpp +++ b/apps/openmw/mwworld/cellref.hpp @@ -56,6 +56,9 @@ namespace MWWorld // Remaining enchantment charge. This could be -1 if the charge was not touched yet (i.e. full). float getEnchantmentCharge() const; + // Remaining enchantment charge rescaled to the supplied maximum charge (such as one of the enchantment). + float getNormalizedEnchantmentCharge(int maxCharge) const; + void setEnchantmentCharge(float charge); // For weapon or armor, this is the remaining item health.