MBF21: SSG autoswitch causes weapons to disappear

[d-bind]

If the last two shells are fired immediately after automatically switching to SSG, the game will crash with an access violation. Similar to DSDA bug reported here, and the demos recorded with DSDA will cause a crash in Woof.

Unlike DSDA, in Woof the crash can be consistently reproduced without the need for tricky timing by using a custom dehacked SSG. To force a crash, the last two shells need to be fired separately (i.e. fired rather than refired). Also unlike DSDA, on playback, the demo recorded in Woof simply ends, without causing a crash (briefly shows "game saved (suppressed)" at the end).

Can't seem to replicate in versions prior to 11.0. Additionally, in 12.0 there seems to be some issue with autoswitching from the rocket launcher as well (stays up with 0 ammo, see demo).

Demos + custom SSG.

[fabiangreffrath]

At least the crash should be fixed since this commit https://github.com/fabiangreffrath/woof/commit/a6f4274553a751e61496f84ca900f0ee722e42f0

[d-bind]

Can confirm, the CI build plays the DSDA demos fine, but now consistently gets stuck without weapons with the dehacked shotgun. The rocket launcher also stays up. Edit: demo in the current build

[fabiangreffrath]

Can confirm, the CI build plays the DSDA demos fine, but now consistently gets stuck without weapons with the dehacked shotgun. The rocket launcher also stays up.

I assume the exact same happens in DSDA-Doom as well?

[d-bind]

Not entirely. DSDA requires exact timing, and the dehacked shotgun works fine there. It's possible that there's a bug in dehacked, I really can't say. But the behavior with disappearing weapons is exactly the same, yes. Also the RL autoswitch seems to only misfire in Woof.

[fabiangreffrath]

I guess they'll behave the same in this regard if "Boom Weapon Auto Switch" is toggled in DSDA's menu.

[d-bind]

That is correct. Gets stuck with custom shotgun with boom autoswitch set to Off. What makes it different from vanilla though, can it be addressed on the dehacked side?

[fabiangreffrath]

Appears to be a bug introduced by MBF21 and inherited to ports that support this complevel?

[d-bind]

I suppose so. Also it seems that replacing A_CheckAmmo with A_CheckReload fixed the custom shotgun. Not sure if it has other consequences.

[SirBofu]

I suspect the issue is in here (on woof/src/p_spr.c):

void A_CheckReload(player_t *player, pspdef_t *psp)
{
  if (!P_CheckAmmo(player) && mbf21)
  {
    // cph 2002/08/08 - In old Doom, P_CheckAmmo would start the weapon lowering
    // immediately. This was lost in Boom when the weapon switching logic was
    // rewritten. But we must tell Doom that we don't need to complete the
    // reload frames for the weapon here. G_BuildTiccmd will set ->pendingweapon
    // for us later on.
    boom_weapon_state_injection = true;
    P_SetPsprite(player, ps_weapon, weaponinfo[player->readyweapon].downstate);
  }
}

Looks like we're calling the downstate without actually selecting a weapon to switch to.

Altering the code to add this line should fix it, but we'll need to test demos in MBF21 in which the player runs out of ammo using the SSG:

void A_CheckReload(player_t *player, pspdef_t *psp)
{
  if (!P_CheckAmmo(player) && mbf21)
  {
    // cph 2002/08/08 - In old Doom, P_CheckAmmo would start the weapon lowering
    // immediately. This was lost in Boom when the weapon switching logic was
    // rewritten. But we must tell Doom that we don't need to complete the
    // reload frames for the weapon here. G_BuildTiccmd will set ->pendingweapon
    // for us later on.
    boom_weapon_state_injection = true;
++    player->pendingweapon = P_SwitchWeapon(player);
    P_SetPsprite(player, ps_weapon, weaponinfo[player->readyweapon].downstate);
  }
}

It's also possible this will need to be locked behind a new compatibility setting, especially if DSDA goes that route. That being said, I'm skeptical about the need to preserve this current behavior unless it introduces desyncs in existing demos that don't show off the bug. I don't think anyone will be terribly offended if the demos they made to demonstate it stop working if it means they don't run into the bug anymore.

[fabiangreffrath]

Thanks for investigating this issue! Whatever solution we apply, we need to make sure that it's the same fix in Woof and DSDA. I agree that losing the demos which demonstrate this bug is not a problem, but the switch will most probably have to be coupled to a new complevel - so MBF21 will still exhibit it.

Maybe your proposed solution needs an explicit check e.g. if player->pendingweapon > wp_supershotgun.

[SirBofu]

Adding the check (and a comp setting check) definitely works in both Woof and DSDA when I test it. Since consistency with DSDA is needed, I'll hold off on making a pull request until there's some consensus.

It could theoretically be temporarily introduced as a compatibility-breaking option similar to the Blockmap fix option, too.