From 86fce41a3967ca31fbd535482e0018d672451c35 Mon Sep 17 00:00:00 2001 From: Petr Mikheev Date: Sun, 23 Apr 2023 20:37:28 +0200 Subject: [PATCH] Keep refnum when moving objects to/from inventory (#6148) --- apps/openmw/mwclass/misc.cpp | 69 ++++++++++------ apps/openmw/mwclass/misc.hpp | 1 + apps/openmw/mwgui/itemmodel.cpp | 2 + apps/openmw/mwlua/objectbindings.cpp | 105 +++++++++++++++---------- apps/openmw/mwworld/cellref.cpp | 8 +- apps/openmw/mwworld/cellref.hpp | 4 +- apps/openmw/mwworld/class.cpp | 12 ++- apps/openmw/mwworld/class.hpp | 7 +- apps/openmw/mwworld/containerstore.cpp | 7 +- 9 files changed, 135 insertions(+), 80 deletions(-) diff --git a/apps/openmw/mwclass/misc.cpp b/apps/openmw/mwclass/misc.cpp index eca29f74ce..fdb2a7274f 100644 --- a/apps/openmw/mwclass/misc.cpp +++ b/apps/openmw/mwclass/misc.cpp @@ -173,35 +173,33 @@ namespace MWClass return info; } + static MWWorld::Ptr createGold(MWWorld::CellStore& cell, int goldAmount) + { + std::string_view base = "gold_001"; + if (goldAmount >= 100) + base = "gold_100"; + else if (goldAmount >= 25) + base = "gold_025"; + else if (goldAmount >= 10) + base = "gold_010"; + else if (goldAmount >= 5) + base = "gold_005"; + + const MWWorld::ESMStore& store = *MWBase::Environment::get().getESMStore(); + MWWorld::ManualRef newRef(store, ESM::RefId::stringRefId(base)); + const MWWorld::LiveCellRef* ref = newRef.getPtr().get(); + + MWWorld::Ptr ptr(cell.insert(ref), &cell); + ptr.getCellRef().setGoldValue(goldAmount); + ptr.getRefData().setCount(1); + return ptr; + } + MWWorld::Ptr Miscellaneous::copyToCell(const MWWorld::ConstPtr& ptr, MWWorld::CellStore& cell, int count) const { MWWorld::Ptr newPtr; - - const MWWorld::ESMStore& store = *MWBase::Environment::get().getESMStore(); - if (isGold(ptr)) - { - int goldAmount = getValue(ptr) * count; - - std::string_view base = "gold_001"; - if (goldAmount >= 100) - base = "gold_100"; - else if (goldAmount >= 25) - base = "gold_025"; - else if (goldAmount >= 10) - base = "gold_010"; - else if (goldAmount >= 5) - base = "gold_005"; - - // 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)); - const MWWorld::LiveCellRef* ref = newRef.getPtr().get(); - - newPtr = MWWorld::Ptr(cell.insert(ref), &cell); - newPtr.getCellRef().setGoldValue(goldAmount); - newPtr.getRefData().setCount(1); - } + newPtr = createGold(cell, getValue(ptr) * count); else { const MWWorld::LiveCellRef* ref = ptr.get(); @@ -209,8 +207,29 @@ namespace MWClass newPtr.getRefData().setCount(count); } newPtr.getCellRef().unsetRefNum(); + newPtr.getRefData().setLuaScripts(nullptr); MWBase::Environment::get().getWorldModel()->registerPtr(newPtr); + return newPtr; + } + MWWorld::Ptr Miscellaneous::moveToCell(const MWWorld::Ptr& ptr, MWWorld::CellStore& cell) const + { + MWWorld::Ptr newPtr; + if (isGold(ptr)) + { + newPtr = createGold(cell, getValue(ptr)); + newPtr.getRefData() = ptr.getRefData(); + newPtr.getCellRef().setRefNum(ptr.getCellRef().getRefNum()); + newPtr.getCellRef().setGoldValue(ptr.getCellRef().getGoldValue()); + newPtr.getRefData().setCount(1); + } + else + { + const MWWorld::LiveCellRef* ref = ptr.get(); + newPtr = MWWorld::Ptr(cell.insert(ref), &cell); + } + ptr.getRefData().setLuaScripts(nullptr); + MWBase::Environment::get().getWorldModel()->registerPtr(newPtr); return newPtr; } diff --git a/apps/openmw/mwclass/misc.hpp b/apps/openmw/mwclass/misc.hpp index 7f43ab2861..dafeb0c764 100644 --- a/apps/openmw/mwclass/misc.hpp +++ b/apps/openmw/mwclass/misc.hpp @@ -13,6 +13,7 @@ namespace MWClass public: MWWorld::Ptr copyToCell(const MWWorld::ConstPtr& ptr, MWWorld::CellStore& cell, int count) const override; + MWWorld::Ptr moveToCell(const MWWorld::Ptr& ptr, MWWorld::CellStore& cell) const override; void insertObjectRendering(const MWWorld::Ptr& ptr, const std::string& model, MWRender::RenderingInterface& renderingInterface) const override; diff --git a/apps/openmw/mwgui/itemmodel.cpp b/apps/openmw/mwgui/itemmodel.cpp index 810c8944dd..9b9815c98a 100644 --- a/apps/openmw/mwgui/itemmodel.cpp +++ b/apps/openmw/mwgui/itemmodel.cpp @@ -57,6 +57,8 @@ namespace MWGui MWWorld::Ptr ItemModel::moveItem(const ItemStack& item, size_t count, ItemModel* otherModel) { + // TODO(#6148): moving an item should preserve RefNum and Lua scripts (unless the item stack is merged with + // already existing stack). MWWorld::Ptr ret = otherModel->copyItem(item, count); removeItem(item, count); return ret; diff --git a/apps/openmw/mwlua/objectbindings.cpp b/apps/openmw/mwlua/objectbindings.cpp index cf59c14d81..6ab6776d7a 100644 --- a/apps/openmw/mwlua/objectbindings.cpp +++ b/apps/openmw/mwlua/objectbindings.cpp @@ -284,81 +284,100 @@ namespace MWLua localScripts->removeScript(*scriptId); }; - auto removeFn = [context](const GObject& object, int countToRemove) { - MWWorld::Ptr ptr = object.ptr(); + using DelayedRemovalFn = std::function; + auto removeFn = [](const MWWorld::Ptr ptr, int countToRemove) -> std::optional { int currentCount = ptr.getRefData().getCount(); if (countToRemove <= 0 || countToRemove > currentCount) throw std::runtime_error("Can't remove " + std::to_string(countToRemove) + " of " + std::to_string(currentCount) + " items"); ptr.getRefData().setCount(currentCount - countToRemove); // Immediately change count - if (ptr.getContainerStore() || currentCount == countToRemove) - { - // Delayed action to trigger side effects - context.mLuaManager->addAction([object, countToRemove] { - MWWorld::Ptr ptr = object.ptr(); - // Restore original count - ptr.getRefData().setCount(ptr.getRefData().getCount() + countToRemove); - // And now remove properly - if (ptr.getContainerStore()) - ptr.getContainerStore()->remove(ptr, countToRemove); - else - { - MWBase::Environment::get().getWorld()->disable(object.ptr()); - MWBase::Environment::get().getWorld()->deleteObject(ptr); - } - }); - } + if (!ptr.getContainerStore() && currentCount > countToRemove) + return std::nullopt; + // Delayed action to trigger side effects + return [countToRemove](MWWorld::Ptr ptr) { + // Restore the original count + ptr.getRefData().setCount(ptr.getRefData().getCount() + countToRemove); + // And now remove properly + if (ptr.getContainerStore()) + ptr.getContainerStore()->remove(ptr, countToRemove); + else + { + MWBase::Environment::get().getWorld()->disable(ptr); + MWBase::Environment::get().getWorld()->deleteObject(ptr); + } + }; }; - objectT["remove"] = [removeFn](const GObject& object, sol::optional count) { - removeFn(object, count.value_or(object.ptr().getRefData().getCount())); + objectT["remove"] = [removeFn, context](const GObject& object, sol::optional count) { + std::optional delayed + = removeFn(object.ptr(), count.value_or(object.ptr().getRefData().getCount())); + if (delayed.has_value()) + context.mLuaManager->addAction([fn = *delayed, object] { fn(object.ptr()); }); }; - objectT["split"] = [removeFn](const GObject& object, int count) -> GObject { - removeFn(object, count); - + objectT["split"] = [removeFn, context](const GObject& object, int count) -> GObject { // Doesn't matter which cell to use because the new instance will be in disabled state. MWWorld::CellStore* cell = MWBase::Environment::get().getWorldScene()->getCurrentCell(); const MWWorld::Ptr& ptr = object.ptr(); MWWorld::Ptr splitted = ptr.getClass().copyToCell(ptr, *cell, count); splitted.getRefData().disable(); - return GObject(getId(splitted)); + + std::optional delayedRemovalFn = removeFn(ptr, count); + if (delayedRemovalFn.has_value()) + context.mLuaManager->addAction([fn = *delayedRemovalFn, object] { fn(object.ptr()); }); + + return GObject(splitted); }; objectT["moveInto"] = [removeFn, context](const GObject& object, const Inventory& inventory) { - // Currently moving to or from containers makes a copy and removes the original. - // TODO(#6148): actually move rather than copy and preserve RefNum - int count = object.ptr().getRefData().getCount(); - removeFn(object, count); - context.mLuaManager->addAction([item = object, count, cont = inventory.mObj] { - auto& refData = item.ptr().getRefData(); + const MWWorld::Ptr& ptr = object.ptr(); + int count = ptr.getRefData().getCount(); + std::optional delayedRemovalFn = removeFn(ptr, count); + context.mLuaManager->addAction([item = object, count, cont = inventory.mObj, delayedRemovalFn] { + const MWWorld::Ptr& oldPtr = item.ptr(); + auto& refData = oldPtr.getRefData(); refData.setCount(count); // temporarily undo removal to run ContainerStore::add refData.enable(); - cont.ptr().getClass().getContainerStore(cont.ptr()).add(item.ptr(), count, false); + cont.ptr().getClass().getContainerStore(cont.ptr()).add(oldPtr, count, false); refData.setCount(0); + if (delayedRemovalFn.has_value()) + (*delayedRemovalFn)(oldPtr); }); }; objectT["teleport"] = [removeFn, context](const GObject& object, const sol::object& cellOrName, const osg::Vec3f& pos, const sol::optional& optRot) { MWWorld::CellStore* cell = findCell(cellOrName, pos); MWWorld::Ptr ptr = object.ptr(); - if (ptr.getRefData().isDeleted()) - throw std::runtime_error("Object is removed"); + int count = ptr.getRefData().getCount(); + if (count == 0) + throw std::runtime_error("Object is either removed or already in the process of teleporting"); + osg::Vec3f rot = optRot ? *optRot : ptr.getRefData().getPosition().asRotationVec3(); if (ptr.getContainerStore()) { - // Currently moving to or from containers makes a copy and removes the original. - // TODO(#6148): actually move rather than copy and preserve RefNum - MWWorld::Ptr newPtr = ptr.getClass().copyToCell(ptr, *cell, ptr.getRefData().getCount()); - newPtr.getRefData().disable(); - removeFn(object, ptr.getRefData().getCount()); - ptr = newPtr; + DelayedRemovalFn delayedRemovalFn = *removeFn(ptr, count); + context.mLuaManager->addAction( + [object, cell, pos, rot, count, delayedRemovalFn] { + MWWorld::Ptr oldPtr = object.ptr(); + oldPtr.getRefData().setCount(count); + MWWorld::Ptr newPtr = oldPtr.getClass().moveToCell(oldPtr, *cell); + oldPtr.getRefData().setCount(0); + newPtr.getRefData().disable(); + teleportNotPlayer(newPtr, cell, pos, rot); + delayedRemovalFn(oldPtr); + }, + "TeleportFromContainerAction"); } - osg::Vec3f rot = optRot ? *optRot : ptr.getRefData().getPosition().asRotationVec3(); - if (ptr == MWBase::Environment::get().getWorld()->getPlayerPtr()) + else if (ptr == MWBase::Environment::get().getWorld()->getPlayerPtr()) context.mLuaManager->addTeleportPlayerAction( [cell, pos, rot] { teleportPlayer(cell, pos, rot); }); else + { + ptr.getRefData().setCount(0); context.mLuaManager->addAction( - [obj = Object(ptr), cell, pos, rot] { teleportNotPlayer(obj.ptr(), cell, pos, rot); }, + [object, cell, pos, rot, count] { + object.ptr().getRefData().setCount(count); + teleportNotPlayer(object.ptr(), cell, pos, rot); + }, "TeleportAction"); + } }; } } diff --git a/apps/openmw/mwworld/cellref.cpp b/apps/openmw/mwworld/cellref.cpp index 4a2ba37609..dcfc7ae6b7 100644 --- a/apps/openmw/mwworld/cellref.cpp +++ b/apps/openmw/mwworld/cellref.cpp @@ -25,8 +25,6 @@ namespace MWWorld { } - static const ESM::RefNum emptyRefNum = {}; - const ESM::RefNum& CellRef::getRefNum() const { return std::visit(ESM::VisitOverload{ @@ -61,11 +59,11 @@ namespace MWWorld return refNum; } - void CellRef::unsetRefNum() + void CellRef::setRefNum(ESM::RefNum refNum) { std::visit(ESM::VisitOverload{ - [&](ESM4::Reference& ref) { ref.mId = emptyRefNum; }, - [&](ESM::CellRef& ref) { ref.mRefNum = emptyRefNum; }, + [&](ESM4::Reference& ref) { ref.mId = refNum; }, + [&](ESM::CellRef& ref) { ref.mRefNum = refNum; }, }, mCellRef.mVariant); } diff --git a/apps/openmw/mwworld/cellref.hpp b/apps/openmw/mwworld/cellref.hpp index 0d13abb12e..f32312b4c0 100644 --- a/apps/openmw/mwworld/cellref.hpp +++ b/apps/openmw/mwworld/cellref.hpp @@ -31,8 +31,10 @@ namespace MWWorld // If RefNum is not set, assigns a generated one and changes the "lastAssignedRefNum" counter. const ESM::RefNum& getOrAssignRefNum(ESM::RefNum& lastAssignedRefNum); + void setRefNum(ESM::RefNum refNum); + // Set RefNum to its default state. - void unsetRefNum(); + void unsetRefNum() { setRefNum({}); } /// Does the RefNum have a content file? bool hasContentFile() const { return getRefNum().hasContentFile(); } diff --git a/apps/openmw/mwworld/class.cpp b/apps/openmw/mwworld/class.cpp index 209789778b..28777656c8 100644 --- a/apps/openmw/mwworld/class.cpp +++ b/apps/openmw/mwworld/class.cpp @@ -374,6 +374,17 @@ namespace MWWorld Ptr newPtr = copyToCellImpl(ptr, cell); newPtr.getCellRef().unsetRefNum(); // This RefNum is only valid within the original cell of the reference newPtr.getRefData().setCount(count); + newPtr.getRefData().setLuaScripts(nullptr); + MWBase::Environment::get().getWorldModel()->registerPtr(newPtr); + if (hasInventoryStore(newPtr)) + getInventoryStore(newPtr).setActor(newPtr); + return newPtr; + } + + MWWorld::Ptr Class::moveToCell(const Ptr& ptr, CellStore& cell) const + { + Ptr newPtr = copyToCellImpl(ptr, cell); + ptr.getRefData().setLuaScripts(nullptr); MWBase::Environment::get().getWorldModel()->registerPtr(newPtr); if (hasInventoryStore(newPtr)) getInventoryStore(newPtr).setActor(newPtr); @@ -384,7 +395,6 @@ namespace MWWorld { Ptr newPtr = copyToCell(ptr, cell, count); newPtr.getRefData().setPosition(pos); - return newPtr; } diff --git a/apps/openmw/mwworld/class.hpp b/apps/openmw/mwworld/class.hpp index 6feb79f7cd..59f228a4d4 100644 --- a/apps/openmw/mwworld/class.hpp +++ b/apps/openmw/mwworld/class.hpp @@ -313,7 +313,12 @@ namespace MWWorld virtual Ptr copyToCell(const ConstPtr& ptr, CellStore& cell, int count) const; - virtual Ptr copyToCell(const ConstPtr& ptr, CellStore& cell, const ESM::Position& pos, int count) const; + // Similar to `copyToCell`, but preserves RefNum and moves LuaScripts. + // The original is expected to be removed after calling this function, + // but this function itself doesn't remove the original. + virtual Ptr moveToCell(const Ptr& ptr, CellStore& cell) const; + + Ptr copyToCell(const ConstPtr& ptr, CellStore& cell, const ESM::Position& pos, int count) const; virtual bool isActivator() const { return false; } diff --git a/apps/openmw/mwworld/containerstore.cpp b/apps/openmw/mwworld/containerstore.cpp index adcc340f68..5de939b6e9 100644 --- a/apps/openmw/mwworld/containerstore.cpp +++ b/apps/openmw/mwworld/containerstore.cpp @@ -23,6 +23,7 @@ #include "manualref.hpp" #include "player.hpp" #include "refdata.hpp" +#include "worldmodel.hpp" namespace { @@ -303,9 +304,11 @@ MWWorld::ContainerStoreIterator MWWorld::ContainerStore::add( Ptr player = MWBase::Environment::get().getWorld()->getPlayerPtr(); MWWorld::ContainerStoreIterator it = addImp(itemPtr, count, resolve); + itemPtr.getRefData().setLuaScripts(nullptr); // clear Lua scripts on the original (removed) item. // The copy of the original item we just made MWWorld::Ptr item = *it; + MWBase::Environment::get().getWorldModel()->registerPtr(item); // we may have copied an item from the world, so reset a few things first item.getRefData().setBaseNode( @@ -325,10 +328,6 @@ MWWorld::ContainerStoreIterator MWWorld::ContainerStore::add( item.getCellRef().setFaction(ESM::RefId()); item.getCellRef().setFactionRank(-2); - // must reset the RefNum on the copied item, so that the RefNum on the original item stays unique - // maybe we should do this in the copy constructor instead? - item.getCellRef().unsetRefNum(); // destroy link to content file - const ESM::RefId& script = item.getClass().getScript(item); if (!script.empty()) {