Spell Issues

[Jabantiz]

I fixed an issue where if the initial cast of a spell is resisted its ticks were still being applied. In my effort to fix this I discovered that spells never called the tick() function in its spell script and fixed that issue (Details for Zcoretri, and those who are curious, below), also fixed Bug ID: 602 but not sure on that one, details in its post, and finally made it so ticks can't be resisted if the inital cast landed, although we should probably have some sort of flag for this.

I only set out to fix the first problem and kept stumbling upon more issues and that is the purpose of this post, if you know of any more issues with spells post them here and I will look into them, the only other issue I am aware of right now is the casting bar can get stuck open and prevent the player from casting anymore and not sure how to fix that.



Now for the details.

When a spell is cast, if it has a duration it is added to a MutexList, in the main loop for the spell process it goes through that list and cast that spell again if it was time for a tick, and it would always call the cast() function in the lua script, the fix was actually simple all I had to do was change

Code: Select all

if(!ProcessSpell(spell))

to

Code: Select all

if(!ProcessSpell(spell, false))

and now tick() will be called.

I also verified tick() was never called by modifying soulrot.lua (necro DoT) and added Say(Caster, "Cast!!!") in the cast function, Say(Caster, "Tick!!!") in the tick function and Say(Caster, "Remove!!!") in the remove function, cast was said on every tick and remove when it was finished.

[John Adams]

Some historical info on Tick(), which far as I remember, used to work fine.

LE's original thought

Solution

Talk of DoTs

and as you see here, it used to work:
http://www.eq2emulator.net/phpBB3/viewt ... 8127#p8127


If you can root out WHY this stopped working, it might go a long way towards preventing us from breaking things when new features are implemented in the future.

[Jabantiz]

No clue why it stopped working, can only assume the false was removed in some commit, checked svn and in rev1310, jun 2011 (as far back as tortise svn is showing me) the false wasn't there so it has been broken since at least then.

[Jabantiz]

Found an issue where if a spell had a duration but wasn't an until canceled spell and didn't have a tick function in the lua script (short duration buffs like the tradeskill spells) it would cause stats from the buff never being removed, client would get booted eventually, server dead lock, and eventually a server crash. Found and fixed the cause and added details in a comment in the code.

Code: Select all

// ProcessSpell(spell, flase) will atempt to call the tick() function in the lua script
// if there is no tick function it will return false, this will cause the server to crash in the event
// of a spell that has a duration but is not a "until canceled" spell or a spell with a tick (tradeskill buffs)
// to counter this check to see if the spell has a call_frequency > 0 before we call ProcessSpell()
if(spell->spell->GetSpellData()->call_frequency > 0 && !ProcessSpell(spell, false))

Also there is an issue where if you cast the spell again before it was removed from your maintained spells it would cancel the effect instead of refreshing it. This can cause some annoying results while crafting but won't cause any issues. I haven't hunted this bug down yet.

There is also the issue of shared timers we don't currently support. This is where 1 spell is cast and can set the recast of itself and another spell, this will let the player use all 6 tradeskill spells at once making it extremely easy to craft.

And finally one not related to tradeskills, we should add an option to determine if a spell can be resisted per tick. Spells, such as DoT's, can never be resisted on a tick, only on the initial cast. Others, such as charm's, can be resisted every tick.

[Jabantiz]

Tradeskill spells, or other short duration buffs, should no longer cancel out a previous cast that hasn't expired yet, should now just refresh the timer on that effect.

[KhaineGB]

Ultraviolet Beam - Illusionist spell issue.

Casting the spell works fine. It does damage as it's supposed to (assuming it's not resisted), and the reuse timer starts. This is where the problem kicks in. If you are in combat, the reuse timer clears properly. If you cast the spell and kill the mob, thus placing you OUT of combat, the reuse timer "appears" to complete, but the spell icon is still greyed out as if it's still under reuse.

If you enter combat and try to cast the spell, it will cast with no problems. However, the spell then uses the old, partially completed reuse timer, rather than starting a new one. For the sake of argument, let's say the icon is greyed out and the spell has 0.1 seconds left on reuse. If you enter combat, you can cast the spell, but it is then subject to the 0.1 second reuse, rather than the 2 second reuse.

