Latest Patch - SK lifetap buff spells not working

[Irreverent]

LV 67 Shroud of Discord
LV 63 Shroud of Chaos
LV 55 Shroud of Death
LV 37 Scream of Death
LV 22 Vampiric Embrace

They are castable, show up in buff bar, but do not proc at all. Anyone else notice this?

[So_1337]

There has been a bit of discussion about this over at PEQ's forums in this thread if you're interested in following it. In the meantime, Derision added a bit of a fix to the problem in revision 519, but it still needs more work.

[ChaosSlayerZ]

well your guys broke soemthing =)
cuase i played my Sk back in January and everything was procing just fine =P

[ChaosSlayerZ]

so any update on fixes for broken procs?

I just took rangers call of the sky and during 15 min it didn't proc even once.
at same time i took a regular weapon with proc- and it proced many many times

running Rev 535 btw

[warhawk]

Can confirm that Call of sky is not working. not one proc in 30 mins. This is on the Grand creation.

[AndMetal]

From what I can gather after looking through the source, we didn't used to take into account the chance the spell had to proc, which I believe is what Wolftousen was trying to correct (and for the most part did). That's what iChance is for in Mob:AddProcToWeapon():

bool Mob::AddProcToWeapon(int16 spell_id, bool bPerma, int16 iChance) {
	if(spell_id == SPELL_UNKNOWN)
		return(false);

	int i;
	if (bPerma) {
 		for (i = 0; i < MAX_PROCS; i++) {
			if (PermaProcs[i].spellID == SPELL_UNKNOWN) {
				PermaProcs[i].spellID = spell_id;
				PermaProcs[i].chance = iChance;
				PermaProcs[i].pTimer = NULL;
				mlog(SPELLS__PROCS, "Added permanent proc spell %d with chance %d to slot %d", spell_id, iChance, i);

				return true;
			}
		}
		mlog(SPELLS__PROCS, "Too many perma procs for %s", GetName());
    } else {
		for (i = 0; i < MAX_PROCS; i++) {
			if (SpellProcs[i].spellID == SPELL_UNKNOWN) {
				SpellProcs[i].spellID = spell_id;
				SpellProcs[i].chance = iChance;
				SpellProcs[i].pTimer = NULL;
				mlog(SPELLS__PROCS, "Added spell-granted proc spell %d with chance %d to slot %d", spell_id, iChance, i);
				return true;
			}
		}
		mlog(SPELLS__PROCS, "Too many procs for %s", GetName());
	}
    return false;
}

However, I don't think there's any need to change the calculation for the chance to proc, just make sure we're taking it into account:
zone/attack.cpp

		if (SpellProcs[i].spellID != SPELL_UNKNOWN) {
			int chance = ProcChance * (SpellProcs[i].chance);
			if(MakeRandomInt(0, 100) < chance) {
				mlog(COMBAT__PROCS, "Spell proc %d procing spell %d (%d percent chance)", i, SpellProcs[i].spellID, chance);
				ExecWeaponProc(SpellProcs[i].spellID, on);
			} else {
				mlog(COMBAT__PROCS, "Spell proc %d failed to proc %d (%d percent chance)", i, SpellProcs[i].spellID, chance);
			}
		}

The biggest problem we're running into now is the default for SpellProcs[i].chance for several spells is 0, so we're multiplying by 0. What we should probably be doing is adding a percentage if it's not zero.

From the PEQ Forums:

This will fix any proc that comes from a buff/spell. CD has the new fix for it that will make these procs account for the PROC_RATE_MOD you find in a lot of their discriptions (this was previously not being taken into account). So for BL pets, they will proc 1.5x as much as a normal character with their stats would with that proc (BL pet buffs have a mod of 150...), spirit of puma willl proc 4x more, etc...

If that's the case, we should be able to change this in zone/attack.cpp:

		int16 spid = SpellProcs[i].spellID;
		if (spid != SPELL_UNKNOWN) {
			int8 spc = SpellProcs[i].chance;
			int chance = ProcChance;
			if (spc > 0) {
				chance = ProcChance * spc / 100;
				mlog(COMBAT__PROCS, "Proc mod on spell proc %d (spell %d): adjusting chance from %d to %d (%d mod)", i, spid, ProcChance, chance, spc);
			}
			if(MakeRandomInt(0, 100) < chance) {
				mlog(COMBAT__PROCS, "Spell proc %d procing spell %d (%d percent chance)", i, spid, chance);
				ExecWeaponProc(spid, on);
			} else {
				mlog(COMBAT__PROCS, "Spell proc %d failed to proc %d (%d percent chance)", i, spid, chance);
			}
		}

