test script zone attack

[AlexeWarr]

Hello, made this script so when ever a player dies it will send a world message <[pvp] "map name" is in fighting!>

But something aint working right, if I kill my self in eastern kingdoms will show that message
but if I kill my self on another map It will show the map name from another index in the table.. idk how tables are working really, im still noobish with them.. if anyone can help me with this..

local delay_message = 60 -- in seconds


local ZA_TM = -- Zone attack triggered maps
{
	{0, 	"Eastern Kingdoms"}, 	-- map: id, name
	{1, 	"Kalimdor"}, 			-- map: id, name
	{560, 	"Durnholde"}, 			-- map: id, name
}

local ZAD_Prefix, ZAD_Subfix = "|cFFffcc00[PVP] ", " is in fighting!|r"
local ZAD_M = {} -- Zone attack delay maps

local function OnTriggerZoneAttack(killer, killed)
	-- if suicide then stop doing anything
	if (not killer:GetGUIDLow() == killed:GetGUIDLow()) then return end
	
	-- if map is not registered in a table, we create one
	if (not ZAD_M[killer:GetMapId()]) then
		ZAD_M[killer:GetMapId()] = {}
	
	-- if map is registered already, skip everything
	elseif ( ZAD_M[killer:GetMapId()] and (os.time()-ZAD_M[killer:GetMapId()] < delay_message) ) then return end
	-- seconds remain untill next message is available = os.time()-ZAD_M[killer:GetMapId()]
	
	-- if just registered now, add delay
	ZAD_M[killer:GetMapId()] = nil -- clean before adding current game time
	ZAD_M[killer:GetMapId()] = os.time()
	
	-- if delay given after map just registered send zone attack message
	for _,v in pairs(ZA_TM) do
	if ( string.find(killer:GetMapId(), v[1]) ) then
		SendWorldMessage(ZAD_Prefix..v[2]..ZAD_Subfix)
return	end
	end
end

function OnPVPTrigger(event, killer, killed)
OnTriggerZoneAttack(killer, killed)
end

function OnPVETrigger(event, killer, killed)
OnTriggerZoneAttack(killer, killed)
end

RegisterServerHook(6, OnPVPTrigger)
RegisterServerHook(7, OnPVETrigger)

 

[Laurea]

You're using string.find. It'll return true if it finds the map id anywhere in the player's map id, so if you're on map 10, it will return true for maps 0, 1 and 10. Here's how I would have done it, I even commented the script for you. (Comments... *shudder*)

--CONFIG START
local Prefix = "|cFFffcc00[PVP] ";
local Affix = " is in fighting!|r";
local Delay = 60;
--CONFIG END

local delay_list = {};
local list = {}; --We're filling this up at the bottom, but need the declaration here to access in OnTriggerZoneAttack

local function OnTriggerZoneAttack(killer, killed)
	--Check if player somehow managed to kill themselves
	if (killer:GetGUIDLow() == killed:GetGUIDLow()) then
		return;
	end
	--Two function calls takes longer than one function call and accessing a variable twice
	local time = os.time();
	local map = killer:GetMapId();
	--Check if map is in list and delay has expired
	if (list[map] == nil) then
		return;
	elseif (delay_list[map] and time < delay_list[map]) then
		return;
	end
	--Set new delay. Also no need to "clean up" the variable, we're still using the old memory address
	delay_list[map] = time + Delay;
	--Send message
	SendWorldMessage(list[map]);
end

function OnPVPTrigger(event, killer, killed)
	OnTriggerZoneAttack(killer, killed);
end

function OnPVETrigger(event, killer, killed)
	OnTriggerZoneAttack(killer, killed);
end

RegisterServerHook(6, OnPVPTrigger);
RegisterServerHook(7, OnPVETrigger);

--Just in case someone derped up the config. These are inline if's, by the way
Prefix = type(Prefix) == "string" and Prefix or "|cFFffcc00[PVP] ";
Affix = type(Affix) == "string" and Affix or " is in fighting!|r";
Delay = tonumber(Delay); Delay = Delay or 60;

list = {
	--Use map id as index, it's faster than iterating over possibly the whole list.
	--Also, these lists can get quite long, and it's rather annoying to scroll down two hundred
	--lines just to get to the code
	[0]		= "Eastern Kingdoms",
	[1]		= "Kalimdor",
	[560]	= "Durnholde",
};

--Why concatinate this in the function all the time when we can just do it once?
for k, v in pairs (list) do
	list[k] = Prefix..v..Affix;
end

 

[Rochet2]

I find it funny that you dont know tables since I didnt know any lua when you already scripted : /
Note!! if (not killer:GetGUIDLow() == killed:GetGUIDLow()) then return end
Using not like this means this:
if((not a) == b) then
Since nil and false are false in lua (0 is not false) and not changes it the other way around, not a is always false!
This means that you are doing: if(false == killed:GetGUIDLow()) then
a number is never false. so this will never work.
Use not(a == b) or then you could use (a ~= b), it means the same as != in C++
also, you shouldnt be using not at all in that place, since it is suicide when a == b (guids are the same)
Also note that since you have hooked creature AND player hooks together and player and creature lowguids CAN be the same, you should add a check that it is a player vs player hook when checking the guids.

