transhumandesign / kag-base

King Arthur's Gold base folder.
257 stars 119 forks source link

No resupplies when starting a new match #1344

Closed mugg91 closed 1 year ago

mugg91 commented 2 years ago

Description

One problem I see commonly in build 3900 is when I spawn in a new match as a builder I get no materials but the timer says 40 seconds remaining. I guess that the resupplies spawned but they were given to my teammate instead, because they overlap with my character.

Perhaps it's happening because my character had a full inventory in the previous match and the code in CTF_GiveSpawnItems.as SetMaterials() thinks my inventory is full, therefore merely places it at my position.

It should be checked if this specific issue was fixed by the recent resupply fix PR(s).

Reproduction

Happens sometimes when a match ends and you are builder, then spawn into the next match with no materials. Maybe happens always if your inventory was full in the previous match. Needs testing.

mugg91 commented 2 years ago

This is still present in the current "4137 hotfix" build. It happened to me and one other player at the same time, when a new match started in CTF EU.

mugg91 commented 1 year ago

I notice often that when a player joins in Sandbox server, they will appear on one side, then disappear and leave materials behind, then appear on the other side. Scoreboard shows 0 deaths.

Maybe the same thing happens in CTF servers; game first places you on one side, then removes you and you drop your mats, then you appear on the other side with no mats.

mugg91 commented 1 year ago

Here are my results so far. https://i.imgur.com/48IqiqA.png Now just need to find out why the blob dies and work around that. Maybe add a "is eligible for spawn materials" tag as soon as we know the blob is there and will not be removed and then check for that in CTF_GiveSpawnItems.as.

mugg91 commented 1 year ago

Tried to find the root cause of the blob being made to die in the bug scenario (see the image linked above).

1) (...) First blob is created and materials are added to its inventory 2) AddPlayerSpawn() is called from onPlayerRequestSpawn() in CoreHooks.as 3) AddPlayerSpawn() executes RemovePlayerBlob() (in RulesCore.as) 4) RemovePlayerBlob() executes blob.server_Die(); - first blob dies, second blob has to spawn and bug happens

Tried to figure out why onPlayerRequestSpawn() happens. There are two client_RequestSpawn() calls in CTF_PickSpawn.as. Of these two, one is called by BuildRespawnMenu() at client-side at game time 0. The bug happens at about tick 11 to 14 (if going to a new map) so I don't think that's causing it. I was told there are more client_RequestSpawn() calls in engine-side so the root cause may be possible to explore on the engine-side.

Gingerbeard5773 commented 1 year ago

The bug happens when the player's team is changed on next map. If the player's team stays the same throughout maps the bug doesn't happen.

asumagic commented 1 year ago

The onPlayerRequestSpawn hook is being invoked, seemingly as a result to the team swap in engine, i.e. the first time the player spawns, they already are in the correct team.
This happens after "10+ ticks" in your case because it seems to be lag dependent (localhost is like 1, 200ms emulated ping is ~22).

This hook triggers AddPlayerSpawn, which triggers RemovePlayerBlob. The reason why the bug is inconsistent is because the player blob is sometimes ready by that point and sometimes isn't. As to why that is true even on localhost is beyond me, though. It makes sense that gingerbeard's workaround works then because it doesn't trigger the spurious kill.

ChangePlayerTeam ends on

        RemovePlayerBlob(player);

        u8 oldteam = player.getTeamNum();
        p.setTeam(newTeamNum);
        player.server_setTeamNum(newTeamNum);

        // vvv this breaks spawning on team change ~~~Norill
        //if(is_spawning || oldteam == rules.getSpectatorTeamNum())
        {
            respawns.AddPlayerToSpawn(player);
        }

If you check AddPlayerSpawn, which is triggered by the team swap, you will notice there is a lastSpawnRequest check. AddPlayerToSpawn does not set it, but AddPlayerSpawn does at the end. So changing ChangePlayerTeam to use it instead seems more robust anyway.

Thoughts from those who touched that code? cc @mugg91 @Gingerbeard5773

Gingerbeard5773 commented 1 year ago

The concern is that using the lastSpawnRequest check could not work if the player has really high ping, which would be solved by making the request time threshold longer, though I wouldn't opt for that.

asumagic commented 1 year ago

I'll check, but I don't think this is the case. IIRC no matter your ping it happens one tick after you spawned.

Gingerbeard5773 commented 1 year ago

The first game-start spawn always happens on tick zero (because its done by server), but the second spawn (caused by team change) relies on the client instead so the tick time differs. I was seeing 15-22 ticks after game start that my second spawn came in.

asumagic commented 1 year ago

I'll have to double check but my observation was that there isn't a first spawn of that sort. I could be wrong though.

asumagic commented 1 year ago

I emulated heavy lag (200ms + 200ms variance) and got this:

[14:43:18] Changing player team Asu @1
[14:43:18] Changing player team Henry~3 @1
[14:43:18] Changing player team Henry~2 @1
[14:43:18] Changing player team Henry @1
[14:43:18] spawned 235 @3
[14:43:18] onSetPlayer235 Henry~2
[14:43:18] spawned 238 @7
[14:43:18] onSetPlayer238 Henry~3
[14:43:18] spawned 241 @12
[14:43:18] onSetPlayer241 Asu
[14:43:18] onPlayerRequestSpawn for Asu @12
[14:43:18] spawned 244 @18
[14:43:18] onSetPlayer244 Henry
[14:43:19] spawned 247 @28
[14:43:19] onSetPlayer247 Asu

So my proposed fix wouldn't work, I think. Meh.