Doom/Doom2: Donut line action differs from vanilla

[ManxLoaghtan]

Background

Version of Chocolate Doom: built from Git repository 2023-02-05

Operating System and version: Windows

Game: Doom2

Any loaded WADs and mods (please include full command line):
chocolate-doom -file baddonut.wad

Bug description

Observed behavior:
The floor donut switch action (9) can break in chocolate-doom if it targets multiple sectors where at least one of them doesn't move. I made a map for doom2 in to demonstrate. Unlike vanilla and other accurate ports, the switch will not lower the exit platform. It seems confused by the inert sector outside the map to the right.
Download Example: https://anonfiles.com/zex6l3Waya

Expected behavior:
Hit the switch to trigger the donut and lower the exit. Both the pool and pillar are supposed to move to the surrounding floor height.

[fabiangreffrath]

and other accurate ports

Which ports would that be?

[ManxLoaghtan]

I tried it in a few ports as well as Doom2 version 1.9 through dosbox. It doesn't work in the latest chocolate or any close derivative like crispy (5.12.0x64) or inter doom (6.2.1x64). It works fine in Woof (10.5.1x64) and DSDA-Doom (0.25.2) even when specifying -complevel 2. It also works fine in Gzdoom with compatibility set to Doom (strict).
It also weirdly does not work in PrBoom-Plus (2.6.2) under any compat (the switch will not even animate), and it crashes Vavoom (1.33).

[rfomin]

The problem is in this line:

chocolate-doom/src/doom/p_spec.c

Line 1304 in 066f74f

break;

If we change break to continue it will work like in MBF/PrBoom+.

[fabiangreffrath]

Surprising to find a desyncing bug in Chocolate Doom after all these years - especially one that other ports did get right.

@rfomin While at it, it seems that we are still missing a certain overflow emulation in Woof, i.e. the case when s3 == NULL.

[rfomin]

Surprising to find a desyncing bug in Chocolate Doom after all these years - especially one that other ports did get right.

It seems check for s2 == NULL was added in Boom: https://github.com/Doom-Utils/historic-ports/blob/3ae2d25740b9d4ea896f0fde0c73f6b112135ce5/p_floor.c#L849
I think another overflow emulation is needed for true vanilla compatibility.

it seems that we are still missing a certain overflow emulation in Woof

I never found a map/demo that depends on it. Just a test file donut_av.wad in the PrBoom+ repository. Maybe we should advise @ManxLoaghtan to just not abuse this bug?

[fabiangreffrath]

It seems check for s2 == NULL was added in Boom: https://github.com/Doom-Utils/historic-ports/blob/3ae2d25740b9d4ea896f0fde0c73f6b112135ce5/p_floor.c#L849

Oh, andI think I introduced a bug in Woof when I fixed (!s2->lines[i]->flags & ML_TWOSIDED) to !(s2->lines[i]->flags & ML_TWOSIDED) a few lines later.

[rfomin]

(!s2->lines[i]->flags & ML_TWOSIDED) to !(s2->lines[i]->flags & ML_TWOSIDED)

Hmm, I don't know the actual order of evaluation here, but your fix looks logical.

[fabiangreffrath]

Hmm, I don't know the actual order of evaluation here, but your fix looks logical.

Yes, it does. 😉 But there's a comment in PrBoom+: The original code evaluates to 0 & ML_TWOSIDED.

[nveaa1]

Do you happen to have a new link to the wad file? The link in the report doesn't work now.

[ManxLoaghtan]

Do you happen to have a new link to the wad file? The link in the report doesn't work now.

Yes, I didn't notice at the time github supported zip uploads directly. This one should be much more permanent.
baddonut.zip