From b692b13bbcf19347e43cadec6942e53cd18a3b23 Mon Sep 17 00:00:00 2001
From: BruceChen <hi@brucec.cn>
Date: Thu, 19 Mar 2026 01:26:20 +0800
Subject: [PATCH] Fix EntityProperties crash and add default attribute registry
 fallback
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

After the previous commit (99ac3d0) moved attribute lookup from a hardcoded
dictionary to the dynamic RegistryData, MCC would crash immediately upon
joining a vanilla 1.20.6 server with:

  System.ArgumentException: An item with the same key has already been added.
  Key: unknown

Root cause: When KnownDataPacks negotiation tells the server that MCC already
has the "minecraft" data pack, the server skips sending RegistryData for
registries it considers "known" — including minecraft:attribute. This left
the dynamic attribute map empty, so every VarInt attribute ID resolved to
"unknown". The EntityProperties packet often contains multiple attributes
(e.g. armor, max_health, movement_speed), and `keys.Add("unknown", ...)` on
the second "unknown" attribute threw ArgumentException.

Two fixes applied:

1. World.GetAttributeNameById(): When the dynamic attribute map is empty
   (server didn't send the registry), automatically load the vanilla 1.20.6
   default attribute order (22 entries matching Attributes.java registration
   order). This mirrors the pattern used for dimensions where defaults are
   loaded when RegistryData is not sent. If a modded server sends a custom
   attribute registry, the dynamic map takes precedence.

2. Protocol18.cs EntityProperties handler: Change `keys.Add(propertyKey,
   propertyValue2)` to `keys[propertyKey] = propertyValue2` to tolerate
   duplicate keys defensively, in case an unknown attribute ID still appears.

Tested: MCC now connects to a vanilla 1.20.6 offline-mode server, stays
online for 6+ minutes with no crashes or disconnections. Verified: chat
messages received, inventory listing (item names/counts correct), entity
detection, TPS query, and health query all work correctly.

Made-with: Cursor
---
 MinecraftClient/Mapping/World.cs              | 34 +++++++++++++++++++
 .../Protocol/Handlers/Protocol18.cs           |  2 +-
 2 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/MinecraftClient/Mapping/World.cs b/MinecraftClient/Mapping/World.cs
index be666f4f..c0999761 100644
--- a/MinecraftClient/Mapping/World.cs
+++ b/MinecraftClient/Mapping/World.cs
@@ -244,12 +244,46 @@ namespace MinecraftClient.Mapping
 
         /// <summary>
         /// Get attribute name by its registry VarInt ID. Returns null if the ID is unknown.
+        /// When KnownDataPacks negotiation tells the server we already have vanilla data,
+        /// the server skips sending the attribute registry. In that case we fall back to
+        /// the built-in vanilla 1.20.6 attribute order (22 entries).
         /// </summary>
         public static string? GetAttributeNameById(int id)
         {
+            if (attributeIdMap.Count == 0)
+                LoadDefaultAttributes();
             return attributeIdMap.TryGetValue(id, out var name) ? name : null;
         }
 
+        private static void LoadDefaultAttributes()
+        {
+            attributeIdMap = new Dictionary<int, string>
+            {
+                { 0, "generic.armor" },
+                { 1, "generic.armor_toughness" },
+                { 2, "generic.attack_damage" },
+                { 3, "generic.attack_knockback" },
+                { 4, "generic.attack_speed" },
+                { 5, "player.block_break_speed" },
+                { 6, "player.block_interaction_range" },
+                { 7, "player.entity_interaction_range" },
+                { 8, "generic.fall_damage_multiplier" },
+                { 9, "generic.flying_speed" },
+                { 10, "generic.follow_range" },
+                { 11, "generic.gravity" },
+                { 12, "generic.jump_strength" },
+                { 13, "generic.knockback_resistance" },
+                { 14, "generic.luck" },
+                { 15, "generic.max_absorption" },
+                { 16, "generic.max_health" },
+                { 17, "generic.movement_speed" },
+                { 18, "generic.safe_fall_distance" },
+                { 19, "generic.scale" },
+                { 20, "zombie.spawn_reinforcements" },
+                { 21, "generic.step_height" }
+            };
+        }
+
         /// <summary>
         /// Store one dimension - Directly used in 1.16.2 to 1.18.2
         /// </summary>
diff --git a/MinecraftClient/Protocol/Handlers/Protocol18.cs b/MinecraftClient/Protocol/Handlers/Protocol18.cs
index 7806af21..e34306b7 100644
--- a/MinecraftClient/Protocol/Handlers/Protocol18.cs
+++ b/MinecraftClient/Protocol/Handlers/Protocol18.cs
@@ -2556,7 +2556,7 @@ namespace MinecraftClient.Protocol.Handlers
                             if (op0.Count > 0) propertyValue2 += op0.Sum();
                             if (op1.Count > 0) propertyValue2 *= 1 + op1.Sum();
                             if (op2.Count > 0) propertyValue2 *= op2.Aggregate((a, _x) => a * _x);
-                            keys.Add(propertyKey, propertyValue2);
+                            keys[propertyKey] = propertyValue2;
                         }
 
                         handler.OnEntityProperties(entityId, keys);