SethRobinson / RTDink

Dink Smallwood HD is an old school zelda-like adventure/RPG that has been ported to many platforms. C++/OpenGL
Other
59 stars 7 forks source link

Issue installing dmods using older tar formats... #6

Closed drone1400 closed 7 months ago

drone1400 commented 1 year ago

The other day RobJ notified me that he was having trouble installing a dmod packed using my Martridge launcher - I decided to investigate the issue and today I found the problem...

First of all, in Martridge I use a modified version of SharpCompress to package dmods. (I changed the bzip2 algorithm in the original repo to use multi-threaded block processing ).

Before, I had only tested the packed dmods using tools like 7zip or winrar or the old DFArc V3 by Beuc. Since they worked in all of those, I was certain that my packed dmod file format was correct and bug free.

Anyway, it turns out, Sharp Compress does not use the "USTAR" format for its .tar files! Instead, it seems to use the old v7 header https://en.wikipedia.org/wiki/Tar_(computing)#Header

The problem is that the tar handler that DinkHD uses, proton\shared\util\archive\TarHandler.cpp only supports "USTAR" format, and not the older format.

Now that I know what the issue is, I'm thinking I should probably further modify my fork of SharpCompress to use the "USTAR" format for TARs, since it supports longer path names and it's just a better idea in general.

But on the other hand, I don't think DinkHD should refuse to work with the older v7 format either...

I also did a simple test, and it seems that commenting out these lines in Proton's TarHandler.cpp "fixes" the issue:

        if (strncmp(m_tarHeader.magic, "ustar", 5) != 0)
        {
            //bad header.  We probably need to push it forward by 512 bytes for some reason
            return WriteBZipStream(pData + 512, size - 512);
        }

Although I haven't studied how the code works too in depth right now and I can't say if this wouldn't break some sort of compatibility with "USTAR" format TARs...

I'll look into this the next few days and run some proper tests! For now I just wanted to let you know about the issue...

drone1400 commented 1 year ago

PS: I lied, I haven't looked into this at all after opening the initial issue. Perhaps in the near future...

SethRobinson commented 1 year ago

Hey, sorry for the slow reply and thanks very much. I'll try adding your fix and leave it in if it looks like nothing breaks.

SethRobinson commented 1 year ago

Thing is, I definitely added that 'fix' for some specific dmod probably, I just can't remember which one, doh. Do you know of a v7 older style dmod that I can use for testing btw? I might be able to add detection in a safer way.

drone1400 commented 1 year ago

Hello, saw your reply a couple of days ago but didn't get to respond. I don't really know a v7 style dmod right now off the top of my head, and finding one might take a while, but I just packed RobJ's skeleton_gelatin release using Martridge (which is built with the SharpCompres version that uses the old tar format).

I have attached it, but I had to ZIP the already packed DMOD because github was rejecting it when I tried to attach it.

It fails to install using the current DinkHD V1.99 release.

skeleton_gelatin_test_for_seth.zip

PS: About that 'fix' being for a specific DMOD... Well, that's tricky to find.. Hmm, I wonder if I could download ALL the latest DMOD versions, slam em in a folder, and set up some script that tries installing all the DMODs one by one and checks if they files were actually successfully extracted! Maybe I'll do that this weekend.

SethRobinson commented 1 year ago

Thanks very much! I hope to take a look at it soon, probably in a week or so. And yeah, some automated testing using proton\shared\util\archive\TarHandler.cpp on all the dmods would be handy indeed.

drone1400 commented 1 year ago

Uh, I downloaded all the DMODs and today I tried setting up a test project from scratch, but utterly failed in configuring everything required to get the Proton SDK to work...

I think I'll just make a clone of the RTConsole project and run the TarHandler tests there?...

Only problem is, I haven't actually thought about what a test is supposed to do exactly. I suppose extracting the files with the TarHandler, and then extracting the files with something like 7zip and comparing that the results match?

SethRobinson commented 1 year ago

Yeah, good questions. I could give it a whack as well, ... uh... any way you could put up all the dmods in one giant zip for me to grab? (Yeah, yeah, I guess I could just parse https://dinknetwork.com/api and automate some kind of downloader but...)

drone1400 commented 1 year ago

Oof, sorry for the late reply!

I uploaded my Martridge webcache over on MegaUpload, contains the DMODs along with DMOD html pages/screenshots/reviews/etc.

https://mega.nz/file/Yr9n0I6A#Z-zmwoaTuPyyX5Q-kXCU-9udGvnt6i43CuFUFje9E

The dmods are in the download subfolder of the 7zip archive.

SethRobinson commented 1 year ago

Thanks, grabbed it!

Ok, I did make DMOD extractor util that can work on directories (yeah, I cloned RTConsole as a base, good idea), and in debug mode it's easy to see the errors - if I remove the ustar 512 byte padder, the newer format dmods ALL get errors but only after successfully extracting (?).

Shouldn't be hard to fix it to work with both, gonna take a look tomorrow.

SethRobinson commented 1 year ago

Update: Ok, fixed it I think! Thanks for your help on this. Also helped me find and fix an issue with running out of stack space on a specific DMOD, installing every DMOD at once is a pretty useful test.

drone1400 commented 1 year ago

That's great news, glad to hear!

drone1400 commented 6 months ago

I should mention a correction. I had initially assumed that SharpCompress was using the 'v7' TAR header format, but it seems I was wrong. It actually uses some GNU TAR format that supports file names of any length. It just so happens that when the file names are short, it kind of looks like the 'v7' hence my confusion.

Either way, since SimonK mentioned that the iOS version still had this issue, I finally went and implemented USTAR header format in my fork of SharpCompress that I use in Martridge to ensure compatibility with older DinkHD releases, like the iOS one.