Implement .weekendxp config command and allow rates to be floats (#30)

* Implement `.weekendxp config` command and allow rates to be `float`s

Also changed the existing strings to use `{}` rather than the c-like formatters, since they seemed to not work in `master`

* Implement mod version migration system and general fixes

* Fix error string since now the range [0,1] is allowed for xp rates
* Add a changelog to the cpp file
* Add migration system so that players that previously changed their personal rate dont need to change it after the mod is updated
* Identify and note some potential improvements

* Remove changelog from source and unnecessary comments

Author: OAguinagalde

data/sql/db-world/mod-weekend-xp-texts.sql

@@ -1,9 +1,14 @@
 SET @STRING_ENTRY := 11120;
-DELETE FROM `acore_string` WHERE `entry` IN  (@STRING_ENTRY+0,@STRING_ENTRY+1);
+DELETE FROM `acore_string` WHERE `entry` IN  (@STRING_ENTRY+0,@STRING_ENTRY+1,@STRING_ENTRY+2);
 INSERT INTO `acore_string` (`entry`, `content_default`) VALUES
-(@STRING_ENTRY+0, 'Your experience rates were set to %i.'),
-(@STRING_ENTRY+1, 'Wrong value specified. Please specify a value between 1 and %i');
+(@STRING_ENTRY+0, 'Your experience rates were set to {}.'),
+(@STRING_ENTRY+1, 'Wrong value specified. Please specify a value between 0 and {}'),
+(@STRING_ENTRY+2, 'The rate being applied to you is {}.\nThe current weekendxp configuration is:\nAnnounce {}\nAlwaysEnabled {}\nQuestOnly {}\nMaxLevel {}\nxpAmount {}\nIndividualXPEnabled {}\nEnabled {}\nMaxAllowedRate {}');
 
 DELETE FROM `command` WHERE `name` IN ('weekendxp rate');
 INSERT INTO `command` (`name`, `security`, `help`) VALUES
 ('weekendxp rate', 0, 'Syntax: weekendxp rate $value \nSet your experience rate up to the allowed value.');
+
+DELETE FROM `command` WHERE `name` IN ('weekendxp config');
+INSERT INTO `command` (`name`, `security`, `help`) VALUES
+('weekendxp config', 0, 'Syntax: weekendxp config\nDisplays the current configuration for the weekendxp mod.');

src/mod-double-xp-weekend.cpp

@@ -10,122 +10,236 @@ enum WeekendXP
 {
     SETTING_WEEKEND_XP_RATE = 0,
     SETTING_WEEKEND_XP_DISABLE = 1,
+    SETTING_WEEKEND_XP_VERSION = 2,
 
     LANG_CMD_WEEKEND_XP_SET   = 11120,
     LANG_CMD_WEEKEND_XP_ERROR = 11121,
+    LANG_CMD_WEEKEND_XP_CONFIG = 11122,
 
     WD_FRIDAY   = 5,
     WD_SATURDAY = 6,
     WD_SUNDAY   = 0,
 };
 
-class weekendxp_commandscript : public CommandScript
+class DoubleXpWeekend
 {
+
 public:
-    weekendxp_commandscript() : CommandScript("weekendxp_commandscript") { }
+    
+    DoubleXpWeekend() { }
+    
+    // NOTE We need to access the DoubleXpWeekend logic from other
+    // places, so keep all the logic accessible via a singleton here,
+    // and have the `CommandScript` and the `PlayeScript` access this
+    // for the functionality they need.
+    static DoubleXpWeekend* instance()
+    {
+        static DoubleXpWeekend instance;
+        return &instance;
+    }
 
-    ChatCommandTable GetCommands() const override
+    uint32 OnGiveXP(Player* player, uint32 originalAmount, uint8 xpSource) const
     {
-        static ChatCommandTable commandTable =
+        if (!IsEventActive())
         {
-            { "weekendxp rate", HandleSetXPBonusCommand, SEC_PLAYER, Console::No },
-        };
+            return originalAmount;
+        }
 
-        return commandTable;
+        if (ConfigQuestOnly() && xpSource != PlayerXPSource::XPSOURCE_QUEST && xpSource != PlayerXPSource::XPSOURCE_QUEST_DF)
+        {
+            return originalAmount;
+        }
+
+        if (player->GetLevel() >= ConfigMaxLevel())
+        {
+            return originalAmount;
+        }
+
+        float newAmount = (float)originalAmount * GetExperienceRate(player);
+        return (uint32) newAmount;
     }
 
-    static bool HandleSetXPBonusCommand(ChatHandler* handler, int8 rate)
+    void OnLogin(Player* player, ChatHandler* handler) const
+    {
+        // TODO I am assuming that this is always called when a character logs in...
+        // if that is not the case, thing migh get weird... Adding some asserts or warnings would be nice
+        // but I'm not sure how to handle "This shouldn't be happening but it is" kind of scenarios in acore
+        MigratePlayerSettings(player, handler);
+
+        if (ConfigAnnounce())
+        {
+            if (IsEventActive() && !ConfigAlwaysEnabled())
+            {
+                float rate = GetExperienceRate(player);
+                handler->PSendSysMessage("It's the weekend! Your XP rate has been set to: {}", rate);
+            }
+            else if (IsEventActive() && ConfigAlwaysEnabled())
+            {
+                float rate = GetExperienceRate(player);
+                handler->PSendSysMessage("Your XP rate has been set to: {}", rate);
+            }
+            else
+            {
+                handler->PSendSysMessage("This server is running the |cff4CFF00Double XP Weekend |rmodule.");
+            }
+        }
+    }
+
+    bool HandleSetXPBonusCommand(ChatHandler* handler, float rate) const
     {
         Player* player = handler->GetPlayer();
 
-        int8 maxRate = sConfigMgr->GetOption<int8>("XPWeekend.MaxAllowedRate", 2);
+        float maxRate = ConfigMaxAllowedRate();
 
-        if (rate <= 0 || rate > maxRate)
+        if (rate <= 0.0f || rate > maxRate)
         {
             handler->PSendSysMessage(LANG_CMD_WEEKEND_XP_ERROR, maxRate);
             handler->SetSentErrorMessage(true);
             return true;
         }
 
-        player->UpdatePlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_RATE, rate);
+        PlayerSettingSetRate(player, rate);
         handler->PSendSysMessage(LANG_CMD_WEEKEND_XP_SET, rate);
 
+        // TODO if the `EnablePlayerSettings` is not set, the setting wont be remembered by the
+        // server after the player logs out, meaning the player needs to do this again on next login
+
         return true;
     }
