Remove never-working caching and fix bugs in region code (#2716)

Fixes #2684
EngineHub · Feb 18, 2025 · c20366d · c20366d
1 parent 8a13912
commit c20366d
Show file tree

Hide file tree

Showing 4 changed files with 59 additions and 38 deletions.
diff --git a/verification/src/changes/accepted-core-public-api-changes.json b/verification/src/changes/accepted-core-public-api-changes.json
@@ -28,5 +28,42 @@
                 "CONSTRUCTOR_REMOVED"
             ]
         }
+    ],
+    "No one should be using these fields outside the class anyways": [
+        {
+            "type": "com.sk89q.worldedit.world.storage.FileMcRegionChunkStore",
+            "member": "Class com.sk89q.worldedit.world.storage.FileMcRegionChunkStore",
+            "changes": [
+                "FIELD_REMOVED_IN_SUPERCLASS"
+            ]
+        },
+        {
+            "type": "com.sk89q.worldedit.world.storage.McRegionChunkStore",
+            "member": "Field cachedReader",
+            "changes": [
+                "FIELD_REMOVED"
+            ]
+        },
+        {
+            "type": "com.sk89q.worldedit.world.storage.McRegionChunkStore",
+            "member": "Field curFilename",
+            "changes": [
+                "FIELD_REMOVED"
+            ]
+        },
+        {
+            "type": "com.sk89q.worldedit.world.storage.TrueZipMcRegionChunkStore",
+            "member": "Class com.sk89q.worldedit.world.storage.TrueZipMcRegionChunkStore",
+            "changes": [
+                "FIELD_REMOVED_IN_SUPERCLASS"
+            ]
+        },
+        {
+            "type": "com.sk89q.worldedit.world.storage.ZippedMcRegionChunkStore",
+            "member": "Class com.sk89q.worldedit.world.storage.ZippedMcRegionChunkStore",
+            "changes": [
+                "FIELD_REMOVED_IN_SUPERCLASS"
+            ]
+        }
     ]
 }
diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/world/snapshot/SnapshotRestore.java b/worldedit-core/src/main/java/com/sk89q/worldedit/world/snapshot/SnapshotRestore.java
@@ -29,6 +29,8 @@
 import com.sk89q.worldedit.world.chunk.Chunk;
 import com.sk89q.worldedit.world.storage.ChunkStore;
 import com.sk89q.worldedit.world.storage.MissingChunkException;
+import org.apache.logging.log4j.LogManager;
+import org.apache.logging.log4j.Logger;
 
 import java.io.IOException;
 import java.util.ArrayList;
