Rework buff callbacks

The first implementation was way too complicated and prone to bugs.
This is much more simple flexible; first off, std::function is now used
instead of a raw function pointer, so lambdas and binds are fair game
which is great for scripting. Second, callbacks for all stacks are
executed. It is up to the callback target to ensure correct behavior.

src/Buffs.cpp

@@ -2,67 +2,58 @@
 
 #include "PlayerManager.hpp"
 
-#include <assert.h>
-#include <set>
-
 using namespace Buffs;
 
-void Buff::onTick() {
-    assert(!stacks.empty());
-
-    std::set<BuffCallback> callbacks;
+void Buff::tick() {
     auto it = stacks.begin();
     while(it != stacks.end()) {
         BuffStack& stack = *it;
-        if(stack.onTick != nullptr && callbacks.count(stack.onTick) == 0) {
-            // unique callback
-            stack.onTick(self, &stack);
-            callbacks.insert(stack.onTick);
-        }
+        if(stack.onTick) stack.onTick(self, &stack);
 
         if(stack.durationTicks > 0) stack.durationTicks--;
         if(stack.durationTicks == 0) {
             it = stacks.erase(it);
-            if(stack.onExpire != nullptr) stack.onExpire(self, &stack);
+            if(stack.onExpire) stack.onExpire(self, &stack);
         } else it++;
     }
 }
 
-void Buff::onExpire() {
-    assert(!stacks.empty());
-
-    std::set<BuffCallback> callbacks;
+void Buff::clear() {
     while(!stacks.empty()) {
         BuffStack stack = stacks.back();
         stacks.pop_back();
-        if(stack.onExpire != nullptr && callbacks.count(stack.onExpire) == 0) {
-            // execute unique callback
-            callbacks.insert(stack.onExpire);
-            stack.onExpire(self, &stack);
-        }
+        if(stack.onExpire) stack.onExpire(self, &stack);
     }
 }
 
 void Buff::addStack(BuffStack* stack) {
+    if(stack->onApply) stack->onApply(self, stack);
     stacks.push_back(*stack);
 }
 
-BuffStack* Buff::getDominantBuff() {
-    assert(!stacks.empty());
-
-    BuffStack* dominant = nullptr;
+bool Buff::hasClass(BuffClass buffClass) {
     for(BuffStack& stack : stacks) {
-        if(stack.buffClass > dominant->buffClass)
-            dominant = &stack;
+        if(stack.buffClass == buffClass)
+            return true;
     }
-    return dominant;
+    return false;
+}
+
+BuffClass Buff::maxClass() {
+    BuffClass buffClass = BuffClass::NONE;
+    for(BuffStack& stack : stacks) {
+        if(stack.buffClass > buffClass)
+            buffClass = stack.buffClass;
+    }
+    return buffClass;
 }
 
 bool Buff::isStale() {
     return stacks.empty();
 }
 
-static void timeBuffUpdate(EntityRef self, BuffStack* buff, int type) {
+#pragma region Handlers
+void Buffs::timeBuffUpdate(EntityRef self, BuffStack* buff, int status) {
 
     if(self.kind != EntityKind::PLAYER)
         return; // not implemented
@@ -70,16 +61,22 @@ static void timeBuffUpdate(EntityRef self, BuffStack* buff, int type) {
     Player* plr = (Player*)self.getEntity();
     if(plr == nullptr)
         return;
+
+    if(status == ETBU_DEL && plr->hasBuff(buff->id))
+        return; // no premature status removal!
+
+    int cbf = plr->getCompositeCondition();
+    if(status == ETBU_ADD) cbf |= CSB_FROM_ECSB(buff->id);
     
     INITSTRUCT(sP_FE2CL_PC_BUFF_UPDATE, pkt);
     pkt.eCSTB = buff->id; // eCharStatusTimeBuffID
-    pkt.eTBU = type; // eTimeBuffUpdate
+    pkt.eTBU = status; // eTimeBuffUpdate
     pkt.eTBT = (int)buff->buffClass;
-    pkt.iConditionBitFlag = plr->getCompositeCondition();
+    pkt.iConditionBitFlag = cbf;
     self.sock->sendPacket((void*)&pkt, P_FE2CL_PC_BUFF_UPDATE, sizeof(sP_FE2CL_PC_BUFF_UPDATE));
 }
 
-static void timeBuffTimeoutViewable(EntityRef self) {
+void Buffs::timeBuffTimeoutViewable(EntityRef self, BuffStack* buff, int ct) {
     if(self.kind != EntityKind::PLAYER)
         return; // not implemented
     
@@ -88,21 +85,15 @@ static void timeBuffTimeoutViewable(EntityRef self) {
         return;
 
     INITSTRUCT(sP_FE2CL_CHAR_TIME_BUFF_TIME_OUT, pkt); // send a buff timeout to other players
-    pkt.eCT = 1;
+    pkt.eCT = ct; // 1 for eggs, at least
     pkt.iID = plr->iID;
     pkt.iConditionBitFlag = plr->getCompositeCondition();
     PlayerManager::sendToViewable(self.sock, pkt, P_FE2CL_CHAR_TIME_BUFF_TIME_OUT);
 }
 
-void Buffs::timeBuffUpdateAdd(EntityRef self, BuffStack* buff) {
-    timeBuffUpdate(self, buff, ETBU_ADD);
-}
-
-void Buffs::timeBuffUpdateDelete(EntityRef self, BuffStack* buff) {
-    timeBuffUpdate(self, buff, ETBU_DEL);
-}
-
+/* MOVE TO EGG LAMBDA
 void Buffs::timeBuffTimeout(EntityRef self, BuffStack* buff) {
     timeBuffUpdate(self, buff, ETBU_DEL);
     timeBuffTimeoutViewable(self);
-}
+} */
+#pragma endregion

src/Buffs.hpp

@@ -5,6 +5,7 @@
 #include "Entities.hpp"
 
 #include <vector>
+#include <functional>
 
 /* forward declaration(s) */
 struct BuffStack;
@@ -21,7 +22,7 @@ enum class BuffClass {
 	CASH_ITEM = ETBT_CASHITEM
 };
 
-typedef void (*BuffCallback)(EntityRef, BuffStack*);
+typedef std::function<void(EntityRef, BuffStack*)> BuffCallback;
 
 struct BuffStack {
     int id; // ECSB
@@ -29,8 +30,13 @@ struct BuffStack {
     EntityRef source;
     BuffClass buffClass;
 
+    /* called just before the stack is added */
     BuffCallback onApply;
+
+    /* called when the stack is ticked */
     BuffCallback onTick;
+
+    /* called just after the stack is removed */
     BuffCallback onExpire;
 };
 
@@ -40,20 +46,17 @@ private:
     std::vector<BuffStack> stacks;
 
 public:
-    void onTick();
-    void onExpire();
+    void tick();
+    void clear();
     void addStack(BuffStack* stack);
 
     /*
-    * Why do this madness? Let me tell you why.
-    * We need to be able to distinguish whether a player's
-    * buff is from something like an egg vs. their nano,
-    * because there are cases where the behavior is different.
-    * Now, this is impossible to do when they all get composited
-    * to a single bitfield. So we use a "buff class" and pick
-    * the buff stack that is most "dominant".
+    * Sometimes we need to determine if a buff
+    * is covered by a certain class, ex: nano
+    * vs. coco egg in the case of infection protection
     */
-    BuffStack* getDominantBuff();
+    bool hasClass(BuffClass buffClass);
+    BuffClass maxClass();
 
     /* 
      * In general, a Buff object won't exist
@@ -71,7 +74,6 @@ public:
 };
 
 namespace Buffs {
-    void timeBuffUpdateAdd(EntityRef self, BuffStack* buff);
-    void timeBuffUpdateDelete(EntityRef self, BuffStack* buff);
-    void timeBuffTimeout(EntityRef self, BuffStack* buff);
+    void timeBuffUpdate(EntityRef self, BuffStack* buff, int status);
+    void timeBuffTimeoutViewable(EntityRef self, BuffStack* buff, int ct);
 }

src/Combat.cpp

@@ -12,6 +12,7 @@
 
 #include <assert.h>
 #include <iostream>
+#include <functional>
 
 using namespace Combat;
 
@@ -20,37 +21,35 @@ std::map<int32_t, std::map<int8_t, Bullet>> Combat::Bullets;
 
 void Player::addBuff(BuffStack* buff) {
     EntityRef self = PlayerManager::getSockFromID(iID);
-    if(!hasBuff(buff->id)) {
-        buffs[buff->id] = new Buff(self, buff);
-    }
+    if(!hasBuff(buff->id)) buffs[buff->id] = new Buff(self, buff);
     else buffs[buff->id]->addStack(buff);
-    buff->onApply(self, buff);
 }
 
-BuffStack* Player::getBuff(int buffId) {
+Buff* Player::getBuff(int buffId) {
     if(hasBuff(buffId)) {
-        return buffs[buffId]->getDominantBuff();
+        return buffs[buffId];
     }
     return nullptr;
 }
 
 void Player::removeBuff(int buffId) {
     if(hasBuff(buffId)) {
-        buffs[buffId]->onExpire();
+        buffs[buffId]->clear();
         delete buffs[buffId];
         buffs.erase(buffId);
     }
 }
 
 bool Player::hasBuff(int buffId) {
-    return buffs.find(buffId) != buffs.end();
+    auto buff = buffs.find(buffId);
+    return buff != buffs.end() && !buff->second->isStale();
 }
 
 int Player::getCompositeCondition() {
     int conditionBitFlag = 0;
-    for(auto buffEntry : buffs) {
-        if(!buffEntry.second->isStale())
-            conditionBitFlag |= CSB_FROM_ECSB(buffEntry.first);
+    for(auto buff : buffs) {
+        if(!buff.second->isStale())
+            conditionBitFlag |= CSB_FROM_ECSB(buff.first);
     }
     return conditionBitFlag;
 }
@@ -82,7 +81,7 @@ void Player::step(time_t currTime) {
 
 void CombatNPC::addBuff(BuffStack* buff) { /* stubbed */ }
 
-BuffStack* CombatNPC::getBuff(int buffId) { /* stubbed */
+Buff* CombatNPC::getBuff(int buffId) { /* stubbed */
     return nullptr;
 }
 
@@ -366,9 +365,14 @@ static void dotDamageOnOff(CNSocket *sock, CNPacketData *data) {
             -1, // infinite
             sock, // self-inflicted
             BuffClass::ENVIRONMENT,
-            Buffs::timeBuffUpdateAdd,
-            nullptr, // client ticks for us! todo anticheat lol
-            Buffs::timeBuffUpdateDelete
+
+            [](EntityRef host, BuffStack* stack)  {
+                Buffs::timeBuffUpdate(host, stack, ETBU_ADD);
+            },
+            nullptr, // client toggles for us! todo anticheat lol
+            [](EntityRef host, BuffStack* stack)  {
+                Buffs::timeBuffUpdate(host, stack, ETBU_DEL);
+            },
         };
         plr->addBuff(&infection);
     } else if(!pkt->iFlag && plr->hasBuff(ECSB_INFECTION)) {
@@ -387,13 +391,13 @@ static void dealGooDamage(CNSocket *sock, int amount) {
     sP_FE2CL_CHAR_TIME_BUFF_TIME_TICK *pkt = (sP_FE2CL_CHAR_TIME_BUFF_TIME_TICK*)respbuf;
     sSkillResult_DotDamage *dmg = (sSkillResult_DotDamage*)(respbuf + sizeof(sP_FE2CL_CHAR_TIME_BUFF_TIME_TICK));
 
-    BuffStack* protectionBuff = plr->getBuff(ECSB_PROTECT_INFECTION);
+    Buff* protectionBuff = plr->getBuff(ECSB_PROTECT_INFECTION);
     if (protectionBuff != nullptr) {
         amount = -2; // -2 is the magic number for "Protected" to appear as the damage number
         dmg->bProtected = 1;
 
         // eggs allow protection without nanos
-        if (protectionBuff->buffClass == BuffClass::NANO && plr->activeNano != -1)
+        if (protectionBuff->maxClass() <= BuffClass::NANO && plr->activeNano != -1)
             plr->Nanos[plr->activeNano].iStamina -= 3;
     } else {
         plr->HP -= amount;
@@ -799,8 +803,12 @@ static void playerTick(CNServer *serv, time_t currTime) {
         while(it != plr->buffs.end()) {
             int buffId = (*it).first;
             Buff* buff = (*it).second;
-            buff->onTick();
-            if(buff->isStale()) it = plr->buffs.erase(it);
+            buff->tick();
+            if(buff->isStale()) {
+                // garbage collect
+                it = plr->buffs.erase(it);
+                delete buff;
+            }
             else it++;
         }
         //

src/Entities.hpp

@@ -93,7 +93,7 @@ public:
     virtual ~ICombatant() {}
 
     virtual void addBuff(BuffStack* buff) = 0;
-    virtual BuffStack* getBuff(int buffId) = 0;
+    virtual Buff* getBuff(int buffId) = 0;
     virtual void removeBuff(int buffId) = 0;
     virtual bool hasBuff(int buffId) = 0;
     virtual int getCompositeCondition() = 0;
@@ -159,7 +159,7 @@ struct CombatNPC : public BaseNPC, public ICombatant {
     virtual bool isExtant() override { return hp > 0; }
 
     virtual void addBuff(BuffStack* buff) override;
-    virtual BuffStack* getBuff(int buffId) override;
+    virtual Buff* getBuff(int buffId) override;
     virtual void removeBuff(int buffId) override;
     virtual bool hasBuff(int buffId) override;
     virtual int getCompositeCondition() override;

src/Items.cpp

@@ -497,9 +497,14 @@ static void itemUseHandler(CNSocket* sock, CNPacketData* data) {
         durationMilliseconds / MS_PER_PLAYER_TICK,
         sock,
         BuffClass::CASH_ITEM, // or BuffClass::ITEM?
-        Buffs::timeBuffUpdateAdd,
-        nullptr,
-        Buffs::timeBuffUpdateDelete,
+
+        [](EntityRef host, BuffStack* stack)  {
+            Buffs::timeBuffUpdate(host, stack, ETBU_ADD);
+        },
+        nullptr, // client toggles for us! todo anticheat lol
+        [](EntityRef host, BuffStack* stack)  {
+            Buffs::timeBuffUpdate(host, stack, ETBU_DEL);
+        },
     };
     player->addBuff(&gumballBuff);
 

src/Player.hpp

@@ -90,7 +90,7 @@ struct Player : public Entity, public ICombatant {
     virtual void disappearFromViewOf(CNSocket *sock) override;
 
     virtual void addBuff(BuffStack* buff) override;
-    virtual BuffStack* getBuff(int buffId) override;
+    virtual Buff* getBuff(int buffId) override;
     virtual void removeBuff(int buffId) override;
     virtual bool hasBuff(int buffId) override;
     virtual int getCompositeCondition() override;