fix: fix 'magnetic furniture' bug (#2340)

skymp5-client/src/services/services/remoteServer.ts

@@ -186,12 +186,15 @@ export class RemoteServer extends ClientListener {
 
       let functionChecker: (() => boolean) | null = null;
       let factName = "";
+      let delaySeconds = -1.0;
       if (baseType === FormType.Container) {
         functionChecker = () => Ui.isMenuOpen("ContainerMenu");
         factName = "'ContainerMenu open'";
+        delaySeconds = 0.0;
       } else if (baseType === FormType.Furniture) {
         functionChecker = () => !!Game.getPlayer()?.getFurnitureReference();
         factName = "'getFurnitureReference not null'";
+        delaySeconds = 1.0;
       }
 
       if (functionChecker === null) {
@@ -218,12 +221,16 @@ export class RemoteServer extends ClientListener {
           }
         };
 
-        this.controller.emitter.emit("sendMessage", {
-          message: message,
-          reliability: "reliable"
-        });
+        logTrace(this, "onOpenContainerMesage - waiting", delaySeconds, "seconds before sending ActivateMessage");
+
+        Utility.waitMenuMode(delaySeconds).then(() => {
+          this.controller.emitter.emit("sendMessage", {
+            message: message,
+            reliability: "reliable"
+          });
 
-        logTrace(this, "onOpenContainerMesage - sent ActivateMessage", message);
+          logTrace(this, "onOpenContainerMesage - sent ActivateMessage", message);
+        });
       })();
     });
   }

skymp5-server/cpp/server_guest_lib/MpActor.cpp

@@ -44,8 +44,11 @@
 
 struct MpActor::Impl
 {
-  std::map<uint32_t, Viet::Promise<VarValue>> snippetPromises;
+  // TODO: consider optimizing data structure and/or general refactoring
   std::set<std::shared_ptr<DestroyEventSink>> destroyEventSinks;
+  std::set<std::shared_ptr<DisableEventSink>> disableEventSinks;
+
+  std::map<uint32_t, Viet::Promise<VarValue>> snippetPromises;
   uint32_t snippetIndex = 0;
   uint32_t respawnTimerIndex = 0;
   bool isRespawning = false;
@@ -374,6 +377,10 @@ void MpActor::Disable()
     return;
   }
 
+  for (auto& sink : pImpl->disableEventSinks) {
+    sink->BeforeDisable(*this);
+  }
+
   MpObjectReference::Disable();
 
   for (auto [snippetIdx, promise] : pImpl->snippetPromises) {
@@ -508,6 +515,16 @@ void MpActor::RemoveEventSink(std::shared_ptr<DestroyEventSink> sink)
   pImpl->destroyEventSinks.erase(sink);
 }
 
