COMMITTED: NPC signal update

[Tabasco]

It's a pretty rare case, but I've seen threads related to the issue before and now I'm doing some work where it is relevant and easy to demonstrate so I figured I would post my changes.

Basically, the NPC signal check is called every AI_Process(), which is 150ms from what I'm seeing in the latest source. There is a slim chance that two signals can be sent to the same NPC in that time window and those signals will overwrite one another since NPC only has storage for one signal at a time.
This just trades signal_id for a deque of signal_id's and changes CheckSignal and SignalNPC accordingly.


Index: npc.h
================================================== =================
--- npc.h (revision 2008)
+++ npc.h (working copy)
@@ -24,6 +24,7 @@
//#include "spawn.h"



#include <list>

+#include <deque>

using namespace std;



#include "spawn2.h"

@@ -193,9 +194,9 @@
int32 GetSwarmOwner();

int32 GetSwarmTarget();

void SetSwarmTarget(int target_id = 0);

-

- inline void SignalNPC(int _signal_id) { signaled = true; signal_id = _signal_id; }

-

+

+ void SignalNPC(int _signal_id);

+

inline sint32 GetNPCFactionID() const { return npc_faction_id; }

inline sint32 GetPrimaryFaction() const { return primary_faction; }

sint32 GetNPCHate(Mob* in_ent) {return hate_list.GetEntHate(in_ent);}

@@ -355,10 +356,11 @@
Timer taunt_timer; //for pet taunting



bool npc_aggro;

-

- int signal_id;

- bool signaled; // used by quest signal() command

-

+

+ //int signal_id;

+ deque<int> signal_q; //Storage so signals within our update window don't get trampled

+ bool signaled; // used by quest signal() command

+

//waypoint crap:

vector<wplist> Waypoints;

void _ClearWaypints();

Index: npc.cpp
================================================== =================
--- npc.cpp (revision 2008)
+++ npc.cpp (working copy)
@@ -2091,3 +2091,9 @@
return max_mana;

}

}

+

+void NPC::SignalNPC(int _signal_id)

+{

+ signaled = true;

+ signal_q.push_back(_signal_id);

+}

Index: MobAI.cpp
================================================== =================
--- MobAI.cpp (revision 2008)
+++ MobAI.cpp (working copy)
@@ -2178,17 +2178,24 @@
}



void NPC::CheckSignal() {

- if (signaled) {

- char buf[32];

- snprintf(buf, 31, "%d", signal_id);

- buf[31] = '\0';

+ if (signaled) {

+ char buf[32];

+

+ //Sanity check

+ if(signal_q.size() < 1) { signaled = false; return; }

+

+ int signal_id = signal_q.front();

+ signal_q.pop_front();

+

+ snprintf(buf, 31, "%d", signal_id);

+ buf[31] = '\0';

parse->EventNPC(EVENT_SIGNAL, this, NULL, buf, 0);

- signaled=false;

- }

+

+ if(signal_q.size() < 1) { signaled = false; }

+ }

}





-

/*

alter table npc_types drop column usedspells;

alter table npc_types add column npc_spells_id int(11) unsigned not null default 0 after merchant_id;

[trevius]

Very cool, Tabasco :)

I have always just set a 1 second timer between multiple signals, but this solution is much better.

[sorvani]

has anyone tested this out? I would love to get this committed to the SVN for PEQ to start using it.

[trevius]

I haven't had time to test it yet, but I think you should be OK to just commit it. Looks like a pretty straight-forward change. If it causes any issues, it can always be reverted easy enough.

Probably don't need to add this line though:

+ //int signal_id;

[lerxst2112]

I'd use if(signal_q.empty()) instead of if(signal_q.size() < 1), but it looks good to me.

In case anyone cares about the longer explanation, the empty() function is guaranteed to be constant time where the size() function is not. In practice, for most containers including deque they are both constant time and empty() just returns size() == 0, but there are some implementations where that isn't true. It's talked about in Effective STL #4.

[Leere]

Committed in rev2010 with some minor changes.

Tested this by creating 5 npcs that send a signal on their death to a 6th, and then AEd the 5 npcs to death. The controller receiving the signals happily counted all of their deaths.

[Tabasco]

In case anyone cares about the longer explanation, the empty() function is guaranteed to be constant time where the size() function is not. In practice, for most containers including deque they are both constant time and empty() just returns size() == 0, but there are some implementations where that isn't true. It's talked about in Effective STL #4.
That's awesome actually, thanks.

http://www.uml.org.cn/c++/pdf/EffectiveSTL.pdf