dumb icon indicating copy to clipboard operation
dumb copied to clipboard

Fix usage of pack_fseek in dumb_packfile_skip

Open connorjclark opened this issue 2 years ago • 5 comments

According to the Allegro 4 docs, the second argument to pack_fseek denotes a relative offset, but dumb assumes it is an absolute offset. This resulted in a bug where music files would not load: for example, in it_xm_load_sigdata the call to dumbfile_skip would mess up the file handle and result in the subsequent version check seeing garbage data.

There is another faulty usage of pack_fseek in this file from dumb_packfile_seek, however I don't see a way to resolve it as there is no way to jump to an arbitrary position in an allegro packfile. Besides, it seems unused, so probably best to delete it.

connorjclark avatar Jun 02 '22 08:06 connorjclark

Besides, it (seek) seems unused, so probably best to delete it.

Oh, it's actually used for mod.

I found the dumb_register_stdfiles feature and am using that instead with no problems.

connorjclark avatar Jun 03 '22 06:06 connorjclark

This does nothing for dumb_packfile_seek, which expects an absolute offset.

kode54 avatar Jun 04 '22 00:06 kode54

Yes, as I said I didn't fix that one because I don't think there is a way to use packfile api to do it

connorjclark avatar Jun 04 '22 01:06 connorjclark

It didn't occur to me that this library even had a point in being deeply integrated with Allegro. And in fact, I already recommend replacing it with libOpenMPT, as it's both faster and more accurate, and handles quirks with different versions of trackers/files better.

kode54 avatar Jun 04 '22 02:06 kode54

The packfile usage certainly doesn't make any sense to me, that entire "filesystem interface" should probably just be deleted. Besides, it seems it's lacking some important functional (seeking). The stdlib fs should be good for any allegro user.

connorjclark avatar Jun 04 '22 03:06 connorjclark