fix(Core/Characters): Fix crash on characters deletion with COD mails.

[walkline]

Changes Proposed:

This PR proposes changes to:

• [x] Core (units, players, creatures, game systems).
• [ ] Scripts (bosses, spell scripts, creature scripts).
• [ ] Database (SAI, creatures, etc).

Issues Addressed:

• Closes https://github.com/azerothcore/azerothcore-wotlk/issues/18741

SOURCE:

The changes have been validated through:

• [ ] Live research (checked on live servers, e.g Classic WotLK, Retail, etc.)
• [ ] Sniffs (remember to share them with the open source community!)
• [ ] Video evidence, knowledge databases or other public sources (e.g forums, Wowhead, etc.)
• [ ] The changes promoted by this pull request come partially or entirely from another project (cherry-pick). Cherry-picks must be committed using the proper --author tag in order to be accepted, thus crediting the original authors, unless otherwise unable to be found

Tests Performed:

This PR has been:

• [x] Tested in-game by the author.
• [ ] Tested in-game by other community members/someone else other than the author/has been live on production servers.
• [ ] This pull request requires further testing and may have edge cases to be tested.

How to Test the Changes:

• [x] This pull request can be tested by following the reproduction steps provided in the linked issue
• [ ] This pull request requires further testing. Provide steps to test your changes. If it requires any specific setup e.g multiple players please specify it as well.

Known Issues and TODO List:

• [ ]
• [ ]

How to Test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).

For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.

[pangolp]

What operating systems did you test on, and are the steps the same as for the other pull request? Please, if you can complete and give more information, we could ask or perform tests. Complete if possible the fields (even if not all) but the necessary ones so that whoever reads the contribution, knows how to test it. Thank you.

[walkline]

@pangolp The steps to reproduce from your issue (https://github.com/azerothcore/azerothcore-wotlk/issues/18741) are valid, but I could reproduce it only on Windows (but not on macOS).

[pangolp]

@pangolp The steps to reproduce from your issue (https://github.com/azerothcore/azerothcore-wotlk/issues/18741) are valid, but I could reproduce it only on Windows (but not on macOS).

The problem was apparently always Windows. I thought it was something global, but it seems not.

[PkllonG]

The server didn't crash after PR (windows10 x64)

[pangolp]

The server didn't crash after PR (windows10 x64)

Is the character deleted without deleting the mail then?

[PkllonG]

The server didn't crash after PR (windows10 x64)

Is the character deleted without deleting the mail then?

Press the test in your video, crash before PR, don't crash after PR

[pangolp]

The server didn't crash after PR (windows10 x64)

Is the character deleted without deleting the mail then?

Press the test in your video, crash before PR, don't crash after PR

Beyond the fix, which I have to try, today the emulator does not fail. Unless you had the emulator without updating or using the command as mentioned in the topic. I promise to try it now in a while, because as I mentioned before, the important thing is that it is solved, regardless of who solves it.

[sudlud]

just tested this PR

• no crash when deleting the character with pending COD mail
• after character deletion the pending mail still exists in table acore_characters.mail and still references the guid of the now-deleted character

is this the expected result?

[heyitsbench]

is this the expected result?

From what I can tell, that is the expected and correct result. Based on old forum posts, it does seem to be possible to delete a character with mail destined to it and the mail would return after thirty days.

[sudlud]

is this the expected result?

From what I can tell, that is the expected and correct result. Based on old forum posts, it does seem to be possible to delete a character with mail destined to it and the mail would return after thirty days.

when I login with the mail sender after deletion of the mail receiver, the mail is actually instantly returned.

[sudlud]

also no crash when using .character erase

[pangolp]

I just tested on Windows 10, and apparently, with this change, the crash no longer occurs. However I wonder what happens with the mail... if after 30 days, it returns to the player who sent it, or if in the future, we will have to review that issue as well, but for the moment, I would say that this change is fine. In the rest of the operating systems was it also tested?

[sudlud]

I just tested on Windows 10, and apparently, with this change, the crash no longer occurs. However I wonder what happens with the mail... if after 30 days, it returns to the player who sent it, or if in the future, we will have to review that issue as well, but for the moment, I would say that this change is fine. In the rest of the operating systems was it also tested?

Please see my earlier comment - the mail gets returned instantly when the mail sender logs in again.

[pangolp]

I just tested on Windows 10, and apparently, with this change, the crash no longer occurs. However I wonder what happens with the mail... if after 30 days, it returns to the player who sent it, or if in the future, we will have to review that issue as well, but for the moment, I would say that this change is fine. In the rest of the operating systems was it also tested?

Please see my earlier comment - the mail gets returned instantly when the mail sender logs in again.

I did 2 tests. In the first one, what you said happened, however, it seemed to me that the money was not returned, only the item. So I took the test again, and since the email didn't arrive, I continued with another topic and that's why I asked. Did the gold also reach you? Either way, it may be cause for another pull request. It doesn't have to be repaired right here. But we would have to take it into account to at least create the issue so that the precedent of the failure remains and then we try to repair it.

[sudlud]

This also confused me, because I didn't understand what "C.O.D." Mail is. You actually send an item and request a certain amount of gold from the receiver, that he has to pay to get the item from the mail.

So the returning mail does not have any gold in it, just the item.

[pangolp]

This also confused me, because I didn't understand what "C.O.D." Mail is. You actually send an item and request a certain amount of gold from the receiver, that he has to pay to get the item from the mail.

So the returning mail does not have any gold in it, just the item.

Exact. In case of non-payment or deletion, both must be returned. But I'm just saying this, so that we create the issue and then resolve it in another pull request. This pull request seems to solve the problem and should be merged. But let's not forget to do the problem, to review and solve the other problem.

[NexXuZR]

Tested on Windows 10, great job. Thx.