Skip to content
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

ID24 interlevel lumps implementation #2674

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Cacodemon345
Copy link
Contributor

This PR implements ID24's interlevel lumps format, which is needed for intermission screens for Legacy of Rust mapset.

Copy link
Member

@coelckers coelckers left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just from looking over the code there's 3 minor things that need changing:

  1. Make the new feature available to regular MAPINFO and ZMAPINFO as well, not just UMAPINFO.
  2. G_InitNew is not the right place to set the secret level flag. This should be done right after parsing all MAPINFOs. G_InitNew will be called each time a game starts.
  3. The JSON parser for intermissions should not abort - if it throws an error, catch it, print it to the console and instead show the default intermission screen

@Cacodemon345
Copy link
Contributor Author

Done.

@coelckers
Copy link
Member

The animations of the second episode do not play. Is this the result of what people were talking about on Doomworld regarding the level numbers?

If so, the format is indeed broken by design, any chance to do something about it?

@Cacodemon345
Copy link
Contributor Author

Cacodemon345 commented Sep 13, 2024

The animations of the second episode assume that the level numbers will restart from 1 with each episode defined.

The first map of the second episode is defined with an episode field in the UMAPINFO as expected, but the port seems to assume that the level numbers for the interlevel lumps specified with it and afterward will restart from 1 and keep incrementing with each map definition.

It can be easily dealt with by adding an additional field to account for this problem that also follows the expected behaviour and make it work again. At least it shouldn't negatively affect behaviour for single-episode/Doom II mapsets.

@coelckers
Copy link
Member

That's what I meant. The spec is broken, plain and simple - making such assumptions is not good.

@Cacodemon345
Copy link
Contributor Author

I wouldn't say it's broken; it's basically an assumption made because the base UMAPINFO spec has no provisions for arbitrary (or implicit) map numbers. It at least can be dealt with by adding two fields to the internal map structure, one indicating the episode number and the other one indicating the map number within the episode. For MAPINFO/ZMAPINFO using these interlevel lumps, those follow normal rules for level numbers.

It does suck, sure, but nowhere near as a deal breaker as the TRANMAP stuff in the ID24 specification is, and at the very least can be worked around.

@Cacodemon345 Cacodemon345 marked this pull request as draft September 13, 2024 17:12
@Cacodemon345
Copy link
Contributor Author

Marked as draft in anticipation of the official level number behaviour getting amended in future patches.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants