-
Notifications
You must be signed in to change notification settings - Fork 93
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
Add "GAMEINFO" IWAD field support #603
base: master
Are you sure you want to change the base?
Conversation
834c660
to
36f7406
Compare
Why not support I don't think we should add another lump, many WADs already contain GAMEINFO/GAMECONF. |
I agree. The IWAD lump was more an example about how it would work. This PR was mostly for the functionality of choosing an IWAD based on a PWAD lump. Personally I'd rather support |
Basically, DSDA-Doom already has a ZDoom scanner, see |
Thanks! This took a little longer because I was looking at Your code really helped... Although I'd like to get better at understand C++ and learn how to get C and C++ to work together :) Now that I see the "GAMEINFO" support in action, I really like it! |
64611ac
to
29de070
Compare
29de070
to
dfb0429
Compare
I like the idea of supporting GAMEINFO. But lets also support the LOAD field yeah? And woof should probably do the same |
Is it practical? I have never seen a PWAD that uses it.
Yes, agreed. |
I also dont know of any PWAD that uses it, but I can imagine reasons to want it. |
I'm not sure this works the way you think it works exactly. According to the ZDoom wiki, the LOAD field loads certain wad files BEFORE the wad (usually iwad) that I includes the GAMEINFO lump. In the example of a wide-screen asset wad, the LOAD lump wouldn't allow for wide-screen assets to be used, because they would be overwritten by the iwad. |
My bad then, I didnt read the "before" in the wiki. Well that sucks... what uses does loading before even have? The simple "IWAD" lump you suggested before is looking cleaner now tbh. With the drawback that no current wads support this lump yet, unlike GAMEINFO. I think its worth thinking more about this, as supporting a lump just for one field seems wrong. Wouldve prefered if GAMEINFO had fields for iwad, pwads and complvl. |
I think it's more focused around zdoom projects separating resources into different pk3s (like graphics, sounds, etc)... Perhaps even making some resources optional, which the iwad would then check if those existed.
I actually disagree here. The main reason I took the "IWAD" lump approach before is because I thought it'd be too difficult to parse the "GAMEINFO" lump. If I knew it was as easy as this, I would've gone with "GAMEINFO" originally. There's a couple points I want to point out here. GAMEINFO is a GZDoom lump.. so it's not that weird to only parse some of the lump. In fact, it doesn't make any sense for it to parse complevel, since that isn't even zdoom related. I actually am highly against having a new lump for multiple reasons.
|
We can add new fields to GAMEINFO, GZDoom would ignore them. GAMECONF has |
We could do this... but honestly it feels kinda useless and undermines the whole point of the COMPLVL lump. The main point of the COMPLVL lump was that it was supported by a majority of source ports. So most wads would keep this lump regardless, unless you are expecting other source ports to magically support GAMEINFO as well. The main appeal for me was that old wads already had GAMEINFO to pick the IWAD from.
I realise this is probably just a "me" thing, but I very much dislike GAMECONF. It would be much more code to support it, due to it being in JSON format. I recently asked my discord server if they preferred GAMEINFO or GAMECONF, and they said GAMEINFO due to it being easier to write from scratch. I think it may possibly be worth supporting at some point... maybe... Although me and many other modders are quite unhappy about how ID24 is formatted and how not user-friendly it is. So I'll admit I'm a bit biased against GAMECONF, but if we eventually support GAMECONF, we definitely should support GAMEINFO. |
But we can add an |
3416cc3
to
bb698fd
Compare
Figured I'd mention this is ready for an other review. Now zip files have been fixed, as well GAMEINFO reading via autoload. This PR should be fully complete now, in regards to the functionality of allowing PWADs to define an IWAD and GAMEINFO support. IMO any other lumps defining IWADs should be a separate PR. |
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.
Seems to work fine, thank you.
prboom2/src/d_main.c
Outdated
Z_Free(iwadlump); | ||
} | ||
|
||
G_ReloadDefaults(); // killough 3/4/98: set defaults just loaded. |
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.
Why did you move G_ReloadDefault()
here? HandlePlayback()
may be affected by this. I'd rather not move G_ReloadDefault()
if possible.
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.
The main reason why I moved G_ReloadDefault()
is because the game crashes if it doesn't come after the second W_Init()
call.
I haven't figured out the reason why. I'd prefer it to be above Handleplayback()
like before, but currently it's there to avoid the crash.
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.
Ah I figured it out! See 37979ea.
The crash was caused because lumpnum
and lumpinfo
weren't being cleared... Which is a little weird because I know at some point I had that code there.
Anyway, G_ReloadDefaults()
should be in the same place as it was before now, and I moved the iwadlump lprintf
to happen a bit later so that it shows just before when the "Detected COMPLVL" text shows.
3d956dd
to
d4ae017
Compare
d4ae017
to
37979ea
Compare
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.
LGTM
Figured I'd mention that I added the "Doom 1 lump detection" code into this PR. Hopefully that's fine... |
So this is going to be a fun one...
This solution is pretty jank and probably not the best implementation... but it does work!
This is similar to the
GAMEINFO
lump that GZDoom uses that can specify an IWAD to load via a PWAD lump.Essentially to search the PWADs for an "IWAD" lump, I needed to cache the lumps before choosing the IWAD. So what ends up happening is that I cache it once (as a sort of quick cache), read the IWAD lump and then clear the cache for the "Real" lump cache.
Currently the main thing I don't like is, I haven't been able to combine the zip functions together as one with the variable "MainLumpCache".
If you're wondering what this is useful for, it's so I can drag-and-drop a plutonia based wad on dsda-doom, and it can load
plutonia.wad
if it finds it in the directory. I have a clause that defaults to normal behaviour if the wad specified in the "IWAD" lump isn't found.