/alwaysrandom 1 does not always give full armor

[bretonium]

Steps to reproduce:

1. do /alwaysrandom 1
2. Play several rounds and note the armor in the beginning

Expectation:
I always spawn with full armor, as if i chose "random character"

Observed:
Almost always i spawn with 0 armor

[Kaffeine]

The follow fix causes crashes:

diff --git src/game/server/infclass/infcplayer.cpp src/game/server/infclass/infcplayer.cpp
index 7c8c724d93..91a4c445c2 100644
--- src/game/server/infclass/infcplayer.cpp
+++ src/game/server/infclass/infcplayer.cpp
@@ -184,7 +184,13 @@ void CInfClassPlayer::SetClass(int newClass)
 .       .       SetCharacterClass(new(m_ClientID) CInfClassInfected(this));
 .       }
 
-.       m_pInfcPlayerClass->SetCharacter(GetCharacter());
+.       // Skip the SetCharacter() routine if the World ResetRequested because it
+.       // means that the Character is going to be destroyed during this
+.       // IGameServer::Tick() which also invalidates possible auto class selection.
+.       if(!GameServer()->m_World.m_ResetRequested)
+.       {
+.       .       m_pInfcPlayerClass->SetCharacter(GetCharacter());
+.       }
 .       m_pInfcPlayerClass->OnPlayerClassChanged();
 }

Probably this triggers some different bug.

Crash BT:

Backtrace:
./Infclass-Server(_ZN18CInfClassCharacter14TakeAllWeaponsEv+0x18)[0x44d488]
./Infclass-Server(_ZN14CInfClassHuman19GiveClassAttributesEv+0x9)[0x442329]
./Infclass-Server(_ZN15CInfClassPlayer10TryRespawnEv+0x82)[0x45f702]
./Infclass-Server(_ZN7CPlayer4TickEv+0x1a1)[0x479ce1]
./Infclass-Server(_ZN15CInfClassPlayer4TickEv+0x30)[0x45ff20]
./Infclass-Server(_ZN12CGameContext6OnTickEv+0x33a)[0x470f6a]
./Infclass-Server(_ZN7CServer3RunEv+0x6e1)[0x43b301]
./Infclass-Server(main+0x284)[0x429d84]
/lib64/libc.so.6(__libc_start_main+0xf5)[0x7f9d899cd505]
??:?(_start)[0x42a1aa]

[Kaffeine]

Delay this fix to v1.3.1.

[Kaffeine]

Better BT:

(gdb) bt
#0  0x0000000000483672 in CInfClassCharacter::TakeAllWeapons (this=0x0)
    at src/game/server/infclass/entities/infccharacter.cpp:1893
#1  0x000000000047453e in CInfClassPlayerClass::GiveClassAttributes (this=0x751d18 <ms_PoolDataCInfClassHuman+440>)
    at src/game/server/infclass/classes/infcplayerclass.cpp:293
#2  0x0000000000471c1e in CInfClassHuman::GiveClassAttributes (this=0x751d18 <ms_PoolDataCInfClassHuman+440>)
    at src/game/server/infclass/classes/humans/human.cpp:160
#3  0x00000000004740bf in CInfClassPlayerClass::SetCharacter (this=0x751d18 <ms_PoolDataCInfClassHuman+440>,
    character=0x757d90 <ms_PoolDataCInfClassCharacter+18480>)
    at src/game/server/infclass/classes/infcplayerclass.cpp:130
#4  0x000000000049782f in CInfClassPlayer::TryRespawn (this=0x770580 <ms_PoolDataCInfClassPlayer+10912>)
    at src/game/server/infclass/infcplayer.cpp:43
#5  0x00000000004b72bc in CPlayer::Tick (this=0x770580 <ms_PoolDataCInfClassPlayer+10912>)
    at src/game/server/player.cpp:132
#6  0x00000000004978c8 in CInfClassPlayer::Tick (this=0x770580 <ms_PoolDataCInfClassPlayer+10912>)
    at src/game/server/infclass/infcplayer.cpp:55
#7  0x000000000049cf52 in CGameContext::OnTick (this=0x7f653dda8010)
    at src/game/server/gamecontext.cpp:1111
#8  0x0000000000461f05 in CServer::Run (this=0x7f6539c44010)
    at src/engine/server/server.cpp:2318
#9  0x00000000004642fb in main (argc=3, argv=0x7fffce880118)
    at src/engine/server/server.cpp:3072

CInfClassPlayerClass::SetCharacter (frame #3):

void CInfClassPlayerClass::SetCharacter(CInfClassCharacter *character)
{
	if(m_pCharacter == character)
	{
		return;
	}

	if(m_pCharacter)
	{
		DestroyChildEntities();
		m_pCharacter->SetClass(nullptr);
	}

	m_pCharacter = character;

	if(m_pCharacter)
	{
		m_pCharacter->SetClass(this);
		GiveClassAttributes();
	}
}

void CInfClassPlayerClass::GiveClassAttributes()
{
	m_pCharacter->TakeAllWeapons();
}

character is 0x751d18. GiveClassAttributes() calls TakeAllWeapons() on m_pCharacter which is 0x0 at the moment of the crash. I don't see how this is possible.
I added an excessive check for m_pCharacter == nullptr into GiveClassAttributes() and hope for the best. 🙈

[Kaffeine]

OK, thanks to @bretonium we figured out the root of the issue.

Preconditions:

1. The player with /alwaysrandom 1 killed 1-2 seconds before the final explosion.
2. The character has 3 seconds timeout for the spawning.
3. Inf characters spawn is blocked if the final explosion is started.
4. During that final explosion, game ticks are still going on.

What happened next:
on the new round, the player has m_Spawning = true and m_RespawnTick <= Server()->Tick() (the respawn tick is at the number of the tick during the explosion and the server tick is still on the explosion over tick.

Then (in CGameContext::OnTick():

1. m_World.Tick(); does nothing (because the world is still paused.
2. m_pController->Tick(); checks for if(Server()->Tick() > m_GameOverTick+Server()->TickSpeed()*g_Config.m_InfShowScoreTime)
3. The score board time is over, so
4. The game controller calls StartRound().
5. The method calls ResetGame(), which does m_World.m_ResetRequested = true;
6. <all the other 'new round' machinery happens inside the controller>
7. CGameContext::OnTick() gets to the players and calls CPlayer::Tick().
8. The player spawns a new character (because all the conditions are met).
9. The player sets the new character for the player class (class NONE at this point) note this point
10. The new character opens map menu on the spawn.
11. The OpenClassChooser checks for /alwaysrandom option and sets another human class to the player
12. In the (human, e.g. LOOPER) class setter, the character is reset to nullptr due to the new logic "if the world reset is in progress then ignore the character which is going to die on this tick"
13. The player is trying to setup the newly spawned character from #9 and crashes because the character is now nullptr, despite of it being spawned a moment ago on this game Tick.

So:

1. The crash (due to the constant final explosion speed) had higher chances on bigger maps and lower chances on small maps. There are 10x more chances to catch this on infc_skull than on infc_hardcorepit.
2. The idea that "human must win to repro the bug" is not entirely correct. The last human can die due to the final explosion and the crash will still happen.
3. The idea that the crash happens on a human character was not correct (in fact, it happens only on infected classes).

There is no option to fix the world destroying order because it is a significant game logic chance and it causes a number of regressions and bugs in the teeworlds core code.

[Kaffeine]

Prevent character spawning in case of world.reset == true. This should help.

[Kaffeine]

b2bdde6 Deny spawn during the world reset
57e6d8e Ignore characters during the world reset