When to add default parameters to temporary effects?

[yufra]

I ran into a strange bug when trying to merge effects using the default parameters (currently an empty table, but below I propose this to be a default value if p is nil). The default parameters were currently loaded in AFTER the merge attempt, and therefore the new_eff would not have a power/etc. This patch loads the before the merge attempt. Is there any specific reason for the current method, or can this be integrated without any concern?

Code: Select all

Index: ActorTemporaryEffects.lua
===================================================================
--- ActorTemporaryEffects.lua	(revision 1126)
+++ ActorTemporaryEffects.lua	(working copy)
@@ -91,10 +91,16 @@
 	-- Beware, setting to 0 means removing
 	if dur <= 0 then return self:removeEffect(eff_id) end
 
+	p = p or {}
+	-- Load the default parameters
+	for k, e in pairs(_M.tempeffect_def[eff_id].parameters) do
+		if not p[k] then p[k] = e end
+	end
+	p.dur = dur
+	
 	-- If we already have it, we check if it knows how to "merge", or else we remove it and re-add it
 	if self:hasEffect(eff_id) then
 		if _M.tempeffect_def[eff_id].on_merge then
-			p.dur = dur
 			self.tmp[eff_id] = _M.tempeffect_def[eff_id].on_merge(self, self.tmp[eff_id], p)
 			self.changed = true
 			return
@@ -103,10 +109,7 @@
 		end
 	end
 
-	for k, e in pairs(_M.tempeffect_def[eff_id].parameters) do
-		if not p[k] then p[k] = e end
-	end
-	p.dur = dur
+
 	self.tmp[eff_id] = p
 	if _M.tempeffect_def[eff_id].on_gain then
 		local ret, fly = _M.tempeffect_def[eff_id].on_gain(self, p)

[darkgod]

I dont think so , I'l ladd it