coelckers / prboom-plus

This is a cleaned up copy of the PrBoom+ SVN repository as a courtesy for those interested in forking that port
287 stars 115 forks source link

[Request] PlaySound codepointer support in all complevels (possibly via compatibility menu) #516

Closed SirBofu closed 2 years ago

SirBofu commented 2 years ago

Non-MBF complevels do not currently support some MBF codepointers. While this is likely to be expected, the absence of a compatibility setting for this feature is somewhat limiting for modders. Introducing an option similar to the sound quirks setting (or simply enabling support for the PlaySound setting.

While I'm sure there's some concern that this could cause demos to desync, I feel like this would only be the case in demos played using WADs with Dehacked patches that contain these codepointers in the first place, which I would think is an edge case. Given that complevel 2 still supports features like the Dropped Item property for Things and that Crispy Doom handles a lot of these codepointers without any apparent desync (recorded a demo in complevel 2 in PrBoom+, demo played back without issue in Crispy Doom, and vice versa), I think it would be nice to at least allow the option.

Thanks for your time.

kraflab commented 2 years ago

If a wad requires a higher compatibility level, then use the higher compatibility level. We're not going to change the behaviour of existing complevels, and compatibility flags only apply to mbf and higher anyway. Perhaps you could explain a bit more why modders are currently limited?

SirBofu commented 2 years ago

The PlaySound codepointer not functioning prevents modders from repurposing Things to act as ambient sound generators or overriding Dehacked monster's default attack sounds via states without targeting complevel 21, despite other features from DEHEXTRA functioning without issue. For instance, the following Dehacked block redefines the Wolfenstein SS:

Thing 24 (SSG Sergeant)
[...]
Dropped item = 79

Frame 736
Duration = 15

Frame 737
Sprite subnumber = 32773
Duration = 0

Frame 738
Sprite subnumber = 32773
Duration = 0

Frame 739
Sprite subnumber = 32773
Duration = 7
Unknown 1 = 4

Frame 740
Sprite subnumber = 32773
Duration = 0

Frame 741
Sprite subnumber = 4
Duration = 14
Next frame = 1089
Unknown 1 = 5

[...]

Frame 1089
Sprite number = 51
Sprite subnumber = 4
Duration = 14
Next frame = 1090
Unknown 1 = 5

Frame 1090
Sprite number = 51
Sprite subnumber = 4
Duration = 13
Next frame = 1091
Unknown 1 = 7

Frame 1091
Sprite number = 51
Sprite subnumber = 4
Duration = 11
Next frame = 728
Unknown 1 = 6

[CODEPTR]
FRAME 737 = SposAttack
FRAME 738 = SposAttack
FRAME 739 = PlaySound
FRAME 740 = FaceTarget
FRAME 741 = NULL
FRAME 1089 = PlaySound
FRAME 1090 = PlaySound
FRAME 1091 = PlaySound

In all complevels, the monster drops a Super Shotgun upon death (instead of the ammo clip that's hardcoded) and functions as expected with the exception of the sound. When run without specifying the command line parameter (or specifying complevel 21), the super shotgun firing sound can be heard on Frame 739, followed by the open, reload, and close sounds respectively on Frames 1089, 1090, and 1091. In complevel 2 and 9, the super shotgun sound does not occur; instead, the shotgun sound associated with the SposAttack command is heard. (Even if the SposAttack command is changed to NULL, the PlaySound pointers do not function.)

If this would utterly break compatibility, then I understand. It just seemed odd that the Dropped Item parameter works as is, but PlaySound requires a specific complevel to function. Complevel 2 is being targeted in the project because of admittedly minor reasons (not wanting monsters to get pushed off of ledges without having to use monster-blocking linedefs, monster infighting behavior, etc), so if this isn't feasible or the effort would be better used on other enhancements or fixes, then that's completely fair. I was just curious if this was a) intentional and b) necessary for compatibility.

kraflab commented 2 years ago

without targeting complevel 21 I mean, that's why we made mbf21 :^) I'm not sure exactly how it was decided to "leak" some features backwards in complevels when adding new dehacked stuff in the past, but I would rather avoid any further leaking - otherwise you end up in a situation where your features only work on certain versions of prboom+ and not in others. PRBoom+ is supposed to be consistent and static for this kind of behaviour.

SirBofu commented 2 years ago

Closing - latest complevel produces the desired effects, and targeting it is not breaking compatibility with other targeted source ports such as Crispy, GZDoom, etc. Will be using it going forward.