@@ -41,6 +43,8 @@
  */
 public class SnapshotRestore {
 
+    private static final Logger LOGGER = LogManager.getLogger();
+
     private final Map<BlockVector2, ArrayList<BlockVector3>> neededChunks = new LinkedHashMap<>();
     private final ChunkStore chunkStore;
     private final EditSession editSession;
@@ -154,6 +158,7 @@ public void restore() throws MaxChangedBlocksException {
             } catch (MissingChunkException me) {
                 missingChunks.add(chunkPos);
             } catch (IOException | DataException me) {
+                LOGGER.info(() -> "Failed to load chunk at " + chunkPos, me);
                 errorChunks.add(chunkPos);
                 lastErrorMessage = me.getMessage();
             }

diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/world/storage/McRegionChunkStore.java b/worldedit-core/src/main/java/com/sk89q/worldedit/world/storage/McRegionChunkStore.java
@@ -32,9 +32,6 @@
 
 public abstract class McRegionChunkStore extends ChunkStore {
 
-    protected String curFilename = null;
-    protected McRegionReader cachedReader = null;
-
     /**
      * Get the filename of a region file.
      *
@@ -50,26 +47,14 @@ public static String getFilename(BlockVector2 position) {
 
     protected McRegionReader getReader(BlockVector2 pos, String worldname) throws DataException, IOException {
         String filename = getFilename(pos);
-        if (curFilename != null) {
-            if (curFilename.equals(filename)) {
-                return cachedReader;
-            } else {
-                try {
-                    cachedReader.close();
-                } catch (IOException ignored) {
-                }
-            }
-        }
         InputStream stream = getInputStream(filename, worldname);
-        cachedReader = new McRegionReader(stream);
-        //curFilename = filename;
-        return cachedReader;
+        return new McRegionReader(stream);
     }
 
     @Override
     public LinCompoundTag getChunkData(BlockVector2 position, World world) throws DataException, IOException {
-        McRegionReader reader = getReader(position, world.getName());
-        try (var chunkStream = new DataInputStream(reader.getChunkInputStream(position))) {
+        try (McRegionReader reader = getReader(position, world.getName());
+             var chunkStream = new DataInputStream(reader.getChunkInputStream(position))) {
             return LinBinaryIO.readUsing(chunkStream, LinRootEntry::readFrom).value();
         }
     }
@@ -84,11 +69,4 @@ public LinCompoundTag getChunkData(BlockVector2 position, World world) throws Da
      */
     protected abstract InputStream getInputStream(String name, String worldName) throws IOException, DataException;
 
-    @Override
-    public void close() throws IOException {
-        if (cachedReader != null) {
-            cachedReader.close();
-        }
-    }
-
 }
diff --git a/worldedit-core/src/main/java/com/sk89q/worldedit/world/storage/McRegionReader.java b/worldedit-core/src/main/java/com/sk89q/worldedit/world/storage/McRegionReader.java
@@ -60,7 +60,9 @@ counts. The chunk offset for a chunk (x, z) begins at byte 4*(x+z*32) in the
 import com.sk89q.worldedit.world.DataException;
 
 import java.io.ByteArrayInputStream;
+import java.io.Closeable;
 import java.io.DataInputStream;
+import java.io.EOFException;
 import java.io.IOException;
 import java.io.InputStream;
 import java.util.zip.GZIPInputStream;
@@ -70,7 +72,7 @@ counts. The chunk offset for a chunk (x, z) begins at byte 4*(x+z*32) in the
  * Reader for a MCRegion file. This reader works on input streams, meaning
  * that it can be used to read files from non-file based sources.
  */
-public class McRegionReader {
+public class McRegionReader implements Closeable {
 
     protected static final int VERSION_GZIP = 1;
     protected static final int VERSION_DEFLATE = 2;
@@ -129,10 +131,10 @@ public synchronized InputStream getChunkInputStream(BlockVector2 position) throw
             throw new DataException("The chunk at " + position + " is not generated");
         }
 
-        int sectorNumber = offset >> 8;
+        long sectorNumber = offset >> 8;
         int numSectors = offset & 0xFF;
 
-        stream.seek((long) sectorNumber * SECTOR_BYTES);
+        stream.seek(sectorNumber * SECTOR_BYTES);
         int length = dataStream.readInt();
 
         if (length > SECTOR_BYTES * numSectors) {
@@ -142,19 +144,17 @@ public synchronized InputStream getChunkInputStream(BlockVector2 position) throw
 
         byte version = dataStream.readByte();
 
+        byte[] data = new byte[length - 1];
+        try {
+            dataStream.readFully(data);
+        } catch (EOFException e) {
+            throw new DataException("MCRegion file does not contain "
+                + x + "," + z + " in full");
+        }
+
         if (version == VERSION_GZIP) {
-            byte[] data = new byte[length - 1];
-            if (dataStream.read(data) < length - 1) {
-                throw new DataException("MCRegion file does not contain "
-                        + x + "," + z + " in full");
-            }
             return new GZIPInputStream(new ByteArrayInputStream(data));
         } else if (version == VERSION_DEFLATE) {
-            byte[] data = new byte[length - 1];
-            if (dataStream.read(data) < length - 1) {
-                throw new DataException("MCRegion file does not contain "
-                        + x + "," + z + " in full");
-            }
             return new InflaterInputStream(new ByteArrayInputStream(data));
         } else {
             throw new DataException("MCRegion chunk at "
@@ -187,6 +187,7 @@ public boolean hasChunk(int x, int z) {
     /**
      * Close the stream.
      */
+    @Override
     public void close() throws IOException {
         stream.close();
     }