collinsmith / riiablo

Diablo II remade using Java and LibGDX
http://riiablo.com
Apache License 2.0
909 stars 101 forks source link

Remaining bytes in stream after DS1 codec reads #73

Open collinsmith opened 4 years ago

collinsmith commented 4 years ago

Some DS1 files are being loaded with remaining bytes left in the stream. So far I've only noticed two instances, both of which are version 13. I haven't looked into this a great deal because everything seems to work just fine, but I'd like to audit all of the used DS1s to see if this occurs for other version as well. My code checks versions 9-13 specifically, but I only have error reports for version 13, so 9-12 may be unnecessary.

Easy thing first though. The algorithm needs to be looked over for typos and then I'll look into possible errors in the file format. At least for the errors below, the data looks alright, I'm guessing this is some extra int towards the end of the stream that either isn't skipped properly or indicates the size of the corresponding list is 0 (both remaining values are 0).

DS1: data/global/tiles/Act1/Outdoors/URiver.ds1 4B still available in stream! version=13; [00, 00, 00, 00]
DS1: com.riiablo.map.DS1@6da246e4[fileName=data/global/tiles/Act1/Outdoors/URiver.ds1,version=13,width=8,height=8,act=1,tagType=0,numFiles=2,numWalls=0,numFloors=1,numTags=0,numShadows=1,layerStream=[8, 10],floorLine=9,floorLen=81,shadowLine=9,shadowLen=81,tagLine=0,tagLen=0,wallLine=0,wallLen=0,numObjects=0,numGroups=0,numPaths=0]
DS1: data/global/tiles/Act1/Outdoors/LRiverC.ds1 4B still available in stream! version=13; [00, 00, 00, 00]
DS1: com.riiablo.map.DS1@699ddf88[fileName=data/global/tiles/Act1/Outdoors/LRiverC.ds1,version=13,width=8,height=8,act=1,tagType=0,numFiles=3,numWalls=1,numFloors=1,numTags=0,numShadows=1,layerStream=[0, 4, 8, 10],floorLine=9,floorLen=81,shadowLine=9,shadowLen=81,tagLine=0,tagLen=0,wallLine=9,wallLen=81,numObjects=0,numGroups=0,numPaths=0]
collinsmith commented 3 years ago

Double checked everything and this appears to be a bug with those files. Maybe padding or something. I'd be more concerned if they were non-zero. I need to test another version 13 ds1 and find out if there is similar behavior.

Des-Nerger commented 1 year ago

If the game doesn't consider them buggy, then they are not. I hypothesize that v13 does actually support paths:

--- a/core/src/main/java/com/riiablo/map/DS1.java
+++ b/core/src/main/java/com/riiablo/map/DS1.java
@@ -408,3 +408,3 @@ public class DS1 {
   private void readPaths(InputStream in) throws IOException {
-    if (version >= 14 && in.available() >= PrimitiveUtils.INT_BYTES) {
+    if (version >= 13 && in.available() >= PrimitiveUtils.INT_BYTES) {
       numPaths = EndianUtils.readSwappedInteger(in);

To check if it's indeed the case, I just need to create an artificial v13 with some valid and observable paths and feed this .ds1 into the game... but I'm lazy right now. Maybe later.

collinsmith commented 1 year ago

IIRC, there were only a few with this version number, so there was not enough data to determine this, specially since it was all zeroed. Would require testing how game interprets.

Lectem commented 1 year ago

If the game doesn't consider them buggy, then they are not. I hypothesize that v13 does actually support paths

No, paths are only read for v14. I just double checked the .DLLs.

Reference here should be correct as it has been checked against behaviour of the original game. You can compare it with riiablo's implementation.