1. torr.samaho
  2. zandronum
  3. Pull requests

Pull requests

#174 Declined
Repository
zalewa
Branch
sv_survival_nomapresetondeath_2
Repository
Torr_Samaho
Branch
default

sv_survival_nomapresetondeath zadmflag (addresses 2941).

Author
  1. Zalewa
Reviewers
Description

This new zadmflag controls map reset when all players lose lives in survival. When enabled, upon death of the last player, all dead players and all players in the queue will receive full amount of max lives and will be able to continue play on the map from the point where they died.

If this flag is enabled then Inventory of dead spectators is carried over between respawns and between maps.

This also fixes the problem where player corpse disappears into thin air upon losing the final life.

Comments (11)

    1. torr.samaho repo owner

      Before I look at the changes, I have a question regarding the aim of the patch: What's the point of playing survival, if you can just continue after everybody lost all lives?

      1. Zalewa author

        This enables a more casual version of survival.

        1. If you die before everyone else you still need to wait, hence the "casual survival" mode is not completely pointless. Of course, if people agree that "if anyone of us dies, we all suicide" then it is pointless but then again they can just play cooperative.
        2. It's an option that I will have use for and it doesn't harm anyone who doesn't want to use it. (EDIT: by this I mean that maybe others will find it useful too, you never know that you need something until you get it.)
        1. torr.samaho repo owner

          It's an option that I will have use for and it doesn't harm anyone who doesn't want to use it.

          A rarely used option is surely fine, as long as its code is not expected to get in the way in the future (e.g. when ZDoom once again decides to completely revamp some internal design). The part that just prevents the map reset seems to be short and simple enough not to get in the way (I still have to check it more closely though), but the part that handles the inventory change is quite large and I would feel much more confident regarding future maintenance if we can drastically simplify this.

          The option to give dead spectators their inventory back after a map change sounds very useful by itself though (and doesn't turn survival essentially into coop :P) and I think it's worth to spend some more thinking on it. Instead of trying to store all the inventory items somewhere, it may be better to simply let the dead spectator body keep the items and have the ZDoom respawn code handle everything else. Spectators are deprived of their inventory to make sure that mods don't abuse any leftover inventory items, but having an option that lets dead spectators keep their inventory (true spectators should still lose all inventory) should be fine.

          1. Zalewa author

            We've noticed that when player loses the final life, his corpse disappears into thin air after finishing the death animation.

            This happens because clientside removes corpses for dead spectators in cl_main.cpp in the else block here (priorState is PST_LIVE):

                    if (( priorState == PST_REBORN ) ||
                        ( priorState == PST_REBORNNOINVENTORY ) ||
                        ( priorState == PST_ENTER ) ||
                        ( priorState == PST_ENTERNOINVENTORY ))
                    {
                        // [BB] This will eventually free the player's body's network ID.
                        G_QueueBody (pOldActor);
                        pOldActor->player = NULL;
                        pOldActor->id = -1;
                    }
                    else
                    {
                        pOldActor->Destroy( );
                        pOldActor->player = NULL;
                        pOldActor = NULL;
                    }
            

            Serverside inventory is destroyed in p_interaction.cpp here:

                    // Take away all of the player's inventory.
                    // [BB] Needs to be done before G_DoReborn is called for dead spectators. Otherwise ReadyWeapon is not NULLed.
                    pPlayer->mo->DestroyAllInventory( );
            

            And then the transfer from the oldactor to p->mo is ignored by p_mobj.cpp in the else if here:

                if (deathmatch)
                { // Give all cards in death match mode.
                    p->mo->GiveDeathmatchInventory ();
                }
                // [BC] Don't filter coop inventory in teamgame mode.
                // [BB] Also don't do so if the player changed the player class.
                else if ((( NETWORK_GetState( ) != NETSTATE_SINGLE ) || (level.flags2 & LEVEL2_ALLOWRESPAWN) ) && state == PST_REBORN && oldactor != NULL && ( teamgame == false ) && ( oldPlayerClass == p->CurrentPlayerClass ) )
                { // Special inventory handling for respawning in coop
                    p->mo->FilterCoopRespawnInventory (oldactor);
                }
                if (oldactor != NULL)
                { // Remove any inventory left from the old actor. Coop handles
                  // it above, but the other modes don't.
                    oldactor->DestroyAllInventory();
                }
            

            The else if block above doesn't get executed because state is set to PST_LIVE for dead spectators by the same code in p_interaction.cpp that does DestroyAllInventory( ).

            From what I gathered the inventory is stored in player's corpse and moved to the new body upon respawn. Dead spectators shouldn't have any inventory to avoid abuse you mentioned. Hence, it's necessary to somehow store the inventory of a dead player where it would be inaccessible by the world. At first I tried to disassociate the AInventory objects from their APlayerPawn and just move them to a separate deadSpectatorInventory pointer but this was causing numerous problems with HUD and crashes and map travel issues and many vulgarisms were uttered. The freeze in the inventory_storage_t was the simplest solution I came up with. It's the bulk of this change, that's true, but it's also purely new code that doesn't mutate ZDoom's original code. As I mentioned in my other comment, this part can be moved to separate source files where it will be completely separate from ZDoom code. Hell, the player_t structure doesn't even need to know that there's an inventory_storage_t. It will however be necessary to put an #include somewhere.

            Most of the rest of the added code is already next to Zandronum's bDeadSpectator and NETWORK_GetState( ).

            Perhaps it might be possible to unfreeze the dead spectator inventory sooner, before P_SpawnPlayer( ). We can unfreeze the inventory in GAMEMODE_RespawnDeadSpectatorsAndPopQueue( ), transfer it to current mo and then allow the original ZDoom code obtain inventory from this mo, provided that player state is set to PST_REBORN. All of this sounds like another gross hack.

            The one thing I definitely don't like is the copy&paste of the code responsible for removal of items during level travel in form of inventory_storage_t::RemoveLevelChangeItems( ) method. Ideally, this method should be removed and the problem should be fixed by respawning the dead spectator just on the map end in G_PlayerFinishLevel(), consuming his inventory storage, and letting the original ZDoom code purge the inventory from the APlayerPawn.

            1. torr.samaho repo owner

              We've noticed that when player loses the final life, his corpse disappears into thin air after finishing the death animation.

              Yeah, I remember that somebody (perhaps Edward-san?) mentioned this before. I'm not sure if he already worked on a patch for this. The behavior is not intentional and it would be nice to fix it.

              Dead spectators shouldn't have any inventory to avoid abuse you mentioned. Hence, it's necessary to somehow store the inventory of a dead player where it would be inaccessible by the world.

              While we should prevent abuse, letting the dead spectator body keep the items may be an option. Currently, the inventory is completely removed because it was the easiest way to prevent abuse. I never thought of giving dead spectators their inventory back when the respawn.

              It's the bulk of this change, that's true, but it's also purely new code that doesn't mutate ZDoom's original code.

              Yes, the code is nicely encapsulated, but that's not what I was worrying about. The code relies heavily on ZDoom's internal API and data structures. Unfortunately, ZDoom has the tendency to drastically change its inner working whenever the ZDoom devs see fit. That's why I think we should at least investigate whether it is feasibly to have ZDoom's existing player spawning code can do all the inventory storage and filtering for us.

  1. Zalewa author

    So, this still isn't fully ready yet, but I'm updating the PR for code review purposes.

    Dead spectators will now hold on to their inventory. And then respawn with this inventory in accordance to the current game mode and inventory preservation settings. Of course, this introduced several peculiar bugs. These are the ones I was able to find without extensive testing:

    • In Heretic, dead spectators can use inventory items (probably applies to all usable items, regardless of the game)
    • Naturally, HUD displays the inventory for dead spectators.
    • Using mouse scroll to change weapons print weapon names on the HUD (although weapons don't raise).
    • Dead players can now use drop CCMD to remove their inventory. Dropped items appear at player's corpse and other players can take them.
    • Needs testing: iced corpses, because that 'if' check was altered.
    1. Zalewa author

      Ok, all points should be fixed with the most recent update of this PR.

      Frankly speaking, I don't think this will be any more resilient to ZDoom's internal changes than the previous change. We can only hope that compiler will show code that breaks when ZDoom base is updated.

  2. torr.samaho repo owner

    Unfortunately, bitbucket hid all in-code comments when the pull request was updated. The comments are still visible under activity and I'll reply in there.