That way if it's 0 or negative, we don't mess with it, and if it's 400, we're multiplying by 4.

On a side note, there's a potential issue that could break this when zoning. From zone/client_packet.cpp:

                case SE_WeaponProc:
					{
					AddProcToWeapon(GetProcID(buffs[j1].spellid, x1), false, 100+spells[buffs[j1].spellid].base2[x1]);
					break;
					}

We should just be able to remove the 100+, and it should be good to go:

                case SE_WeaponProc:
					{
					AddProcToWeapon(GetProcID(buffs[j1].spellid, x1), false, spells[buffs[j1].spellid].base2[x1]);
					break;
					}

I don't have a server up right now to compile & test, so if someone wants to give this a shot, feel free.

[KLS]

I actually took a bit to rework procs a few days back but my changelog note wasn't very clear. They *should* work correctly now.

[AndMetal]

I did some checking, and it looks like we may be adding too much to the modifier from the spell:

[COMBAT__PROCS] Xamlagar: Proc chance 0.11 (0.07 from bonuses)
[COMBAT__PROCS] Xamlagar: Spell proc 0 failed to proc 3129 (10.7125 percent base + 100 percent spell mod = 10 percent chance)
[COMBAT__PROCS] Xamlagar: Spell proc 1 procing spell 6908 (10.7125 percent base + 500 percent spell mod = 53 percent chance)
[COMBAT__PROCS] Xamlagar: Proc chance 0.11 (0.07 from bonuses)

The first one is Call of Sky, which has a modifier of 0 (so it should be 100%). The second is Spirit of the Puma, which has a modifier of 400 (so it should be 400%).

I'm just not really sure if it would be better to put the check in Mob:TryWeaponProc() or Mob:AddProcToWeapon() rather than Mob:SpellEffect().

[KLS]

100% chance to proc + 400% bonus chance to proc. Other than the haste changes it should be functionally identical in regards to spells with mods.

[AndMetal]

Originally Posted by KLS

100% chance to proc + 400% bonus chance to proc.

Which equals 500%, or a 5x chance to proc instead of 4x. Are we sure that's how it should be done? If it was, I would think we could just add 100 to each calculation (which it seems like may be done, even with the check for 0 in spell_effects.cpp).

From the PEQ Forums:

So for BL pets, they will proc 1.5x as much as a normal character with their stats would with that proc (BL pet buffs have a mod of 150...), spirit of puma willl proc 4x more, etc

The only other real info I've been able to find is on Allakhazam. It doesn't have any real concrete info:

Link
[QUOTE]With no extensions this shamys proc spell on the pet procs 3 to 4 times and some times as much as 5 times. On my big shamy with max buff duration AA

[KLS]

Here's the formula I got from the theory crafters at eqoutrider.

ppm = 2 * (100 + procratemod + worn combat effects + WA modifier)/100

so assuming you have no aas or combat effects for that 400 mod spell:

2 * (100 + 400) / 100

I mean the way you're talking about there would be situations where x > 0 && x < 100 where a proc bonus would be detrimental.

[AndMetal]

Originally Posted by KLS

I mean the way you're talking about there would be situations where x > 0 && x < 100 where a proc bonus would be detrimental.

I guess that's the idea. Basically, it's a percentage (or rather divided by 100 it's a percentage), with an exception where !(x > 0). That way, you could have a spell with an insanely awesome proc (think Cazic Touch as the effect), but only has a 1% chance of the normal probability to proc (so a mod value of 1). Realistically though, there aren't currently any base2 values between 1 & 100, it just leaves open the possibility for customization while keeping it simple.

However, based on that formula, it sounds like the calculations should be right.

[KLS]

Well I mean they've parsed it to be a correct formula. And you could still use a negative modifier to make things proc less under it. Unless we have everything unsigned but we can prolly change that.