BETA: Reworking effect's success calculation:

	* Avoiding creation of many Env objects inside for statements.
	* Avoiding some toArray operations.
	* Using static empty array as parameter into some toArray operations since ArrayList only use parameter to get type, there is no need to instantiate a new array of proper size.
	* Passive skills add effects to player in method getPassiveEffects(..) as active skills do in getEffects(..).
	* If calcSuccess() is not overridden in datapack effect implementation, effect success will depend on Formulas.calcEffectSuccess(env) formula.
	* Fixed bug where we calculated success with Formulas.calcEffectSuccess(..), then inside effect implementation as well.
	* To make an effect land always, override calcSuccess() in effect implementation and return true.
	* To use a specific success formula, override calcSuccess() in effect implementation and return the result of the formula.

L2J_Server_BETA/java/com/l2jserver/gameserver/model/CharEffectList.java

@@ -18,6 +18,7 @@
  */
 package com.l2jserver.gameserver.model;
 
+import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
 import java.util.Map;
@@ -54,6 +55,8 @@ import com.l2jserver.gameserver.network.serverpackets.SystemMessage;
 public final class CharEffectList
 {
 	private static final Logger _log = Logger.getLogger(CharEffectList.class.getName());
+	/** Static empty array of effects, used for toArray operations. */
+	private static final L2Effect[] EMPTY_EFFECT_ARRAY = new L2Effect[0];
 	/** List containing all effect buffs for this effect list. */
 	private List<L2Effect> _buffs;
 	/** List containing all effect debuffs for this effect list. */
@@ -685,7 +688,16 @@ public final class CharEffectList
 	
 	/**
 	 * Add a set of effects to this effect list.
-	 * @param effects the effect list to add
+	 * @param effects the effect collection to add
+	 */
+	public void add(Collection<L2Effect> effects)
+	{
+		add(effects.toArray(EMPTY_EFFECT_ARRAY));
+	}
+	
+	/**
+	 * Add a set of effects to this effect list.
+	 * @param effects the effect array to add
 	 */
 	public void add(L2Effect... effects)
 	{

L2J_Server_BETA/java/com/l2jserver/gameserver/model/actor/L2Character.java

@@ -5789,9 +5789,7 @@ public abstract class L2Character extends L2Object implements ISkillsHolder
 			{
 				addChanceTrigger(newSkill);
 			}
-			
-			// Add passive effects if there are any.
-			_effectList.add(newSkill.getPassiveEffects(this));
+			newSkill.getPassiveEffects(this);
 		}
 		return oldSkill;
 	}

L2J_Server_BETA/java/com/l2jserver/gameserver/model/actor/instance/L2CubicInstance.java

@@ -651,6 +651,11 @@ public final class L2CubicInstance
 		}
 	}
 	
+	/**
+	 * @param activeCubic
+	 * @param skill
+	 * @param targets
+	 */
 	public void useCubicMdam(L2CubicInstance activeCubic, L2Skill skill, L2Object[] targets)
 	{
 		for (L2Character target : (L2Character[]) targets)

L2J_Server_BETA/java/com/l2jserver/gameserver/model/effects/L2Effect.java

@@ -261,6 +261,23 @@ public abstract class L2Effect implements IChanceSkillTrigger
 		return _lambda.calc(env);
 	}
 	
