-
Bug
-
Resolution: Invalid
-
None
-
1.17, 1.17.1 Pre-release 1
-
Plausible
-
Maps, Networking
Triage:
Proxy software such as Velocity uses the ClientboundLoginPacket to reset the client before it is connected to a different server (for updating registries). Minecraft normally uses the ClientboundRespawnPacket for changing dimensions, but that does not allow for updating registries and thus isn't sufficient when proxying multiple servers.
Issues:
The MapRenderer is not cleared when the client receives a ClientboundLoginPacket. It keeps references to old instances of MapItemSavedData (/MapInstance). When the client receives a ClientboundMapItemDataPacket with a map id which has already been used before the ClientboundLoginPacket, the MapRenderer will use the old data. Since a proxied server is not aware of other servers on the network, map ids between servers will often overlap, leading to this issue.
Code:
This is the code path that leads to the game failing to update map data on later packets.
First the client receives a ClientboundMapItemDataPacket:
public void handleMapItemData(ClientboundMapItemDataPacket packet) { MapRenderer mapRenderer = this.minecraft.gameRenderer.getMapRenderer(); int mapId = packet.getMapId(); String mapKey = MapItem.makeKey(mapId); MapItemSavedData mapData = this.minecraft.level.getMapData(mapKey); if (mapData == null) { mapData = MapItemSavedData.createForClient(packet.getScale(), packet.isLocked(), this.minecraft.level.dimension()); this.minecraft.level.setMapData(mapKey, mapData); } packet.applyToMap(mapData); mapRenderer.update(mapId, mapData); }
Let's say the map id is 1. And let's give the instances of this.minecraft.level the name level1 and mapData the name mapData1.
The MapRenderer now runs the following code.
public void update(int mapId, MapItemSavedData mapData) { getOrCreateMapInstance(mapId, mapData).updateTexture(); } private MapInstance getOrCreateMapInstance(int mapId, MapItemSavedData mapData) { return this.maps.computeIfAbsent(mapId, id -> new MapInstance(mapId, mapData)); }
The Map in the MapRenderer now contains the mapping for 1 -> mapData1.
Now the client receives a ClientboundLoginPacket which overrides the client level.
public void handleLogin(ClientboundLoginPacket packet) { ... this.level = new ClientLevel(...); this.minecraft.setLevel(this.level); ... }
If the client now receives another ClientboundMapItemDataPacket for the map id 1 the map renderer will not update the map.
public void handleMapItemData(ClientboundMapItemDataPacket packet) { MapRenderer mapRenderer = this.minecraft.gameRenderer.getMapRenderer(); int mapId = packet.getMapId(); String mapKey = MapItem.makeKey(mapId); MapItemSavedData mapData = this.minecraft.level.getMapData(mapKey); if (mapData == null) { mapData = MapItemSavedData.createForClient(packet.getScale(), packet.isLocked(), this.minecraft.level.dimension()); this.minecraft.level.setMapData(mapKey, mapData); } packet.applyToMap(mapData); mapRenderer.update(mapId, mapData); }
This code will once again create an instance of MapItemSavedData (mapData2).
But the MapRenderer still has the mapping 1 -> mapData1 in its Map.
public void update(int mapId, MapItemSavedData mapData) { getOrCreateMapInstance(mapId, mapData).updateTexture(); } private MapInstance getOrCreateMapInstance(int mapId, MapItemSavedData mapData) { return this.maps.computeIfAbsent(mapId, id -> new MapInstance(mapId, mapData)); }
This code will now use the old map data and the map can never be updated by the server again.
Possible fixes:
There are a few possible ways to fix this issue. One option is to copy the map instances over to the new world on login packet, as is done when a ClientboundRespawnPacket is received. Another option would be to call GameRenderer.resetData() (which calls MapRenderer.resetData()) in ClientPacketListener.handleLogin. Lastly, handleMapItemData could check for old map data
The copying code over for the ClientboundRespawnPacket packet was added in 1.17; if similar code is added for ClientboundLoginPacket this issue would be solved.
Map<String, MapItemSavedData> maps = this.level.getAllMapData();
...
this.level = new ClientLevel(...);
...
this.level.addMapData(maps);
Clearing the existing map data on login would also fix the issue. In 1.12, changing worlds (with either respawn or login) called GameRenderer.resetData, which reset map data and thus also avoided this issue. Currently, this data is only cleared when disconnecting from a server.
It would also be possible to look check if the MapItemSavedData already exists in the MapRenderers Map and then use it, as was done prior to 1.17. Here is the 1.16.4 code for reference:
public void handleMapItemData(ClientboundMapItemDataPacket packet) { MapRenderer mapRenderer = this.minecraft.gameRenderer.getMapRenderer(); String mapKey = MapItem.makeKey(packet.getMapId()); MapItemSavedData mapData = this.minecraft.level.getMapData(string4); if (mapData == null) { mapData = new MapItemSavedData(string4); if (mapRenderer.getMapInstanceIfExists(mapKey) != null) { MapItemSavedData existingMapData = mapRenderer.getData(mapRenderer.getMapInstanceIfExists(mapKey)); if (existingMapData != null) mapData = existingMapData; } this.minecraft.level.setMapData(mapData); } packet.applyToMap(mapData); mapRenderer.update(mapData); }
