Remove player from attack list upon death (Fixes a yellow skull issue) #2072
Conversation
| if (Player* player = attacker->getPlayer()) { | ||
| Party* party = player->getParty(); | ||
| if (Player* attackerPlayer = attacker->getPlayer()) { | ||
| if (targetPlayer && attackerPlayer->hasAttacked(targetPlayer)) { |
There was a problem hiding this comment.
Get the target player here instead of up above where it is long unneeded yet.
There was a problem hiding this comment.
Fine, I'll move it xD
| return; | ||
| } | ||
|
|
||
| auto it = std::find(attackedSet.begin(), attackedSet.end(), attacked->guid); |
There was a problem hiding this comment.
A set is implemented as a red-black tree and it's faster to traverse with attackedSet.find(attacked->guid) than the generic std::find.
|
|
||
| auto it = std::find(attackedSet.begin(), attackedSet.end(), attacked->guid); | ||
| if (it == attackedSet.end()) { | ||
| return; |
There was a problem hiding this comment.
You can save a few lines if you use != attackedSet.end() and erase in here.
|
Is there an issue reporting this bug? |
|
No, I don't think there is (I'm unable to find one at least). |
| return; | ||
| } | ||
|
|
||
| auto it = attackedSet.find(attacked->guid); |
There was a problem hiding this comment.
Note that hasAttacked that is called before this function already performs this check. You probably don't need to check hasAttacked then.
There was a problem hiding this comment.
I think it's better to be safe than sorry, as later it can be used in other context if need be
There was a problem hiding this comment.
@gugahoa he doesn't need to check if hasAttacked prior to calling removeAttacked, not the other way around
There was a problem hiding this comment.
Oh, sorry, I misunderstood your comment haha
| Player* targetPlayer = getPlayer(); | ||
| if (targetPlayer && attackerPlayer->hasAttacked(targetPlayer)) { | ||
| attackerPlayer->removeAttacked(targetPlayer); | ||
| } |
There was a problem hiding this comment.
See my comment below, I think those 4 lines above might be reduced to attackerPlayer->removeAttacked(getPlayer()), but please double check.
There was a problem hiding this comment.
Yep! You are absolutely right, I didn't really pay attention to that.
|
I think you just solved a dangling pointer issue :) |
|
Thanks! |
This should eliminate the possibility of receiving a yellow skull after killing another player (if none of them has any skull).