[Bug Fix] Using %T in channel messages on fresh corpse yields mob, not corpse name.

[noudess]

The problem:

Kill a PC or NPC. Target the corpse. The client shows the corpse name as xyz's_corpse. But the result of using %T in channels is just the npc or player name without the ''s corpse' addition. If you zone out and back in, then %T works as expected.

I broke down testing this to the two different cases:

MOB:

The only way I could make the client honor %T was to do a corpse->Spawn() after we do AddCorpse to the entity list. AddCorpse ends up setting the new name, but the client doesn't see this without me adding corpse->Spawn(). We do not get (2) corpses, it just seems to fix the (1).

Concerns:

• If we don't use a backtick in the corpse name, but rather use a tick - the client doesnt fix it. Seems really odd to me. I tested if it was just a matter of changing any character. Nothing I did worked other that using a backtick instead of a tick. Maybe somewhere at some packet levels the tick is getting stripped?

PC:

[ Could not get this to function. The rename worked but caused multiple side effects on the client. Not worth it for just displaying correct name in macros. ]

@neckkola credited for the rename fix. I had uses a spawn packet instead.

[neckkola]

I can take a look in the next few days. See what I can add.

[hgtw]

Using %T on live behaves the same way. Even if you re-enter the zone it only outputs the name without "corpse" appended. If it used to work I'd suspect some unnecessary spawn packets were removed at some point.

As far as client behavior, %t uses the target's DisplayedName (mq variable naming) which is a cached stripped/cleaned name. Other things like Hail, UI labels, etc trims the raw Name of underscore/numbers and uses that. OP_Death appends 's corpse to the raw name when it does its work of setting the entity to corpse type etc. The corpse string doesn't get appended to DisplayedName here since it breaks the loop when it encounters an apostrophe.

