search

wheybags / freeablo

[ARCHIVED] Modern reimplementation of the Diablo 1 game engine
GNU General Public License v3.0
2.16k stars 195 forks source link

Add gif export to celview using a modified version of jo_gif.cpp #413

Closed Exairnous closed 5 years ago

Exairnous commented 5 years ago

Since a lot of CEL/CL2's are animations I have added an option to celview to save as an animated gif. Transparency is supported and the resulting gifs open properly in gimp. In all the tests I did there was never a problem with palette size -- thankfully diablo doesn't seem to use all 256 colors + transparency in each CEL/CL2. The end filesize is not as small as it could be because a full 256 palette is generated for each frame, but since we're dealing with such low resolution I don't think it's a big problem.

I'm not sure if I should have modified cmake files to include the new library (jo_gif.cpp), but it compiles fine for me. So, let me know if this needs any changes.

EDIT: I see travis is set to treat warnings as errors. jo_gif.cpp started off with the warnings about the initializer list, but since it would compile on my system I just left it. If you want I'll see if I can get rid of them.

EDIT2: I tried to fix the travis formatting issue with:

curl https://termbin.com/t4hrn > /tmp/fa_format_diff.patch && git apply /tmp/fa_format_diff.patch

but first I got a certificate error and then when I connected with an insecure connection it couldn't find components/render/sdl2backend.cpp. Any help would be appreciated.

Exairnous commented 5 years ago

I looked into the brace initialization thing a bit more, and for the one on line 160 I think it could be fixed by replacing this:

jo_gif_lzw_t state = {fp, 9};

with this:

jo_gif_lzw_t state = {fp, 9, {}, *"", 0, 0, 0};

For the one on line 218, I think we should just leave it. I'm not sure why travis is complaining about it, I don't get any warnings about it with gcc, and it seems to be pretty clear and standard c++.

Let me know what you think.

wheybags commented 5 years ago

We already have headers for disabling warnings in external files, I've added them now in the commit I just pushed. As for the autoformatting, you need to run it from the root of the repo. Should possibly add some code to cd to the git root.

wheybags commented 5 years ago

Thanks for the PR!

Exairnous commented 5 years ago

Your welcome :smiley: I Didn't know about the disable warning headers. Neat! I'll remember them for next time. About the autoformatting, for some reason I thought it was supposed to affect the pull request on Github, not my local copy that I would then have to push to Github. I think I've got it now :crossed_fingers: But, I'm still a little concerned that curl can't verify the certificate.

wheybags commented 5 years ago

Yeah, I'm not sure why that happened. It started happening for me too lately, it used to be fine. SSL cert is still accepted by my browser, so I dunno what's going on tbh.

On Fri, Jan 11, 2019, at 6:36 AM, Exairnous wrote:

Your welcome :smiley: I Didn't know about the disable warning headers. Neat! I'll remember them for next time. About the autoformatting, for some reason I thought it was supposed to affect the pull request on Github, not my local copy that I would then have to push to Github. I think I've got it now :crossed_fingers: But, I'm still a little concerned that curl can't verify the certificate.

-- You are receiving this because you modified the open/close state. Reply to this email directly or view it on GitHub: https://github.com/wheybags/freeablo/pull/413#issuecomment-453384330