search

collinsmith / riiablo

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

Implement I/O package more specific to project use-case #95

Closed collinsmith closed 3 years ago

collinsmith commented 3 years ago

Current I/O is done through com.riiablo.codec.util.BitStream and variations of InputStream, ByteBuffer, Reader LittleEndianDataInputStream. This has been sufficient, but I'm starting to consider making improvements and implementing something more appropriate for my use-case. Setting everything to use netty's ByteBuf over ByteBuffer may be nice. Another driving factor for this issue is to make debugging issues and file types easier (d2s, item lists, properties). BitStream issues are incredibly difficult to debug, and making sure I have access to the file offset and bit offset will be really useful.

Something like a d2s file requires reading the different sections, separating and processing those individual sections, and slicing them where appropriate. In the case of dcc, the byte stream is sliced further into some bit streams.I think implementing my own I/O package will help to improve code readability and consistency (all codecs will use the same API).

Finally, support for a writer is required, specifically a BitOutput class to write bits to a bit stream (required for #93). Might as well lump in a ByteOutput class too, since that is trivial compared to writing variable bits.

My current research assumes everything is little endian. I need to check and see if anything in the project uses big endian, but I'm pretty sure that it's all in little endian -- at least where the important file types are concerned.

References: Originally posted by @collinsmith in https://github.com/collinsmith/riiablo/issues/93#issuecomment-669768597 Originally posted by @collinsmith in https://github.com/collinsmith/riiablo/issues/93#issuecomment-670678414

collinsmith commented 3 years ago

This is what I've done and looked into so far:

bit-io is a nice library for operating on bit streams, but it's missing some functionality required by this project, and creating my own implementation should have it's own advantages.

I've played with a few different APIs

What I've settled on and will pursue is creating standalone classes for ByteInput, BitInput, ByteOutput, BitOutput. Bit classes are derived from Byte classes, but lazily initialized, so most of the code which doesn't use Bit operations can still use the Byte operations with the same API, and code that does need it just needs to call unalign() to initialize and access a bit stream. Byte variants will hold a ByteBuf reference, enforce alignment, and call ByteBuf methods directly -- I expect this to have a negligible performance impact compared to anything that I currently have.

I experimented with going the stateful route (one monolithic class) with methods that control when the Input is in aligned or unaligned mode, but separating into Bit and Byte implementation classes will help to enforce better code, and Byte classes can use more optimal byte-aligned operations directly. Separating the functionality has the added benefit of allowing Byte classes without ever touching unaligned methods. The big concern with this route would have been really subtle errors that I could see being hard to track down, so I dropped the idea and am pursuing separate classes for {Bit, Byte} × {Input, Output}.

collinsmith commented 3 years ago

Preliminary testing looks good on com.riiablo.io. I've been able to validate reading and writing a simple magic item. I'm happy with the API and very happy with ByteBuf. ByteBuf is tightly coupled with the I/O API for the time being for tests (with Byte classes acting as a wrapper). I'm going to work on rolling this package into the d2s codec, validate, and close this issue and create more as issues pop up for features.

collinsmith commented 3 years ago

d2s codec almost completed -- I've chosen to split it into D2S model and D2SSerlializer factory. I'm extremely happy with how improved this API is. Error handling is significantly more robust, and the API will allow for much more concise testing when those are written. Replacing BitStream is proving difficult to replace -- very tightly coupled in some class hierarchies. I may end up having to deprecate it and replace the remaining uses at a later date when I can get some help.

collinsmith commented 3 years ago

56e6eb480208f901d47d71dd3defca768c4283f7 contains an implemented implementation of this package. Will create new issues and improve code as needed. Closing this issue.