-};
 
-class DoubleXpWeekend : public PlayerScript
-{
-public:
-    DoubleXpWeekend() : PlayerScript("DoubleXpWeekend") { }
+    bool HandleGetCurrentConfigCommand(ChatHandler* handler) const
+    {
+        Player* player = handler->GetPlayer();
 
-    void OnLogin(Player* player) override
+        const float actualRate = GetExperienceRate(player);
+        const bool isAnnounceEnabled = ConfigAnnounce();
+        const bool isAlwaysEnabled = ConfigAlwaysEnabled();
+        const bool isQuestOnly = ConfigQuestOnly();
+        const uint32 maxLevel = ConfigMaxLevel();
+        const float xpRate = ConfigxpAmount();
+        const bool isIndividulaXpEnabled = ConfigIndividualXPEnabled();
+        const bool isEnabled = ConfigEnabled();
+        const float maxXpRate = ConfigMaxAllowedRate();
+
+        handler->PSendSysMessage(LANG_CMD_WEEKEND_XP_CONFIG,
+            actualRate,
+            isAnnounceEnabled,
+            isAlwaysEnabled,
+            isQuestOnly,
+            maxLevel,
+            xpRate,
+            isIndividulaXpEnabled,
+            isEnabled,
+            maxXpRate
+        );
+
+        return true;
+    }
+
+private:
+
+    // NOTE keep options together to prevent having more than 1 potential default value
+    bool ConfigAlwaysEnabled() const { return sConfigMgr->GetOption<bool>("XPWeekend.AlwaysEnabled", false); }
+    bool ConfigAnnounce() const { return sConfigMgr->GetOption<bool>("XPWeekend.Announce", false); }
+    bool ConfigQuestOnly() const { return sConfigMgr->GetOption<bool>("XPWeekend.QuestOnly", false); }
+    uint32 ConfigMaxLevel() const { return sConfigMgr->GetOption<uint32>("XPWeekend.MaxLevel", 80); }
+    float ConfigxpAmount() const { return sConfigMgr->GetOption<float>("XPWeekend.xpAmount", 2.0f); }
+    bool ConfigIndividualXPEnabled() const { return sConfigMgr->GetOption<bool>("XPWeekend.IndividualXPEnabled", false); }
+    bool ConfigEnabled() const { return sConfigMgr->GetOption<bool>("XPWeekend.Enabled", false); }
+    float ConfigMaxAllowedRate() const { return sConfigMgr->GetOption<float>("XPWeekend.MaxAllowedRate", 2.0f); }
+
+    void PlayerSettingSetRate(Player* player, float rate) const
     {
-        if (sConfigMgr->GetOption<bool>("XPWeekend.Announce", false))
-        {
-            if (IsEventActive() && !sConfigMgr->GetOption<bool>("XPWeekend.AlwaysEnabled", false))
-            {
-                ChatHandler(player->GetSession()).PSendSysMessage("It's the weekend! Your XP rate has been set to: {}", GetExperienceRate(player));
-            }
-            else if (IsEventActive() && sConfigMgr->GetOption<bool>("XPWeekend.AlwaysEnabled", false))
-            {
-                ChatHandler(player->GetSession()).PSendSysMessage("Your XP rate has been set to: {}", GetExperienceRate(player));
-            }
-            else
-            {
-                ChatHandler(player->GetSession()).PSendSysMessage("This server is running the |cff4CFF00Double XP Weekend |rmodule.");
-            }
-        }
+        // HACK PlayerSetting seems to store uint32 only, so save our `float` as if it was a `uint32`
+        uint32 encodedRate;
+        float* reinterpretingPointer = (float*)&encodedRate;
+        *reinterpretingPointer = rate;
+        player->UpdatePlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_RATE, encodedRate);
+    }
+    
+    float PlayerSettingGetRate(Player* player) const
+    {
+        uint32 rateStored = player->GetPlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_RATE).value;
+        // HACK PlayerSetting seems to store uint32 only, so save our `float` as if it was a `uint32`
+        float rate = *(float*)&rateStored;
+        return rate;
     }
 