Also for what it's worth live uses an apostrophe e.g., name's_corpse0 (always 0 even if there's multiple corpses of the same name) for the spawn packet when entering a zone that has corpses. This is likely why even re-entering a zone on live doesn't change %T behavior since setting DisplayedName trims anything after encountering the apostrophe.

Sending a spawn packet just to change the name is a bit heavy so it's probably not the right thing to do. If this is really desired I'd recommend adding a rule (since this wouldn't be live like) and just sending a mob rename packet for the corpse. The rule would have to switch server side names to use backticks since apostrophes are trimmed. The spawn packet essentially only does a rename anyway if it detects a player (spawn entity) id already exists in the zone and returns early after setting the name. This wouldn't be as heavy as sending a full spawn packet.

[noudess]

Using %T on live behaves the same way. Even if you re-enter the zone it only outputs the name without "corpse" appended. If it used to work I'd suspect some unnecessary spawn packets were removed at some point.

As far as client behavior, %t uses the target's DisplayedName (mq variable naming) which is a cached stripped/cleaned name. Other things like Hail, UI labels, etc trims the raw Name of underscore/numbers and uses that. OP_Death appends 's corpse to the raw name when it does its work of setting the entity to corpse type etc. The corpse string doesn't get appended to DisplayedName here since it breaks the loop when it encounters an apostrophe.

Also for what it's worth live uses an apostrophe e.g., name's_corpse0 (always 0 even if there's multiple corpses of the same name) for the spawn packet when entering a zone that has corpses. This is likely why even re-entering a zone on live doesn't change %T behavior since setting DisplayedName trims anything after encountering the apostrophe.

Sending a spawn packet just to change the name is a bit heavy so it's probably not the right thing to do. If this is really desired I'd recommend adding a rule (since this wouldn't be live like) and just sending a mob rename packet for the corpse. The rule would have to switch server side names to use backticks since apostrophes are trimmed. The spawn packet essentially only does a rename anyway if it detects a player (spawn entity) id already exists in the zone and returns early after setting the name. This wouldn't be as heavy as sending a full spawn packet.

I tried using the OP_MobRename packet in several different ways. None changed the behavior. Only going back to corpse->Spawn() do I get the results I need. I'm happy to discuss how I did it, or try a suggestion beyond those. @hgtw

Regarding the rule, are we sure we want that? I know this worked at some point on live, and on eqemu. I know we strive to be live like, but who would "choose" to not be able to use %T on a corpse for commands and have them work?

I greatly appreciate you looking into this! It explains why the backtick is needed to get the change to take place.

[neckkola]

I have ran a few tests and have had some success in sending a name update to correct the problem. Will test further and report back. The rename is linked the corpseid in the deathpacket. Travelling today, though will try and get something up for you to test @noudess soon.

[hgtw]

Are we sure live ever output the corpse suffix with %t? Grepping through some old logs from both 2006 and 2014 I'm seeing cases where a heal hotkey was clearly pressed on someone that just died and it just shows their name without the corpse.

It also looks like an apostrophe has always been used for corpses from hail messages not a backtick so this would make sense if the name has always been stripped of everything after an apostrophe (at least looking at 2003 and 2005 clients). The only thing I see in older logs with a backtick is "Venril`s Corpse" which is probably some old venril sathir`s remains quest npc spawn?

edit: It doesn't bother me if it needs to be a rule or not, someone else can decide. It will probably come down to a difference in using backticks vs apostrophes depending on how the rename testing works out.

[noudess]

I have ran a few tests and have had some success in sending a name update to correct the problem. Will test further and report back. The rename is linked the corpseid in the deathpacket. Travelling today, though will try and get something up for you to test @noudess soon.

I saw that the mob version of Kill_Struct did not fill in corpseid, felt odd, but didnt mess with that aspect.

[noudess]

Are we sure live ever output the corpse suffix with %t? Grepping through some old logs from both 2006 and 2014 I'm seeing cases where a heal hotkey was clearly pressed on someone that just died and it just shows their name without the corpse.

It also looks like an apostrophe has always been used for corpses from hail messages not a backtick so this would make sense if the name has always been stripped of everything after an apostrophe (at least looking at 2003 and 2005 clients). The only thing I see in older logs with a backtick is "Venrils Corpse" which is probably some old venril sathirs remains quest npc spawn?

edit: It doesn't bother me if it needs to be a rule or not, someone else can decide. It will probably come down to a difference in using backticks vs apostrophes depending on how the rename testing works out.

My recollections from live come from being having dozens of %T macros. Its just instilled in me that I can tell someone else about a corpse in /gui, and I would have noticed if the tells used the real name not the corpse's name. But as well all know memory can be selective, so I only say that I'd bet $1000 on it, but I could lose.

[noudess]

Is there any evidence in the client code that BodyType was checked and changed client behaviour (ie not stripping off corpse for corpses)?

[hgtw]

Is there any evidence in the client code that BodyType was checked and changed client behaviour (ie not stripping off corpse for corpses)?

Only the spawn type is checked to make sure the entity id is not already a corpse when handling the death packet. It also checks for npc vs pc to form the string a little different (if npc it checks if the class is an object and just appends an id instead of "corpse", it also does something with setting any numbers to 0 in the suffix).

I don't see anything else that would not strip off 's_corpse for %T. DisplayedName is just a cached stripped name in rof2. Older clients called stripName when handling %T while rof2 just caches it when changing the name in the death packet and returns the cached version for %T.

There's also a couple places where the client searches entity names for the substr 's corpse as a fallback if it doesn't find by actual name in some group/raid member code and another place it generates locale eqstr messages if the string contains 's corpse. If client corpses on emu have always been sent with a backtick then whatever these fallbacks are for have probably never worked anyway though. I think they're for some role/leadership stuff.

edit: I had it backwards and npcs were the ones sent with backtick, not pcs

[neckkola]

@noudess , I put together this branch for one version of a potential fix. https://github.com/neckkola/Server/tree/%25T_Updates

It uses the MobRename packet to adjust the stored named used in the %T after the death packet is done. Have not done any regression testing so not sure if there are side effects. Interested in your thoughts and approach.

[noudess]

@hgtw The patch @neckkola works using rename, but still requires a backtick for the new name. So we can get away from the spawn packet.

PC corpse can be fixed the same way, but it creates a bug when PC loots his own corpse with no drop confirmers on.

Working to decide if we should implement this only for NPCs, can fix PC issues, or just leave this whole issue as is.

[joligario]

Currently on PEQ yet

[Akkadius]

@hgtw @noudess @neckkola @joligario what's the status on this guy?

[noudess]

@hgtw @noudess @neckkola @joligario what's the status on this guy?

@neckkola is still looking for time to resolve side effect on looting PC corpses.

If it can't be resolved cleanly, we will either discard this PR or go with the "fix" for NPC corpses only.

[noudess]

Currently on PEQ yet

I believe this to be a separate issue. Maybe @neckkola can answer this one?

[noudess]

@neckkola and I both saw too many issues with PC corpse rename. This works fine for NPC corpses, aside from the name looking a little funky. That said, it already was in place for looking at corpses after you zone in (the corpse list uses the backtick already) so this just makes corpses look the same to those already in the zone as opposed to those new people zoning in.

I'm ready for a merge/approval on this change,

[noudess]

@neckkola You ok with this PR? Basically your code. Its been working locally.

[hgtw]

Since server side is currently already using a backtick for npc corpses it probably does make sense to update the corpse name here on clients.

I still think we should rule this off at some point to use apostrophes just in the interest of live/client accuracy (and less packets) but if it's always been like this then I don't see it hurting anything.

[noudess]

I think this is ready for merge.