+	/**
+	 * Calculates whether this effects land or not.<br>
+	 * If it lands will be scheduled and added to the character effect list.<br>
+	 * Override in effect implementation to change behavior.
+	 * @return {@code true} if this effect land, {@code false} otherwise
+	 */
+	public boolean calcSuccess()
+	{
+		final Env env = new Env();
+		env.setSkillMastery(Formulas.calcSkillMastery(getEffector(), getSkill()));
+		env.setCharacter(getEffector());
+		env.setTarget(getEffected());
+		env.setSkill(getSkill());
+		env.setEffect(this);
+		return Formulas.calcEffectSuccess(env);
+	}
+	
 	private final void startEffectTask()
 	{
 		if (_abnormalTime > 0)

L2J_Server_BETA/java/com/l2jserver/gameserver/model/skills/L2Skill.java

@@ -46,7 +46,6 @@ import com.l2jserver.gameserver.model.actor.L2Character;
 import com.l2jserver.gameserver.model.actor.L2Playable;
 import com.l2jserver.gameserver.model.actor.L2Summon;
 import com.l2jserver.gameserver.model.actor.instance.L2CubicInstance;
-import com.l2jserver.gameserver.model.actor.instance.L2DoorInstance;
 import com.l2jserver.gameserver.model.actor.instance.L2PcInstance;
 import com.l2jserver.gameserver.model.actor.instance.L2SiegeFlagInstance;
 import com.l2jserver.gameserver.model.conditions.Condition;
@@ -63,7 +62,6 @@ import com.l2jserver.gameserver.model.stats.BaseStats;
 import com.l2jserver.gameserver.model.stats.Env;
 import com.l2jserver.gameserver.model.stats.Formulas;
 import com.l2jserver.gameserver.model.zone.ZoneId;
-import com.l2jserver.gameserver.network.SystemMessageId;
 import com.l2jserver.gameserver.network.serverpackets.SystemMessage;
 import com.l2jserver.gameserver.util.Util;
 import com.l2jserver.util.Rnd;
@@ -73,7 +71,7 @@ public abstract class L2Skill implements IChanceSkillTrigger
 	protected static final Logger _log = Logger.getLogger(L2Skill.class.getName());
 	
 	private static final L2Object[] EMPTY_TARGET_LIST = new L2Object[0];
-	private static final L2Effect[] EMPTY_EFFECT_SET = new L2Effect[0];
+	private static final List<L2Effect> EMPTY_EFFECT_LIST = Collections.<L2Effect> emptyList();
 	
 	public static final int SKILL_CREATE_DWARVEN = 172;
 	public static final int SKILL_EXPERTISE = 239;
@@ -98,7 +96,6 @@ public abstract class L2Skill implements IChanceSkillTrigger
 	public static final int COND_CHARGES = 0x0080;
 	public static final int COND_SHIELD = 0x0100;
 	
-	private final boolean _abnormalInstant;
 	/** Skill Id. */
 	private final int _id;
 	/** Skill level. */
@@ -132,6 +129,8 @@ public abstract class L2Skill implements IChanceSkillTrigger
 	private final int _castRange;
 	/** Effect range: how far the skill affect the target. */
 	private final int _effectRange;
+	/** Abnormal instant, used for herbs mostly. */
+	private final boolean _isAbnormalInstant;
 	/** Abnormal level, global effect level. */
 	private final int _abnormalLvl;
 	/** Abnormal type: global effect "group". */
@@ -260,7 +259,7 @@ public abstract class L2Skill implements IChanceSkillTrigger
 	
 	protected L2Skill(StatsSet set)
 	{
-		_abnormalInstant = set.getBool("abnormalInstant", false);
+		_isAbnormalInstant = set.getBool("abnormalInstant", false);
 		_id = set.getInteger("skill_id");
 		_level = set.getInteger("level");
 		_refId = set.getInteger("referenceId", 0);
@@ -497,11 +496,6 @@ public abstract class L2Skill implements IChanceSkillTrigger
 	
 	public abstract void useSkill(L2Character caster, L2Object[] targets);
 	
-	public final boolean abnormalInstant()
-	{
-		return _abnormalInstant;
-	}
-	
 	public final int getConditionValue()
 	{
 		return _conditionValue;
@@ -608,6 +602,14 @@ public abstract class L2Skill implements IChanceSkillTrigger
 		return _blockBuffSlots;
 	}
 	
+	/**
+	 * @return {@code true} if the skill is abnormal instant, {@code false} otherwise
+	 */
+	public final boolean isAbnormalInstant()
+	{
+		return _isAbnormalInstant;
+	}
+	
 	public final int getAbnormalLvl()
 	{
 		return _abnormalLvl;
@@ -1391,17 +1393,17 @@ public abstract class L2Skill implements IChanceSkillTrigger
 	 * @param env
 	 * @return an array with the effects that have been added to effector
 	 */
-	public final L2Effect[] getEffects(L2Character effector, L2Character effected, Env env)
+	public final List<L2Effect> getEffects(L2Character effector, L2Character effected, Env env)
 	{
-		if (!hasEffects() || isPassive())
+		if ((effected == null) || !hasEffects() || isPassive())
 		{
-			return EMPTY_EFFECT_SET;
+			return EMPTY_EFFECT_LIST;
 		}
 		
 		// doors and siege flags cannot receive any effects
-		if ((effected instanceof L2DoorInstance) || (effected instanceof L2SiegeFlagInstance))
+		if (effected.isDoor() || (effected instanceof L2SiegeFlagInstance))
 		{
-			return EMPTY_EFFECT_SET;
+			return EMPTY_EFFECT_LIST;
 		}
 		
 		if (effector != effected)
@@ -1410,53 +1412,43 @@ public abstract class L2Skill implements IChanceSkillTrigger
 			{
 				if (effected.isInvul())
 				{
-					return EMPTY_EFFECT_SET;
+					return EMPTY_EFFECT_LIST;
 				}
 				
-				if ((effector instanceof L2PcInstance) && ((L2PcInstance) effector).isGM())
+				if (effector.isPlayer() && effector.isGM())
 				{
-					if (!((L2PcInstance) effector).getAccessLevel().canGiveDamage())
+					if (!effector.getAccessLevel().canGiveDamage())
 					{
-						return EMPTY_EFFECT_SET;
+						return EMPTY_EFFECT_LIST;
 					}
 				}
 			}
 		}
 		
-		final List<L2Effect> effects = new ArrayList<>(_effectTemplates.size());
 		if (env == null)
 		{
 			env = new Env();
 		}
-		
 		env.setSkillMastery(Formulas.calcSkillMastery(effector, this));
 		env.setCharacter(effector);
 		env.setTarget(effected);
 		env.setSkill(this);
 		
+		final List<L2Effect> effects = new ArrayList<>(_effectTemplates.size());
 		for (EffectTemplate et : _effectTemplates)
 		{
-			if (Formulas.calcEffectSuccess(effector, effected, et, this, env.getShield(), env.isSoulShot(), env.isSpiritShot(), env.isBlessedSpiritShot()))
+			final L2Effect e = et.getEffect(env);
+			if (e != null)
 			{
-				L2Effect e = et.getEffect(env);
-				if (e != null)
+				if (e.calcSuccess())
 				{
 					e.scheduleEffect();
 					effects.add(e);
 				}
 			}
-			// display fail message only for effects with icons
-			else if (et.isIconDisplay() && effector.isPlayer())
-			{
-				SystemMessage sm = SystemMessage.getSystemMessage(SystemMessageId.C1_RESISTED_YOUR_S2);
-				sm.addCharName(effected);
-				sm.addSkillName(this);
-				effector.sendPacket(sm);
-			}
 		}
-		final L2Effect[] list = effects.toArray(new L2Effect[effects.size()]);
-		effected.getEffectList().add(list);
-		return effects.isEmpty() ? EMPTY_EFFECT_SET : list;
+		effected.getEffectList().add(effects);
+		return effects;
 	}
 	
 	/**
@@ -1465,31 +1457,16 @@ public abstract class L2Skill implements IChanceSkillTrigger
 	 * @param effected
 	 * @return
 	 */
-	public final L2Effect[] getEffects(L2Character effector, L2Character effected)
+	public final List<L2Effect> getEffects(L2Character effector, L2Character effected)
 	{
 		return getEffects(effector, effected, null);
 	}
 	
-	/**
-	 * This method has suffered some changes in CT2.2 ->CT2.3<br>
-	 * Effect engine is now supporting secondary effects with independent success/fail calculus from effect skill.<br>
-	 * Env parameter has been added to pass parameters like soulshot, spiritshots, blessed spiritshots or shield defense.<br>
-	 * Some other optimizations have been done<br>
-	 * This new feature works following next rules:
-	 * <ul>
-	 * <li>To enable feature, effectPower must be over -1 (check DocumentSkill#attachEffect for further information)</li>
-	 * <li>If main skill fails, secondary effect always fail</li>
-	 * </ul>
-	 * @param effector
-	 * @param effected
-	 * @param env
-	 * @return
-	 */
-	public final L2Effect[] getEffects(L2CubicInstance effector, L2Character effected, Env env)
+	public final List<L2Effect> getEffects(L2CubicInstance effector, L2Character effected, Env env)
 	{
-		if (!hasEffects() || isPassive())
+		if ((effector == null) || (effected == null) || !hasEffects() || isPassive())
 		{
-			return EMPTY_EFFECT_SET;
+			return EMPTY_EFFECT_LIST;
 		}
 		
 		if (effector.getOwner() != effected)
@@ -1498,94 +1475,97 @@ public abstract class L2Skill implements IChanceSkillTrigger
 			{
 				if (effected.isInvul())
 				{
-					return EMPTY_EFFECT_SET;
+					return EMPTY_EFFECT_LIST;
 				}
 				
 				if (effector.getOwner().isGM() && !effector.getOwner().getAccessLevel().canGiveDamage())
 				{
-					return EMPTY_EFFECT_SET;
+					return EMPTY_EFFECT_LIST;
 				}
 			}
 		}
 		
-		List<L2Effect> effects = new ArrayList<>(_effectTemplates.size());
 		if (env == null)
 		{
 			env = new Env();
 		}
-		
 		env.setCharacter(effector.getOwner());
 		env.setCubic(effector);
 		env.setTarget(effected);
 		env.setSkill(this);
 		
+		final List<L2Effect> effects = new ArrayList<>(_effectTemplates.size());
 		for (EffectTemplate et : _effectTemplates)
 		{
-			if (Formulas.calcEffectSuccess(effector.getOwner(), effected, et, this, env.getShield(), env.isSoulShot(), env.isSpiritShot(), env.isBlessedSpiritShot()))
+			final L2Effect e = et.getEffect(env);
+			if (e != null)
 			{
-				L2Effect e = et.getEffect(env);
-				if (e != null)
+				if (e.calcSuccess())
 				{
 					e.scheduleEffect();
 					effects.add(e);
 				}
 			}
 		}
-		
-		final L2Effect[] list = effects.toArray(new L2Effect[effects.size()]);
-		effected.getEffectList().add(list);
-		return effects.isEmpty() ? EMPTY_EFFECT_SET : list;
+		return effects;
 	}
 	
-	public final L2Effect[] getEffectsSelf(L2Character effector)
+	public final List<L2Effect> getEffectsSelf(L2Character effector)
 	{
-		if (!hasSelfEffects() || isPassive())
+		if ((effector == null) || !hasSelfEffects() || isPassive())
 		{
-			return EMPTY_EFFECT_SET;
+			return EMPTY_EFFECT_LIST;
 		}
 		
+		final Env env = new Env();
+		env.setCharacter(effector);
+		env.setTarget(effector);
+		env.setSkill(this);
+		
 		final List<L2Effect> effects = new ArrayList<>(_effectTemplatesSelf.size());
 		for (EffectTemplate et : _effectTemplatesSelf)
 		{
-			Env env = new Env();
-			env.setCharacter(effector);
-			env.setTarget(effector);
-			env.setSkill(this);
-			L2Effect e = et.getEffect(env);
+			final L2Effect e = et.getEffect(env);
 			if (e != null)
 			{
-				e.setSelfEffect();
-				e.scheduleEffect();
-				effects.add(e);
+				if (e.calcSuccess())
+				{
+					e.setSelfEffect();
+					e.scheduleEffect();
+					effects.add(e);
+				}
 			}
 		}
-		final L2Effect[] list = effects.toArray(new L2Effect[effects.size()]);
-		effector.getEffectList().add(list);
-		return effects.isEmpty() ? EMPTY_EFFECT_SET : list;
+		effector.getEffectList().add(effects);
+		return effects;
 	}
 	
-	public final L2Effect[] getPassiveEffects(L2Character effector)
+	public final List<L2Effect> getPassiveEffects(L2Character effector)
 	{
-		if (!hasPassiveEffects())
+		if ((effector == null) || !hasPassiveEffects())
 		{
-			return EMPTY_EFFECT_SET;
+			return EMPTY_EFFECT_LIST;
 		}
 		
+		final Env env = new Env();
+		env.setCharacter(effector);
+		env.setTarget(effector);
+		env.setSkill(this);
 		final List<L2Effect> effects = new ArrayList<>(_effectTemplatesPassive.size());
 		for (EffectTemplate et : _effectTemplatesPassive)
 		{
-			Env env = new Env();
-			env.setCharacter(effector);
-			env.setTarget(effector);
-			env.setSkill(this);
-			L2Effect e = et.getEffect(env);
+			final L2Effect e = et.getEffect(env);
 			if (e != null)
 			{
-				e.scheduleEffect();
-				effects.add(e);
+				if (e.calcSuccess())
+				{
+					e.scheduleEffect();
+					effects.add(e);
+				}
 			}
 		}
-		return effects.isEmpty() ? EMPTY_EFFECT_SET : effects.toArray(new L2Effect[effects.size()]);
+		effector.getEffectList().add(effects);
+		return effects;
 	}
 	
 	public final void attach(FuncTemplate f)

L2J_Server_BETA/java/com/l2jserver/gameserver/model/stats/Formulas.java

@@ -1557,71 +1557,70 @@ public final class Formulas
 		return elementMod;
 	}
 	