-    void OnGiveXP(Player* player, uint32& amount, Unit* /*victim*/, uint8 xpSource) override
+    void MigratePlayerSettings(Player* player, ChatHandler* /*handler*/) const
     {
-        if (!IsEventActive())
+        static const uint32 VERSION = 1;
+
+        uint32 playersCurrentVersion = player->GetPlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_VERSION).value;
+        bool validMigration = playersCurrentVersion == 0 && VERSION == 1;
+        if (!validMigration) 
         {
+            // Currently there is only either version 0 (the default) or 1
+            // This check should never fail unless a new version is introduced and the
+            // migration here is not updated.
             return;
         }
 
-        if (sConfigMgr->GetOption<bool>("XPWeekend.QuestOnly", false) && xpSource != PlayerXPSource::XPSOURCE_QUEST && xpSource != PlayerXPSource::XPSOURCE_QUEST_DF)
+        // On version 1 the only thing to migrate is the SETTING_WEEKEND_XP_RATE
+        float newRate = ConfigxpAmount();
+        uint32 originalRate = player->GetPlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_RATE).value;
+        if (originalRate <= 0)
         {
-            return;
+            // player setting was never set before, just use default
         }
-
-        if (player->GetLevel() >= sConfigMgr->GetOption<uint32>("XPWeekend.MaxLevel", 80))
+        else if ((float)originalRate > ConfigMaxAllowedRate())
         {
-            return;
+            // player setting was set but the value is not valid, just use default
+        }                
+        else
+        {
+            // player setting was set, use the same rate
+            newRate = (float) originalRate;
         }
 
