Jump to content
Search In
  • More options...
Find results that contain...
Find results in...
El juancho

Episode 5+ Boss Behaviour Discussion

Recommended Posts

The level only ends when bosses die due to an oversight in the UMAPINFO lump, namely the lack of a "bossaction = clear" under E5M8. All the other kinds of MAPINFO indicate that nothing's supposed to happen if a boss dies, which is further supported by the DeHackEd patch in sigil_compat removing the BossDeath pointer from the Mastermind's death. So both DSDA-Doom and Crispy are technically working correctly, but it does mean SIGIL will need an addon to work as intended in ports that support (or don't prioritize another format over) UMAPINFO. So here's one I threw together.

sigumfix.zip

Share this post


Link to post

Why would E5M8 need a `bossaction = clear`? There is no E5M8 in the IWADs, so there shouldn't be any default behavior in need of an override. If a port is applying some default definition for that slot, that seems like the naughty thing.

Share this post


Link to post
14 minutes ago, Xaser said:

There is no E5M8 in the IWADs, so there shouldn't be any default behavior in need of an override.

Nevertheless, there is: doom treats every boss monster as a boss type that can end the level in ExM8 where x > 4. That behaviour is only removed by clearing the bossaction, as far as I understand umapinfo.

See https://github.com/id-Software/DOOM/blob/master/linuxdoom-1.10/p_enemy.c#L1645-L1676

Share this post


Link to post

Huh, that's weird -- hooray sorta-undefined behavior. :P

 

That's something that should definitely be mentioned in the UMAPINFO docs, wherever they are. It's a super-non-obvious thing.

Share this post


Link to post

Wouldn't it make more sense to default-clear the boss action? This was surely never intended and only a result of A_BossDeath's shitty coding.

 

Share this post


Link to post

It'd be much nicer if it was clear by default, yeah -- ideally this would be something explicitly specified in a UMAPINFO spec somewhere, but since that doesn't exist (yet??) then it's mainly a matter of UMAPINFO-implementing ports agreeing on changing it. There's been plenty of precedent lately of making quick fixes like this (plus a spec version bump where applicable), so that'd be great. :P

Share this post


Link to post
21 hours ago, SiFi270 said:

So both DSDA-Doom and Crispy are technically working correctly, but it does mean SIGIL will need an addon to work as intended in ports that support (or don't prioritize another format over) UMAPINFO.

No, it absolutely does not mean SIGIL needs an addon/patch, because it already has one. SIGIL already ships with SIGIL_COMPAT that was designed for use in primitive ports such as pr+ and crispy, said boss behaviour being one of the primary reasons. You are specifically instructed to do so by the textfile. Also you are specifically instructed to use _COMPAT for recording demos. Furthermore, those decisions were made before UMAPINFO was even fully designed, the UMAPINFO lump was a very late addition just before release, so whatever port development happened post-hoc has little to do with Romero or the rest of the team. It's nice that these ports are trying to make e5 work, but that was made primarily for Zdoom and other truly unrestricted ports and does not limit itself with undefined behavioral quirks of pr+. As far as I'm concerned, playing e5 in dsda-doom is unsupported and on your own risk.

Share this post


Link to post

And I'm sorry for snapping, if there's a reasonable consensus re: umapinfo defaults that makes more sense than what the vanilla code does in the switch case, that'd be grand. I don't think there are very many wads to consider, right? DTWID:LE and SIGIL?

 

Both your deh and umapinfo fixes can be useful, so don't let my harsh tone discourage you. The umapinfo fix might certainly help casual players who want to play e5 on dsda-doom, at least until the default behaviour for the boss action is resolved one way or the other. I suggest posting it in the relevant SIGIL thread.

 

The deh file can then be useful for people who simply insist on recording with something like that new choco++ port, however it must be ensured that playback is compatible with the internal bex, as pr+ ports will prefer it over an external file (afaik) and forcing unnecessary files on people who shouldn't need them (pr+/dsda users) is bad praxis anyway.

Share this post


Link to post

I gotta disagree somewhat -- The entire reason for including a UMAPINFO in SIGIL.wad was so ports that supported UMAPINFO later on down the line would gain support for SIGIL.wad proper. The reason there's a problem is because of some funky undefined behavior, so the best thing to do at this point is define it. :P

 

Crispy's patch has the right idea but it's too targeted -- it'd be better to remove that weird default for E6 onward as well, not just hackpatch E5 for Sigil compat. Someone's eventually going to make an E6 and then trip+fall into the same hole.

 

The choco++ port is a different case: the lack of BEX support is a missing required feature, so loading an external DEH is the best option for now. As for why it's BEX, if my memory's correct vanilla DEH did not allow you to zero out a codepointer, only swap them for another. I'm not sure if this is actually a limitation of vanilla dehacked itself or just the dehacked.exe tool, but I don't believe you could produce a patch with "Codep Frame = 0" so it was a bit spooky to try and see if all the various ports actually supported it safely. BEX has a well-defined story for that that, and at the time SIGIL was released, all the notable ports that supported loading internal DEHACKED lumps* supported BEX, so we stuck to the format that we know worked.

 

[*Fun trivia: Chocolate Doom actually does support this, technically-speaking, but it's hardcoded to only happen when loading the Hacx 1.2 IWAD. T'was the only thing stopping Hacx 1.2 from being choco-compatible as-is so Fraggle was cool and added it in. :D ]

 

[EDIT] In case anyone's wondering why a couple of random community folks are talking about SIGIL dev things despite not being Romero, open the wad and press F1. :P

Share this post


Link to post

For DOS Vanilla, Xttl made Sigexe which added SIGIL support to Vanilla - But it was an exe hack and something beyond the confines of DeHacked. It also has a new episode menu, based on Ultimate Doom-Plus.

 

Quote
  • episode menu is now expanded to include a 5th choice
  • sky texture when loading E5 maps is set properly (I used "SKY5_ZD" in the end instead of "SKY5" after all)
  • ending picture after the text works properly (I used "SIGILEND" for this even though GZDoom displays "CREDIT" or "HELP1", not sure which one was really intended...)
  • idmus4x plays E4 music, idmus5x plays E5 music, idmus6x plays intro, intermission, etc.
  • PWAD headers don't trigger modified game warning anymore so it's not shown due to the autoloaded sigil.wad (-file parameter still does it though)
  • E5M8 does not prematurely end anymore

I think everything should pretty much work as expected now. Also, I didn't mention it last time, but the "Doom 1.91" longtics demo patcher of course still works as usual if needed.

 

Share this post


Link to post
1 hour ago, dew said:

SIGIL already ships with SIGIL_COMPAT that was designed for use in primitive ports such as pr+ and crispy, said boss behaviour being one of the primary reasons. You are specifically instructed to do so by the textfile. 

Nevertheless, people wouldn't stop requesting support for Sigil's E5 so I somehow felt I had to bow to the masses and implement the most pragmatic support for it that I could come up with. 

Share this post


Link to post
13 hours ago, dew said:

Also you are specifically instructed to use _COMPAT for recording demos

 

Recording demos for episodes >= 5 is now explicitly disabled in Crispy.

Share this post


Link to post
11 hours ago, Xaser said:

I gotta disagree somewhat -- The entire reason for including a UMAPINFO in SIGIL.wad was so ports that supported UMAPINFO later on down the line would gain support for SIGIL.wad proper. The reason there's a problem is because of some funky undefined behavior, so the best thing to do at this point is define it. :P

 

 

Agreed. It was really just an oversight - when I wrote the A_BossDeath part of UMAPINFO I never considered that the code was written in a way that would let it do incomplete checks on the other M8 slots. The intent here clearly was to have a clean slate for any map slot that doesn't have an explicit action defined, i.e. E1M8, E2M8, E3M8, E4M6, E4M8 and MAP07.

It's not that defining this properly now would do any harm. In Vanilla it wasn't even possible to play these slots so it's not that it'd break many maps, if at all. On the other hand, letting this oversight remain will surely cause a lot more trouble down the line.

 

 

Share this post


Link to post
5 hours ago, fabian said:

Recording demos for episodes >= 5 is now explicitly disabled in Crispy. 

Ah, careful there, I'm sorry if I led you to believe this should be disallowed globally. Vanilla does allow recording on episodes >= 5 and creates legit demos. The exception is SIGIL, which provides a separate file that avoids aforementioned issues by moving the episode and using internal dehacked, however this is not the case for the other prominent example - DtWiD: The Lost Episodes, which features e5 and e6.

 

As Xaser mentioned, nothing is stopping another mapper from putting their mapset in e5, so I suggest crispy and dsda-doom implement a common strategy how to handle this issue - and it should probably treat the boss behaviour (that unfortunate switch/case) rather than recording itself. Sorry for the confusion.

Share this post


Link to post

Right now a mapper checking if their wad works correctly in the vanilla compatibility may get the wrong answer if checking in crispy. If you intend to keep a special exception for sigil, I'd recommend making it for sigil only and not having a general exception for all wads. For all we know, there is one already out in the wild that uses the behaviour that bosses do trigger exits, in which case it is now broken in crispy.

Share this post


Link to post

The only time I think there should be exceptions as required are for officially (or in the case of Sigil semi-officially) released wads. Wads like Master Levels (the combined ps3 version), No Rest for the Living, and Sigil. Overriding pre-defined behavior (even if it's really dumb and not very forward thinking) should be the job of umapinfo or another similar system.

Share this post


Link to post
On 9/18/2021 at 7:04 PM, kraflab said:

As already discussed in that thread, dsda-doom has the correct behaviour. Crispy introduced an exception that violates compatibility here: https://github.com/fabiangreffrath/crispy-doom/commit/6cc4f1c77080892a983bde2cd1bedc915296d01b.

 

So, the fix would be to revert this commit again and let killing the first Baron on Sigil E5M8 quit the level? Or maybe guard it with a conditional like "map E5M8 is from a WAD whose name starts with SIGIL and we are not recording or playing back a demo and are not in a multiplayer game", right? @kraflab

Share this post


Link to post

Maybe a complevel for sigil? Each commercial/official wad that changes behavior tends have its own -complevel/-gameversion.

Edited by Catoptromancy

Share this post


Link to post
15 minutes ago, Catoptromancy said:

Maybe a complevel for sigil? Each commercial/official wad that changes behavior tends have its own -complevel/-gameversion.

No Rest For The Living doesn't (in its case, the changed behavior is the level order).

Share this post


Link to post
2 hours ago, fabian said:

So, the fix would be to revert this commit again and let killing the first Baron on Sigil E5M8 quit the level? Or maybe guard it with a conditional like "map E5M8 is from a WAD whose name starts with SIGIL and we are not recording or playing back a demo and are not in a multiplayer game", right?

I'd suggest the conditional exception to be "map is ExM8 where x > 4 from a WAD that contains an UMAPINFO lump".

Share this post


Link to post
3 hours ago, fabian said:

 

So, the fix would be to revert this commit again and let killing the first Baron on Sigil E5M8 quit the level? Or maybe guard it with a conditional like "map E5M8 is from a WAD whose name starts with SIGIL and we are not recording or playing back a demo and are not in a multiplayer game", right? @kraflab

Personally, I wouldn't have any condition for this in the engine and rather let people either use the compat wads or user patches. There are lots of wads with "zdoomisms" that are almost compatible but not, but we don't edit the engine to make them work. If you do want to have an exception, what you said is how I would do it.

Share this post


Link to post
2 hours ago, Catoptromancy said:

Maybe a complevel for sigil? Each commercial/official wad that changes behavior tends have its own -complevel/-gameversion.

We can't go creating complevels for any pwad that is incompatible with the engine, it's not sustainable 😉

Share this post


Link to post

Okay, I think I have solved this now, though a bit differently than proposed. Now I keep track if the 5th episode is from an auto-loaded Sigil PWAD and only then suppress the triggering of a boss action (and only during a singleplayer game):

 

https://github.com/fabiangreffrath/crispy-doom/commit/e68a08d10911875f9380d4ecbd27ce00821b236c

 

I hope that's it now. I feel a bit confused lately, sharing thoughts between three different source ports and their code bases...

 

Share this post


Link to post

The best thing to do still is:

 

2 hours ago, Gez said:

I'd suggest the conditional exception to be "map is ExM8 where x > 4 from a WAD that contains an UMAPINFO lump".

 

I'm not sure why anything else is being discussed? Why is this poorly-defined "default" being left in place to trip up future wads?

Share this post


Link to post

What puzzles me is what even warrants a discussion here. Vanilla has no episodes above 4, so what baseline does this have to be compatible with?

 

Share this post


Link to post

Semi-related: maybe this needs a split, or a new thread or something. Any proposed fixes should go into pr+um and then get pulled into dsda-doom later, so we're clogging up the wrong thread. :P

Share this post


Link to post

 

33 minutes ago, Xaser said:

Semi-related: maybe this needs a split, or a new thread or something. Any proposed fixes should go into pr+um and then get pulled into dsda-doom later, so we're clogging up the wrong thread. :P

I think this would be a good point to create an independent umapinfo specification itself rather than continue tying it to a particular port's implementation.

 

1 hour ago, Graf Zahl said:

What puzzles me is what even warrants a discussion here. Vanilla has no episodes above 4, so what baseline does this have to be compatible with?

There are lots of contexts that aren't in vanilla but nevertheless we stay compatible to the code as it was written. This case is no different. Of course, the goals of conservative ports are completely different from others. Anyway, this is why we have umapinfo.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now
×