Made code clearer

NitroxPatcher/Patches/Dynamic/Knife_OnToolUseAnim_Patch.cs

@@ -18,9 +18,11 @@ public sealed partial class Knife_OnToolUseAnim_Patch : NitroxPatch, IDynamicPat
      * bool flag = liveMixin.IsAlive();
      * REPLACE below line
      * liveMixin.TakeDamage(this.damage, vector, this.damageType, null);
-     * BY:
+     * 
+     * WITH:
      * liveMixin.TakeDamage(this.damage, vector, this.damageType, Player.mainObject);
      * this.GiveResourceOnDamage(gameObject, liveMixin.IsAlive(), flag);
+     * 
      */
     public static IEnumerable<CodeInstruction> Transpiler(IEnumerable<CodeInstruction> instructions)
     {

NitroxPatcher/Patches/Dynamic/LiveMixin_TakeDamage_Patch.cs

@@ -31,60 +31,74 @@ public static bool Prefix(out float __state, LiveMixin __instance, GameObject de
 
     public static void Postfix(float __state, LiveMixin __instance, float originalDamage, GameObject dealer, bool __runOriginal)
     {
-        bool healthChanged = __state != __instance.health;
-        if (!__runOriginal || !ShouldBroadcastDamage(__instance, dealer, originalDamage, healthChanged))
+        if (!__runOriginal)
         {
             return;
         }
 
-        // Let others know if we have a lock on this entity
-        if (__instance.TryGetIdOrWarn(out NitroxId id) && Resolve<SimulationOwnership>().HasAnyLockType(id))
+        // IsRemoteHealthChanging means we're replicating an action from the server and BaseCell is managed by BaseLeakManager
+        if (Resolve<LiveMixinManager>().IsRemoteHealthChanging || __instance.GetComponent<BaseCell>())
         {
-            Optional<EntityMetadata> metadata = Resolve<EntityMetadataManager>().Extract(__instance.gameObject);
+            return;
+        }
 
-            if (metadata.HasValue)
-            {
-                Resolve<Entities>().BroadcastMetadataUpdate(id, metadata.Value);
-            }
+        // PvP damage is always 0 so we need to check for it before the regular case
+        if (HandlePvP(__instance, dealer, originalDamage))
+        {
+            return;
+        }
+
+        // At this point, if the victim didn't take damage, there's no point in broadcasting it
+        if (__state != __instance.health)
+        {
+            return;
         }
+
+        BroadcastDefaultTookDamage(__instance);
     }
 
-    private static bool ShouldBroadcastDamage(LiveMixin victim, GameObject dealer, float damage, bool healthChanged)
+    private static bool HandlePvP(LiveMixin liveMixin, GameObject dealer, float damage)
     {
-        if (Resolve<LiveMixinManager>().IsRemoteHealthChanging || victim.GetComponent<BaseCell>())
+        if (!liveMixin.TryGetComponent(out RemotePlayerIdentifier remotePlayerIdentifier))
         {
             return false;
         }
 
-        if (victim.TryGetComponent(out RemotePlayerIdentifier remotePlayerIdentifier))
+        // Dealer must be the local player, and we need to know about the item they're holding
+        if (dealer != Player.mainObject || !Inventory.main.GetHeldObject())
         {
-            // Handle it internally
-            HandlePvP(remotePlayerIdentifier.RemotePlayer, dealer, damage);
             return false;
         }
+
+        PvPAttack.AttackType attackType;
+        switch (Inventory.main.GetHeldTool())
+        {
+            case HeatBlade:
+                attackType = PvPAttack.AttackType.HeatbladeHit;
+                break;
+            case Knife:
+                attackType = PvPAttack.AttackType.KnifeHit;
+                break;
+            default:
+                // We don't want to send non-registered attacks
+                return false;
+        }
 
-        // The health change check must happen after the PvP one
-        return healthChanged;
+        Resolve<IPacketSender>().Send(new PvPAttack(remotePlayerIdentifier.RemotePlayer.PlayerId, damage, attackType));
+        return true;
     }
 
-    private static void HandlePvP(RemotePlayer remotePlayer, GameObject dealer, float damage)
+    private static void BroadcastDefaultTookDamage(LiveMixin liveMixin)
     {
-        if (dealer == Player.mainObject && Inventory.main.GetHeldObject())
+        // Let others know if we have a lock on this entity
+        if (liveMixin.TryGetIdOrWarn(out NitroxId id) && Resolve<SimulationOwnership>().HasAnyLockType(id))
         {
-            PvPAttack.AttackType attackType;
-            switch (Inventory.main.GetHeldTool())
+            Optional<EntityMetadata> metadata = Resolve<EntityMetadataManager>().Extract(liveMixin.gameObject);
+
+            if (metadata.HasValue)
             {
-                case HeatBlade:
-                    attackType = PvPAttack.AttackType.HeatbladeHit;
-                    break;
-                case Knife:
-                    attackType = PvPAttack.AttackType.KnifeHit;
-                    break;
-                default:
-                    // We don't want to send non-registered attacks
-                    return;
+                Resolve<Entities>().BroadcastMetadataUpdate(id, metadata.Value);
             }
-            Resolve<IPacketSender>().Send(new PvPAttack(remotePlayer.PlayerId, damage, attackType));
         }
     }
 }

NitroxServer/Communication/Packets/Processors/PvPAttackProcessor.cs

@@ -26,12 +26,20 @@ public PvPAttackProcessor(ServerConfig serverConfig, PlayerManager playerManager
 
     public override void Process(PvPAttack packet, Player player)
     {
-        if (serverConfig.PvPEnabled &&
-            playerManager.TryGetPlayerById(packet.TargetPlayerId, out Player targetPlayer) &&
-            damageMultiplierByType.TryGetValue(packet.Type, out float multiplier))
+        if (!serverConfig.PvPEnabled)
         {
-            packet.Damage *= multiplier;
-            targetPlayer.SendPacket(packet);
+            return;
         }
+        if (!playerManager.TryGetPlayerById(packet.TargetPlayerId, out Player targetPlayer))
+        {
+            return;
+        }
+        if (!damageMultiplierByType.TryGetValue(packet.Type, out float multiplier))
+        {
+            return;
+        }
+
+        packet.Damage *= multiplier;
+        targetPlayer.SendPacket(packet);
     }
 }