-        amount *= GetExperienceRate(player);
+        // HACK PlayerSetting seems to store uint32 only, so save our `float` as if it was a `uint32`
+        uint32 encodedRate;
+        float* reinterpretingPointer = (float*)&encodedRate;
+        *reinterpretingPointer = newRate;
+        player->UpdatePlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_RATE, encodedRate);
+        player->UpdatePlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_VERSION, VERSION);
     }
-
-    int8 GetExperienceRate(Player * player) const
+    
+    // TODO why is there a `GetDisable` player setting but no way to actually modify it? Leaving as is for now...
+    bool PlayerSettingGetDisable(Player* player) const
     {
-        int8 rate = sConfigMgr->GetOption<int8>("XPWeekend.xpAmount", 2);
+        return player->GetPlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_DISABLE).value == (uint32)1;
+    }
 
-        int8 individualRate = player->GetPlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_RATE).value;
+    float GetExperienceRate(Player* player) const
+    {
+        float rate = ConfigxpAmount();
 
-        if (player->GetPlayerSetting("mod-double-xp-weekend", SETTING_WEEKEND_XP_DISABLE).value)
+        if (PlayerSettingGetDisable(player))
         {
-            return 1;
+            return 1.0f;
         }
 
         // If individualxp setting is enabled... and a rate was set, overwrite it.
-        if (sConfigMgr->GetOption<bool>("XPWeekend.IndividualXPEnabled", false) && individualRate)
+        if (ConfigIndividualXPEnabled())
         {
-            rate = individualRate;
+            rate = PlayerSettingGetRate(player);
         }
 
         // Prevent returning 0% rate.
-        return rate ? rate : 1;
+        return rate > 0.0f ? rate : 1.0f;
     }
 
     bool IsEventActive() const
     {
-        if (sConfigMgr->GetOption<bool>("XPWeekend.AlwaysEnabled", false))
+        if (ConfigAlwaysEnabled())
+        {
             return true;
+        }
 
-        if (!sConfigMgr->GetOption<bool>("XPWeekend.Enabled", false))
+        if (!ConfigEnabled())
+        {
             return false;
+        }
 
         time_t t = time(nullptr);
         tm* now = localtime(&t);
@@ -134,8 +248,57 @@ class DoubleXpWeekend : public PlayerScript
     }
 };
 
+class weekendxp_commandscript : public CommandScript
+{
+public:
+    weekendxp_commandscript() : CommandScript("weekendxp_commandscript") { }
+
+    ChatCommandTable GetCommands() const override
+    {
+        static ChatCommandTable commandTable =
+        {
+            { "weekendxp rate", HandleSetXPBonusCommand, SEC_PLAYER, Console::No },
+            { "weekendxp config", HandleGetCurrentConfigCommand, SEC_PLAYER, Console::No },
+        };
+
+        return commandTable;
+    }
+
+    static bool HandleSetXPBonusCommand(ChatHandler* handler, float rate)
+    {
+        DoubleXpWeekend* mod = DoubleXpWeekend::instance();
+        return mod->HandleSetXPBonusCommand(handler, rate);
+    }
+
+    static bool HandleGetCurrentConfigCommand(ChatHandler* handler)
+    {
+        DoubleXpWeekend* mod = DoubleXpWeekend::instance();
+        return mod->HandleGetCurrentConfigCommand(handler);
+    }
+};
+
+class DoubleXpWeekendPlayerScript : public PlayerScript
+{
+public:
+    DoubleXpWeekendPlayerScript() : PlayerScript("DoubleXpWeekend") { }
+
+    void OnLogin(Player* player) override
+    {
+        DoubleXpWeekend* mod = DoubleXpWeekend::instance();
+        ChatHandler handler = ChatHandler(player->GetSession());
+        mod->OnLogin(player, &handler);
+    }
+
+    void OnGiveXP(Player* player, uint32& amount, Unit* /*victim*/, uint8 xpSource) override
+    {
+        DoubleXpWeekend* mod = DoubleXpWeekend::instance();
+        amount = mod->OnGiveXP(player, amount, xpSource);
+    }
+
+};
+
 void AdddoublexpScripts()
 {
-    new DoubleXpWeekend();
+    new DoubleXpWeekendPlayerScript();
     new weekendxp_commandscript();
 }