string.find(killer:GetMapId(), v[1])
Note that find will search the whole thing, so if player is on map 500 and the map in the table is 5, it would return true.
I suggest you either use == instead of the whole find thing, or you use the mapIDs as keys in the table. (see example script)
This way you dont need to loop through the whole thing and can access the element directly. You could also add a return right to the start if the map is wrong avoiding to save unnecessary data.

This possibly works:

local delay_message = 60 -- in seconds


local ZA_TM = -- Zone attack triggered maps
{
--  [mapID] =   "Name",
	[0]     =   "Eastern Kingdoms",
	[1]     =   "Kalimdor",
	[560]   =   "Durnholde",
}

local ZAD_Prefix, ZAD_Suffix = "|cFFffcc00[PVP] ", " is in fighting!|r"
local ZAD_M = {} -- Zone attack delay maps

local function OnTriggerZoneAttack(event killer, killed)
	-- if suicide then stop doing anything (on PVP event)
	if (event == 6 and killer:GetGUIDLow() == killed:GetGUIDLow()) then return end -- note, using (not a == b) doesnt work like you would expect.
    local name = ZA_TM[killer:GetMapId()]
    if(not name) then return end -- not one of the maps
	
	if (not ZAD_M[killer:GetMapId()]) then -- if map is not registered in a table, we create one
		ZAD_M[killer:GetMapId()] = {}
	elseif (os.time()-ZAD_M[killer:GetMapId()] < delay_message) then return end -- if map is registered already, skip everything
	-- seconds remain untill next message is available = os.time()-ZAD_M[killer:GetMapId()]
	
	-- if just registered now, add delay
	-- This is not needed. ZAD_M[killer:GetMapId()] = nil -- clean before adding current game time
	ZAD_M[killer:GetMapId()] = os.time()
	
	-- if delay given after map just registered send zone attack message
    SendWorldMessage(ZAD_Prefix..name..ZAD_Suffix)
end

RegisterServerHook(6, OnTriggerZoneAttack) -- PVP
RegisterServerHook(7, OnTriggerZoneAttack) -- PVE

Read the comments and the explanation!


I see laurea beat me to it, but read my message still, laurea has a bug :3

 

[AlexeWarr]

I find it funny that you dont know tables since I didnt know any lua when you already scripted : /
Note!! if (not killer:GetGUIDLow() == killed:GetGUIDLow()) then return end
Using not like this means this:
if((not a) == b) then
Since nil and false are false in lua (0 is not false) and not changes it the other way around, not a is always false!
This means that you are doing: if(false == killed:GetGUIDLow()) then
a number is never false. so this will never work.
Use not(a == b) or then you could use (a ~= b), it means the same as != in C++
also, you shouldnt be using not at all in that place, since it is suicide when a == b (guids are the same)
Also note that since you have hooked creature AND player hooks together and player and creature lowguids CAN be the same, you should add a check that it is a player vs player hook when checking the guids.

string.find(killer:GetMapId(), v[1])
Note that find will search the whole thing, so if player is on map 500 and the map in the table is 5, it would return true.
I suggest you either use == instead of the whole find thing, or you use the mapIDs as keys in the table. (see example script)
This way you dont need to loop through the whole thing and can access the element directly. You could also add a return right to the start if the map is wrong avoiding to save unnecessary data.

This possibly works:

local delay_message = 60 -- in seconds


local ZA_TM = -- Zone attack triggered maps
{
--  [mapID] =   "Name",
	[0]     =   "Eastern Kingdoms",
	[1]     =   "Kalimdor",
	[560]   =   "Durnholde",
}

local ZAD_Prefix, ZAD_Suffix = "|cFFffcc00[PVP] ", " is in fighting!|r"
local ZAD_M = {} -- Zone attack delay maps

local function OnTriggerZoneAttack(event killer, killed)
	-- if suicide then stop doing anything (on PVP event)
	if (event == 6 and killer:GetGUIDLow() == killed:GetGUIDLow()) then return end -- note, using (not a == b) doesnt work like you would expect.
    local name = ZA_TM[killer:GetMapId()]
    if(not name) then return end -- not one of the maps
	
	if (not ZAD_M[killer:GetMapId()]) then -- if map is not registered in a table, we create one
		ZAD_M[killer:GetMapId()] = {}
	elseif (os.time()-ZAD_M[killer:GetMapId()] < delay_message) then return end -- if map is registered already, skip everything
	-- seconds remain untill next message is available = os.time()-ZAD_M[killer:GetMapId()]
	
	-- if just registered now, add delay
	-- This is not needed. ZAD_M[killer:GetMapId()] = nil -- clean before adding current game time
	ZAD_M[killer:GetMapId()] = os.time()
	
	-- if delay given after map just registered send zone attack message
    SendWorldMessage(ZAD_Prefix..name..ZAD_Suffix)
end

RegisterServerHook(6, OnTriggerZoneAttack) -- PVP
RegisterServerHook(7, OnTriggerZoneAttack) -- PVE

Read the comments and the explanation!


I see laurea beat me to it, but read my message still, laurea has a bug :3

Thanks Rochet and Laurea, at this "Note!! if (not killer:GetGUIDLow() == killed:GetGUIDLow()) then return end" I used that because I wanted to test while killing myself instead of dual boxing
Tested and atm appears to be fine, thank you so much :3

 

[Hyperion]

Appears to be solved

Marking.