+void MpActor::AddEventSink(std::shared_ptr<DisableEventSink> sink)
+{
+  pImpl->disableEventSinks.insert(sink);
+}
+
+void MpActor::RemoveEventSink(std::shared_ptr<DisableEventSink> sink)
+{
+  pImpl->disableEventSinks.erase(sink);
+}
+
 MpChangeForm MpActor::GetChangeForm() const
 {
   auto res = MpObjectReference::GetChangeForm();
@@ -1103,8 +1120,9 @@ void MpActor::ModifyActorValuePercentage(espm::ActorValue av,
 
 void MpActor::BeforeDestroy()
 {
-  for (auto& sink : pImpl->destroyEventSinks)
+  for (auto& sink : pImpl->destroyEventSinks) {
     sink->BeforeDestroy(*this);
+  }
 
   MpObjectReference::BeforeDestroy();
 

skymp5-server/cpp/server_guest_lib/MpActor.h

@@ -74,6 +74,8 @@ class MpActor : public MpObjectReference
 
   [[nodiscard]] bool OnEquip(uint32_t baseId);
 
+  // TODO: consider removing the entire DestroyEventSink feature because it's
+  // only used in unit tests
   class DestroyEventSink
   {
   public:
@@ -84,6 +86,16 @@ class MpActor : public MpObjectReference
   void AddEventSink(std::shared_ptr<DestroyEventSink> sink);
   void RemoveEventSink(std::shared_ptr<DestroyEventSink> sink);
 
+  class DisableEventSink
+  {
+  public:
+    virtual ~DisableEventSink() = default;
+    virtual void BeforeDisable(MpActor& actor) = 0;
+  };
+
+  void AddEventSink(std::shared_ptr<DisableEventSink> sink);
+  void RemoveEventSink(std::shared_ptr<DisableEventSink> sink);
+
   MpChangeForm GetChangeForm() const override;
   void ApplyChangeForm(const MpChangeForm& changeForm) override;
 

skymp5-server/cpp/server_guest_lib/MpObjectReference.cpp

@@ -72,6 +72,10 @@ UpdatePropertyMessage MpObjectReference::PreparePropertyMessage(
   return res;
 }
 
+// TODO: de-duplicate code of
+// OccupantDisableEventSink/OccupantDisableEventSink, the only difference is
+// base classes
+
 class OccupantDestroyEventSink : public MpActor::DestroyEventSink
 {
 public:
@@ -85,8 +89,44 @@ class OccupantDestroyEventSink : public MpActor::DestroyEventSink
 
   void BeforeDestroy(MpActor& actor) override
   {
-    if (!RefStillValid())
+    if (!RefStillValid()) {
       return;
+    }
+
+    if (untrustedRefPtr->occupant == &actor) {
+      untrustedRefPtr->SetOpen(false);
+      untrustedRefPtr->occupant = nullptr;
+    }
+  }
+
+private:
+  bool RefStillValid() const
+  {
+    return untrustedRefPtr == wst.LookupFormById(refId).get();
+  }
+
+  WorldState& wst;
+  MpObjectReference* const untrustedRefPtr;
+  const uint32_t refId;
+};
+
+class OccupantDisableEventSink : public MpActor::DisableEventSink
+{
+public:
+  OccupantDisableEventSink(WorldState& wst_,
+                           MpObjectReference* untrustedRefPtr_)
+    : wst(wst_)
+    , untrustedRefPtr(untrustedRefPtr_)
+    , refId(untrustedRefPtr_->GetFormId())
+  {
+  }
+
+  void BeforeDisable(MpActor& actor) override
+  {
+    if (!RefStillValid()) {
+      return;
+    }
+
     if (untrustedRefPtr->occupant == &actor) {
       untrustedRefPtr->SetOpen(false);
       untrustedRefPtr->occupant = nullptr;
@@ -1455,6 +1495,7 @@ void MpObjectReference::ProcessActivateNormal(
     if (CheckIfObjectCanStartOccupyThis(activationSource, kOccupationReach)) {
       if (this->occupant) {
         this->occupant->RemoveEventSink(this->occupantDestroySink);
+        this->occupant->RemoveEventSink(this->occupantDisableSink);
       }
       SetOpen(true);
       actorActivator->SendToUser(
@@ -1466,7 +1507,11 @@ void MpObjectReference::ProcessActivateNormal(
 
       this->occupantDestroySink.reset(
         new OccupantDestroyEventSink(*GetParent(), this));
-      this->occupant->AddEventSink(occupantDestroySink);
+      this->occupant->AddEventSink(this->occupantDestroySink);
+
+      this->occupantDisableSink.reset(
+        new OccupantDisableEventSink(*GetParent(), this));
+      this->occupant->AddEventSink(this->occupantDisableSink);
     }
   } else if (t == espm::ACTI::kType && actorActivator) {
     // SendOpenContainer being used to activate the object
@@ -1479,6 +1524,7 @@ void MpObjectReference::ProcessActivateNormal(
     if (CheckIfObjectCanStartOccupyThis(activationSource, kOccupationReach)) {
       if (this->occupant) {
         this->occupant->RemoveEventSink(this->occupantDestroySink);
+        this->occupant->RemoveEventSink(this->occupantDisableSink);
       }
 
       // SendOpenContainer being used to activate the object
@@ -1489,7 +1535,11 @@ void MpObjectReference::ProcessActivateNormal(
 
       this->occupantDestroySink.reset(
         new OccupantDestroyEventSink(*GetParent(), this));
-      this->occupant->AddEventSink(occupantDestroySink);
+      this->occupant->AddEventSink(this->occupantDestroySink);
+
+      this->occupantDisableSink.reset(
+        new OccupantDisableEventSink(*GetParent(), this));
+      this->occupant->AddEventSink(this->occupantDisableSink);
     }
   }
 }
@@ -1599,10 +1649,22 @@ bool MpObjectReference::CheckIfObjectCanStartOccupyThis(
   }
 
   if (this->occupant == &activationSource) {
-    spdlog::info("MpObjectReference::ProcessActivate {:x} - occupant is "
-                 "already this object (activationSource = {:x})",
-                 GetFormId(), activationSource.GetFormId());
-    return true;
+    auto& loader = GetParent()->GetEspm();
+    auto base = loader.GetBrowser().LookupById(GetBaseId());
+    auto t = base.rec->GetType();
+    auto actorActivator = activationSource.AsActor();
+    if (t == "FURN" && actorActivator) {
+      spdlog::info("MpObjectReference::ProcessActivate {:x} - occupant is "
+                   "already this object (activationSource = {:x}). Blocking "
+                   "because it's FURN",
+                   GetFormId(), activationSource.GetFormId());
+      return false;
+    } else {
+      spdlog::info("MpObjectReference::ProcessActivate {:x} - occupant is "
+                   "already this object (activationSource = {:x})",
+                   GetFormId(), activationSource.GetFormId());
+      return true;
+    }
   }
 
   spdlog::info("MpObjectReference::ProcessActivate {:x} - occupant is "

skymp5-server/cpp/server_guest_lib/MpObjectReference.h

@@ -39,6 +39,7 @@ struct GridPosInfo
 class MpActor;
 class WorldState;
 class OccupantDestroyEventSink;
+class OccupantDisableEventSink;
 
 class FormCallbacks;
 
@@ -78,6 +79,7 @@ class MpObjectReference
   , protected ChangeFormGuard
 {
   friend class OccupantDestroyEventSink;
+  friend class OccupantDisableEventSink;
 
 public:
   static const char* Type() { return "ObjectReference"; }
@@ -252,6 +254,7 @@ class MpObjectReference
   uint32_t baseId = 0;
   MpActor* occupant = nullptr;
   std::shared_ptr<OccupantDestroyEventSink> occupantDestroySink;
+  std::shared_ptr<OccupantDisableEventSink> occupantDisableSink;
   std::optional<std::chrono::system_clock::duration> relootTimeOverride;
   std::unique_ptr<uint8_t> chanceNoneOverride;
   bool activationBlocked = false;