-	public static boolean calcEffectSuccess(L2Character attacker, L2Character target, EffectTemplate effect, L2Skill skill, byte shld, boolean ss, boolean sps, boolean bss)
+	public static boolean calcEffectSuccess(Env env)
 	{
 		// Effect base rate, if it's -1 (or less) always land.
-		final double baseRate = effect.getEffectPower();
-		if ((baseRate < 0) || skill.hasEffectType(L2EffectType.CANCEL_DEBUFF, L2EffectType.CANCEL))
+		final double baseRate = env.getEffect().getEffectPower();
+		if (baseRate < 0)
 		{
 			return true;
 		}
 		
-		if (skill.isDebuff())
+		if (env.getSkill().isDebuff())
 		{
-			if (skill.getPower() == -1)
+			if (env.getSkill().getPower() == -1)
 			{
-				if (attacker.isDebug())
+				if (env.getCharacter().isDebug())
 				{
-					attacker.sendDebugMessage(skill.getName() + " effect ignoring resists");
+					env.getCharacter().sendDebugMessage(env.getSkill().getName() + " effect ignoring resists");
 				}
 				return true;
 			}
-			else if (target.calcStat(Stats.DEBUFF_IMMUNITY, 0, null, skill) > 0)
+			else if (env.getTarget().calcStat(Stats.DEBUFF_IMMUNITY, 0, null, env.getSkill()) > 0)
 			{
 				return false;
 			}
 		}
 		
 		// Perfect Shield Block.
-		if (shld == SHIELD_DEFENSE_PERFECT_BLOCK)
+		if (env.getShield() == SHIELD_DEFENSE_PERFECT_BLOCK)
 		{
-			if (attacker.isDebug())
+			if (env.getCharacter().isDebug())
 			{
-				attacker.sendDebugMessage(skill.getName() + " effect blocked by shield");
+				env.getCharacter().sendDebugMessage(env.getSkill().getName() + " effect blocked by shield");
 			}
-			
 			return false;
 		}
 		
 		// Calculate BaseRate.
-		double statMod = calcSkillStatMod(skill, target);
+		double statMod = calcSkillStatMod(env.getSkill(), env.getTarget());
 		double rate = (baseRate / statMod);
 		
 		// Resist Modifier.
-		double resMod = calcResMod(attacker, target, skill);
+		double resMod = calcResMod(env.getCharacter(), env.getTarget(), env.getSkill());
 		rate *= resMod;
 		
 		// Lvl Bonus Modifier.
-		double lvlBonusMod = calcLvlBonusMod(attacker, target, skill);
+		double lvlBonusMod = calcLvlBonusMod(env.getCharacter(), env.getTarget(), env.getSkill());
 		rate *= lvlBonusMod;
 		
 		// Element Modifier.
-		double elementMod = calcElementMod(attacker, target, skill);
+		double elementMod = calcElementMod(env.getCharacter(), env.getTarget(), env.getSkill());
 		rate *= elementMod;
 		
 		// Add Matk/Mdef Bonus (TODO: Pending)
 		
 		// Check the Rate Limits.
-		rate = Math.min(Math.max(rate, skill.getMinChance()), skill.getMaxChance());
+		rate = Math.min(Math.max(rate, env.getSkill().getMinChance()), env.getSkill().getMaxChance());
 		
-		if (attacker.isDebug() || Config.DEVELOPER)
+		if (env.getCharacter().isDebug() || Config.DEVELOPER)
 		{
 			final StringBuilder stat = new StringBuilder(100);
-			StringUtil.append(stat, skill.getName(), " power:", String.valueOf(baseRate), " stat:", String.format("%1.2f", statMod), " res:", String.format("%1.2f", resMod), " elem:", String.format("%1.2f", elementMod), " lvl:", String.format("%1.2f", lvlBonusMod), " total:", String.valueOf(rate));
+			StringUtil.append(stat, env.getSkill().getName(), " power:", String.valueOf(baseRate), " stat:", String.format("%1.2f", statMod), " res:", String.format("%1.2f", resMod), " elem:", String.format("%1.2f", elementMod), " lvl:", String.format("%1.2f", lvlBonusMod), " total:", String.valueOf(rate));
 			final String result = stat.toString();
-			if (attacker.isDebug())
+			if (env.getCharacter().isDebug())
 			{
-				attacker.sendDebugMessage(result);
+				env.getCharacter().sendDebugMessage(result);
 			}
 			if (Config.DEVELOPER)
 			{
@@ -2303,7 +2302,7 @@ public final class Formulas
 		
 		// An herb buff will affect both master and servitor, but the buff duration will be half of the normal duration.
 		// If a servitor is not summoned, the master will receive the full buff duration.
-		if ((effected != null) && effected.isServitor() && (skill != null) && skill.abnormalInstant())
+		if ((effected != null) && effected.isServitor() && (skill != null) && skill.isAbnormalInstant())
 		{
 			time /= 2;
 		}