From cfdbf00afebc79607473ea59c6a70395158812fd Mon Sep 17 00:00:00 2001
From: arenacraftwow <61932925+arenacraftwow@users.noreply.github.com>
Date: Wed, 1 Apr 2020 20:16:00 +0200
Subject: [PATCH] fix(Core/Hook): OnFirstLogin can now make persistent changes
 (#2793)

PlayerScript#OnFirstLogin was broken in that if you tried to teach the
player a spell or give an item to him that change wouldn't persist.
Meaning that the next time the Player would log in, his awarded spells
or items would be gone.

Meanwhile PlayerScript#OnLogin didn't have this behavior meaning
that the sematics of PlayerScript#OnFirstLogin and PlayerScript#OnLogin
where very different, which is unexpected.

This issue was caused by an implementation detail in Player#learnSpell
(and similiar) which behaves differently depending on
Player#isBeingLoaded phase. This code change makes its that
PlayerOnFirstLogin hook executes when Player#isBeingLoaded == false
fixing the issue.

Alternatives: The fact that Player#learnSpells and others behave
differently depending on the load phase is questionable and from my
research it seems like this impl detail is only used when giving the
player his template spells. We could add a overload to
Player#learnSpell with a flag like `isSpellTemporary` and make
the template spells loading use it. Then we would be able to remove
this implicit temporary spell mechanism when Player#isBeingLoaded
thing.  However this is a deeper change and not worth the risk.
---
 src/server/game/Handlers/CharacterHandler.cpp | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/server/game/Handlers/CharacterHandler.cpp b/src/server/game/Handlers/CharacterHandler.cpp
index 7dd7fae12..171ee44af 100644
--- a/src/server/game/Handlers/CharacterHandler.cpp
+++ b/src/server/game/Handlers/CharacterHandler.cpp
@@ -1070,12 +1070,6 @@ void WorldSession::HandlePlayerLoginFromDB(LoginQueryHolder* holder)
         SendNotification(LANG_RESET_TALENTS);
     }
 
-    if (pCurrChar->HasAtLoginFlag(AT_LOGIN_FIRST)) {
-        pCurrChar->RemoveAtLoginFlag(AT_LOGIN_FIRST);
-
-        sScriptMgr->OnFirstLogin(pCurrChar);
-    }
-
     if (pCurrChar->HasAtLoginFlag(AT_LOGIN_CHECK_ACHIEVS))
     {
         pCurrChar->RemoveAtLoginFlag(AT_LOGIN_CHECK_ACHIEVS, true);
@@ -1202,6 +1196,13 @@ void WorldSession::HandlePlayerLoginFromDB(LoginQueryHolder* holder)
     }
 
     sScriptMgr->OnPlayerLogin(pCurrChar);
+
+    if (pCurrChar->HasAtLoginFlag(AT_LOGIN_FIRST)) 
+    {
+        pCurrChar->RemoveAtLoginFlag(AT_LOGIN_FIRST);
+        sScriptMgr->OnFirstLogin(pCurrChar);
+    }
+
     delete holder;
 }