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