That means you can basically cast the spell, and then cast it again IMMEDIATELY. As mentioned, this only seems to happen if the spell is under reuse and the character goes out of combat. During combat, it works as it should.

[Jabantiz]

Interesting, never seen that issue before. Thanks for the report and will look into it when I get the time.

[KhaineGB]

No problem. I'll pop on later and see if I have the same issue with DoT spells as well.

[John Adams]

Also there is an issue where if you cast the spell again before it was removed from your maintained spells it would cancel the effect instead of refreshing it. This can cause some annoying results while crafting but won't cause any issues. I haven't hunted this bug down yet.

Add to this, I was just working on a Until Cancelled spell (Armored) and noticed I could not "cancel" the spell to see that the effects wore off. Upon stepping into the code, I found that active_spells.size() was 0, unless you cast the spell a second time - THEN you can Cancel it.

Code: Select all

bool SpellProcess::DeleteCasterSpell(Spawn* caster, Spell* spell){

	bool ret = false;	
	if (caster && spell && active_spells.size() > 0) {

Where I saw some funkiness was in SpellProcess::CastProcessedSpell(...). I put a break point in the "replace_spell" if, and never got hit. Yet below, the if(replace_spell) did get hit when I cast the spell the first time. Then, the code gets hit I think 2 more times... but the 2nd time it is outside the if(replace_spell) and bumps the active_spells.Add() normally.

Not sure if this made sense. I'll look into it further if no one else has time. But the workaround for now is to cast your Maintained Spell twice.

[Jabantiz]

Add to this, I was just working on a Until Cancelled spell (Armored) and noticed I could not "cancel" the spell to see that the effects wore off. Upon stepping into the code, I found that active_spells.size() was 0, unless you cast the spell a second time - THEN you can Cancel it.

This issue should be fixed and maintained spells should now function properly, issue seemed to be with the MutexList, replacing it with a vector created interesting (read bad) results, however I figured out how to force update the MutexList so it should cancel properly.

Found and fixed a crash bug where if you tried to cancel a maintained spell by casting the spell again before the reuse timer was up.

Also found maintained spells were being added to the list twice and fixed that issue.

Found and fixed a crash bug with /reload spells while a player has a maintained spell active. Maintained spells should now be killed for both players and npcs when using /reload spells.

[Jabantiz]

Fixed an issue where an instant cast entity command cast through an object, sign, or spawn shaded all the spell icons and they remained shaded.

Reworked how we shade the spell icons, this required a lot of changes throughout the server so more extensive testing is needed.

In the rework I also added linked timers, if two spells have the same timer set in the DB then both will have there recast set when either are cast. This will only work if the timer ID is greater then 0. This will effect all spells the player has with the same timer ID it is not limited to just 2 spells. Also currently the spells will use their own recast timers, not the timer of the spell actually cast, so it is possible the spells will be on a recast of different times, not sure if it should stay that way or use the recast of the spell actually cast, thoughts?

Also tradeskill spell shading should be fixed.

[thefoof]

Also currently the spells will use their own recast timers, not the timer of the spell actually cast, so it is possible the spells will be on a recast of different times, not sure if it should stay that way or use the recast of the spell actually cast, thoughts?

If we have spells on linked timers it would probably be better to use the recast of the spell that was cast, IMO.

[John Adams]

if two spells have the same timer set in the DB then both will have there recast set when either are cast.

Jab, in your packet sniffing have you ever noticed a common link in the raw packet data (collects) that ties 2 spells together in this way? If not, updating will be a manual process.

[Jabantiz]

I have not seen a link between the spells in packets, although I was never looking for one, I will go through a log I got from beta to see if I can find something but I have a feeling it will mostly be manual work. The good news is tradeskill spells is most of the work, outside of tradeskill spells shared timers seem rare.

[John Adams]

I thought I came up with a scenario, but then it occurred to me that 'spell lines' replace each spell when you get the next line, right? Kinda like you do not have Heal I, Heal II, Heal III apprentice, journeyman, master all in your spell book/bars at the same time... right? If so, that would be the only obvious linked timers we'd need, and could probably handle that programmaticly vs having to manually tie them all together with IDs.