-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
PNG support and loading of internal resources from pk3 #1688
Conversation
* Add support for "sprites" and "colormaps" directories. * Move predefined lumps and autoload files to "base". * Remove -dumplumps and -dumptables.
This would be a lot easier to review without all these |
But then it won't work - it will crash in menus, etc. We can check for PNG graphics at the The code changes that matter are in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were these sounds converted directly from dogs.h? I just want to make sure it still has the fade-in fix applied to prevent clicking: https://github.com/fabiangreffrath/woof/pull/1220/files#diff-e3deb8687f39858b8fc845f21d5d902ba1f941418098989d4bef053ddcd0306aR3567-R3568
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Were these sounds converted directly from dogs.h?
Yes. I dumped all the internal lumps with -dumplumps
and then converted all the graphics and sounds with SLADE.
ooh shiny new features to comment on, once again! ;) I tried this out and I was worried at first because I noticed the UMAPDEF lumps are now in woof.pk3 and thought they might have conflicted with the midi/qol/fix pwads I usually load for both NERVE.WAD and MASTERLEVELS.WAD. Anyway.. Looking forward to this! For reference, the following is the verbose log output I get when running Details
|
Yes, UMAPDEF works like in-engine defaults. Maybe we shouldn't have exposed it to users like that. Thanks for testing, but this PR is very early, I'm not even sure if this approach is the right one. |
yeah I was just "kicking the tyres" on what's already there, out of curiosity |
@@ -0,0 +1,25 @@ | |||
BSD 2-Clause License |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the license to README.md.
V_DrawPatchFullScreen((patch_t *) t); | ||
if (c==2119826587u || c==2391756584u) | ||
// [FG] removed the embedded DOGOVRLY title pic overlay graphic lump | ||
if (W_CheckNumForName("DOGOVRLY") > 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure if this easter-egg has ever been used, but I'd rather have this checked and be sure than throwing it out altogether.
r += 4 | ||
|
||
// | ||
// Converts a linear graphic to a patch with transparency. Mostly straight |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this code was the reason why we added Simon Judd's copyright when you added DBIGFONT support?
|
||
void *buf = W_CacheLumpNum(lump, tag); | ||
|
||
if (memcmp(buf, "\211PNG\r\n\032\n", 8)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check for lump size here, please.
"../share/" PROJECT_SHORTNAME "/" BASE_FILE_NAME , 0); | ||
if (!result) | ||
{ | ||
I_Error("PrepareBasePaths: Failed to open %s", basefile); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Using basefile
here isn't quite exact.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the review. I'm thinking of making a new PR with new functions and a separate PR with graphics conversion/function renaming etc. I also want to work with .zip/.pk3 archives in memory without temporary files.
I assume you are ok with using libsnpg. What do you think about git submodules? I've read some negative comments about them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I assume you are ok with using libsnpg.
Yes, I like it and the fact that it plays nicely with miniz.
What do you think about git submodules? I've read some negative comments about them.
I'm against them. We are mostly importing "static code", and not some vivid external projects like e.g. ZMusic which just happen to be developed in a separate GIT tree. The very meaning of libs like miniz and spng is that they are developed out and do not change frequently anymore.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Understood.
@@ -215,28 +215,6 @@ void V_InitColorTranslation(void) | |||
} | |||
} | |||
|
|||
void WriteGeneratedLumpWad(const char *filename) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was quite useful to extract color translation tables after they have been extended and adapted to the current palette in V_InitColorTranslation()
, but I 'm not sure if people will miss it.
// Allow writing predefined lumps out as a WAD. | ||
// | ||
|
||
if ((p = M_CheckParm("-dumplumps")) && p < myargc-1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please regenerate params.h
when removing CLI.
I want to split this PR into smaller parts and reconstruct it. Thanks again, I will keep this review in mind. |
Question: Should we add support for "textures" folder and TX_START/TX_END markers? https://zdoom.org/wiki/TX_START