Death messages sometimes do not display

Issue #209 resolved
Colin Basnett created an issue

No description provided.

Comments (22)

  1. Colin Basnett reporter

    I'm starting to think that this isn't an issue with the death message display code. It could be something a bit more sinister; perhaps something to do with replication. I've noticed that sometimes, players are seemingly killed by "no one", and this is also reflected in the kill log in the console. It could be that the players doing the killing and/or the dying are not currently replicated on the client's end, which might explain the somewhat anomalous behavior of the death messages. Will look into this further.

  2. Andrew Theel

    I'm pretty sure it only uses the PRI, maybe how we are accessing the PRI to get the PRI info is the issue!

  3. Colin Basnett reporter

    This has been confirmed to be a server-side problem and not a client-side one.

    This can be replicated nearly 100% by killing players with mounted machine-guns (halftrack, marder et al).

    Can also be replicated with about 50% success by running the following commands in rapid succession (spawns bots on opposing teams and lets them kill each other):

    AddBots 32
    DebugSpawnBots 0
    DebugSpawnBots 1
    

    The underlying problem is that the DarkestHourGame.Killed is being passed none as the Killer for some reason. Time to find out why this is.

  4. Colin Basnett reporter

    Did more investigating. The problems stem from the new code put in place to track the InstigatorController in case the player moves to a new pawn.'

    When gunning down bots in the halftrack:

    Log("Instigator(0)" @ Instigator); //reports correct pawn
    UpdateInstigator();
    Log("Instigator(1)" @ Instigator); //reports `none`
    

    Since this was @Matt_UK's code, I'm assigning this to him.

  5. Andrew Theel

    Alright I'm gonna help look into this. Could the issue be something with clients setting their own InstigatorController

    in simulated function PostNetBeginPlay() of class DHBullet it has this line in a simulated function:

    if (Instigator != none) { InstigatorController = Instigator.Controller; }

    Does this if statement need something like this... Role == ROLE_Authority ?

  6. Andrew Theel

    Also I believe this issue has been over complicated as it is actually two different issues in one. Originally it had to do with not displaying the killer for many deaths. Now it seems it is another issue that makes some death messages not appear for some people. BTW another important thing is I'm am still not 100% sure that there is still an issue ever since we disabled PLT. I cannot replicate the issue with bots in singleplayer. More testing is needed.

  7. Matt Hands

    Been doing some testing/logging on this, using a 2nd computer to create true multi-player. I've found what's causing it - although what's causing that is a different matter. Some facts I can confirm:

    Basnett told me that it can easily be recreated in MP with only 2 players, so doesn't seem to be network overload related. Estimated maybe 1 time in 10 deaths. Is correct. Also told me that sometimes one player will see a particular DM and another player won't. Correct. That is easily re-cerated with 2 player MP. PLT is a red herring. My testing was with currently disabled PLT & the bug is easy to recreate. Also happens with shell kills, which have no PLT. The fact that one player sees a DM while another does not tells us for certain that the server is receiving the correct info, e.g. the Killer. Otherwise no player would see the DM. InstigatorController is another red herring. The original IC setting code was correct. IC is handled entirely serverside and the server receives the correct info. So the problem is either (a) the kill is not replicated correctly to the client, or (b) the client doesn't or can't handle the kill correctly when it receives it. I found that it only happened to my puny laptop connected as the 2nd player, never to by main PC. May or may not have any significance.

    The code flow is: Server.ROTeamGame.BroadcastDeathMessage // loops through ControllerList chain & finds each human player Controller (i.e. an ROPlayer) Server.ROPlayer.AddHudDeathMessage // replicated server-to-client function called separately on each ROPlayer (replication condition is bNetOwner) Server.DHHud.AddDeathMessage // adds an Obituary struct to the Obituaries array - if we get here then all works fine

    Now here's the deal - and it's a strange one: When the DM goes wrong for a particular client, the weird cause is that the replicated ROPlayer.AddHudDeathMessage function gets called on the server, instead of being replicated to that client. That's the problem - if the client doesn't receive the replicated function call, then no DM. Why that happens, well ......

    Logging shows the server calling AddHudDeathMessage on both players in turn. When it works, each client log shows the function being called. But when it goes wrong for one player, the server logs AddHudDeathMessage being called on itself immediately after what should be the call to that client. On the server, AddHudDeathMessage does nothing as it has no HUD actor. Appears to be a weird network timing issue. As a test, I set a 1 second timer on the server when it called AddHudDeathMessage on itself (as that is a sure sign of things going wrong), then had the timer call AddHudDeathMessage again, passing saved arguments. It worked - the 2nd time the AddHudDeathMessage function was replicated correctly to the client, so a slightly delayed DM showed. That's not a fix, it's a very hacky test to confirm the timing problem theory.

    I want to read up on how/when a server-to-client replicated function can get called on the server. I remember writing about that in our Slack coding channel.

    I also want to experiment by temporarily replacing the AddHudDeathMessage function with a DH version, without bNetOwner in the replication statement, to see if that makes the function reliably called on the net client. That's just a top of the head thought, without thinking it through !

  8. Matt Hands

    Well, the root cause seems to be the bNetOwner qualification in RO's AddHudDeathMessage() replication statement. bNetOwner has no reason being in a function replication statement; it is only relevant to replicated variables.

    Have replaced with new ClientAddHudDeathMessage() function, as cannot override a replication statement, so need to declare a new function with its own replication statement (without bNetOwner). BroadcastDeathMessage() function modified to only call new function. Seems to work fine, so will commit in a few minutes ! Another 12 year old bug .........

  9. Matt Hands

    Believe fixed in commit 69d414f, but we'll test before closing.

    While doing this, noticed bug in RO's BroadcastDeathMessage() function, with faulty logic affecting one death message mode. Fixed in same commit (issue logged and resolved, for the record). Two 12 year old bugs for the price of one.

    Believe we need to revert projectile's UpdateInstigator() function back to how originally written, as don't think connected with this problem & will now sometimes give faulty results. But will discuss separately with Basnett to avoid clogging this DM issue further.

    Fingers crossed !

  10. Andrew Theel

    I don't think this is a 12 yo bug, well perhaps with bullets. Though it should have been. I believe the delay in...the bullet code "// Matt: modified to remove delayed destruction of bullet on a server, as serves no purpose for a bullet" was some how making up for the issue. As I've played a lot of DH and I've also seen a lot of RO played, and have never noticed the bug. If not that, then something else in the logic changes have created the bug to at least be more noticeable. Regardless if the fix works, I think it's a better design/code than the RO guys had. Simply put TWI had a Churchill mentality with RO code: "Victory at all costs" while we have a mentality "Fuck TWI, this is how it's done" lol.

  11. Matt Hands

    I've just tried exactly the same test set up in DH 5.1 and it was easy to recreate the missing DMs for one player and not the other. Again it was my crappy laptop that got the problem, whether as killer or killed, and not the decent main PC.

    Theel and I spent some time testing the fix today and it seemed to work ok, both killing each other and blowing up huge hordes of bots.

    The removal of serverside bullet delayed destruction isn't relevant, as we discussed. The next comment line after the one you quoted above reads "// Bullet is bNetTemporary, meaning it gets torn off on client as soon as it replicates, receiving no further input from server, so delaying destruction on server has no effect". Once they have spawned, net temp actors like bullets have absolutely no connection between the server and client, nor any way of making one. The server's 0.1 sec delayed destruction was utterly pointless; the client is completely unaware of what any server bullet is doing. The serverside broadcasting of DMs is triggered by damage functionality, triggered when the bullet hits a player, and is unrelated to the bullet's subsequent destruction. Put simply, what happens to a bullet after it damages something is irrelevant and the DM functionality does not need a bullet. In any event, that old RO delayed destruction was only in ROBullet.HitWall() event and not in ProcessTouch(), which is what gets called when a projectile hits a player pawn, not HitWall(). And it wasn't ever in the shell class, which certainly also suffered from the DM bug. So stacks of reasons to show it isn't related in any way to DMs, but it does seem to work now, so all looks good ! And yeah, I think we do have much better code in many areas that the original game (the benefits of much more time to perfect stuff) !! :)

    We should be able to close this very soon.

  12. Log in to comment