New palette changer script #1108
Conversation
This script allows a user to change Freedoom's palette, while also changing any graphics that use the old palette to use the new colours
|
This seems like a neat idea, but before I dig into it, and you spend more time on it, I'd like to understand the big picture better, and I'd like to tell you about some scripts I added, just in case there's overlap: The The Is that relevant to what you want to do? |
|
I know about your scripts and what they do, but IIRC neither of your scripts offered the features I wanted. Furthermore, you used the I took a bit of code (recursive directory/file processing) from your |
|
OK, I understand it better. Thanks for explaining. Yes, I should've documented the pypy requirement somewhere - I started out with Pillow, but it didn't give me the low level control I needed, and rebuilding IDAT by hand was more than I wanted to get into. That script will be run infrequently anyway. I'll open a PR to add it to the I'm glad my script could be helpful in that way. |
Do not update a colour if all of its duplicates have not been replaced Attempt to check whether the IDAT(s) contain any modified colours before re-writing it
Properly processing the IDAT chunks in a PNG file requires decompression, decoding, de-filtering, and de-interlacing. Using pypng takes care of this for the developer.
Talon1024
changed the title
Don't just "return True" at the end of process_png. Use relpath for more readable paths
Most of the features a PNG decoder should have, except for deinterlacing, are implemented.
|
Confirmed it builds properly on my end without having to install anything else. Takes like 2 more seconds but that's not unexpected given all the stuff it's got to do. Which... stuff... I'm still not qualified to give the final review for. I've approved the PR but someone else who knows what they're doing please take one last look. |
|
I can look at it this weekend. Unless I'm missing something this change just adds a script that can optionally be run, so it does not impact the usual build time. |
Add a description of the script to the ArgumentParser Remove the directory argument, since it has limited use, and is a potential security flaw Rename "dir" argument to "directory", since "dir" is a built-in function.
|
Could you provide an example invocation of this script? My first guess was: But that just truncates |
|
The script uses You're supposed to give it a new palette, like the one in this zip file. If you extract it to
Or if you just want to see what would change without actually making any changes:
You should then see a bunch of lines like |
There was a problem hiding this comment.
Neat script. I know your motivation may be to fix the color 255 issue, but there are other things that could be done as well.
Freedoom could include a directory of optional palettes that could be applied. For example, just for the fun of it I tried making a gray scale palette and passing it to the script. Here's how E1M6 looks with it in GZDoom:
The code and palette I used:
gray.zip
Since Freedoom is inclined to accommodate color blindness maybe there could be a palette for that, although I'm not sure exactly that that means (high color saturation?, a brightness difference between red and green?). But there may be other reasons, like people preferring an overall hue or tone. If you did this maybe you could make up a unique extension like pal or palette instead of data just to make it clearer what the files are.
If I were to have done this I probably would've had it skip any PNG that does not exactly have the current game palette (that is not exactly lumps/playpal/playpal-base.lmp). Then the new palette would be blindly applied leaving it an exercise for the user to make sure that it lines up index by index as intended, and no other chunks would be updated. You've solved a harder problem, and that's great, but it may be slower in some cases.
When and if this gets incorporated in the build my inclination would be for the default all target to not depend on it, but maybe fix or dist so it gets run less frequently. This is assuming you end up fixing the color 255 issue with it. I'm personally not too worried about color 255 even though I wrote scripts to remap it. How many custom PWADs actually have custom color 255?
This script edits both PNGs files and lumps/playpal/playpal-base.lmp. You could have options to only do one or the other since I can see either being useful depending on the situation.
Anyway, currently this PR just adds a script without anything to invoke it, which makes it quite safe. I'm happy to apply this now. Just let me know when and if you're done. My suggestions are just that, so don't feel that you have to do anything.
Freedoom needs a very good reason to have a custom PNG decoder, but there is no good reason to write one. Furthermore, this may make it easier to write code to modify true-colour PNGs. However, because of issues I'm having (see comments), the code is currently disabled.
|
For whatever little it's worth, I just tried building this and confirm I didn't need to install anything new. EDIT: I just realized this script isn't run during build, disregard |
|
The custom PNG decoder appears to be gone as per fragglet's feedback. Looks good me. |
This script allows a user to change Freedoom's palette, while also changing any graphics that use the old palette to use the new colours.
Since some of the other scripts in the Freedoom repo use pypng, and I needed access to low-level PNG features, this script also uses pypng. It should work with Python 3.8 (Ubuntu 20.04).