Canary Will this spell crash the server? in any way?

[endziu2222]

local celestialJudgment = Spell("instant")

function celestialJudgment.onCastSpell(creature, var)
    local origin = creature:getPosition()
    local casterId = creature:getId()
    creature:say("Judgment!", TALKTYPE_MONSTER_SAY)
    doSendMagicEffect(origin, CONST_ME_HOLYAREA)
    
    -- Stage 1: Fire a holy projectile at the target after 500ms.
    addEvent(function()
        local caster = Creature(casterId)
        if not caster or not caster:isCreature() then
            return
        end
        local target = caster:getTarget()
        if not target or not target:isCreature() then
            return
        end
        
        local combatBeam = Combat()
        combatBeam:setParameter(COMBAT_PARAM_TYPE, COMBAT_HOLYDAMAGE)
        combatBeam:setParameter(COMBAT_PARAM_EFFECT, CONST_ME_HOLYAREA)
        combatBeam:setFormula(COMBAT_FORMULA_DAMAGE, -4000, 0, -9000, 0)
        doSendDistanceShoot(origin, target:getPosition(), CONST_ANI_HOLY)
        combatBeam:execute(caster, positionToVariant(target:getPosition()))
    end, 500)
    
    -- Stage 2: After 2500ms, cause a ring explosion around target's current position.
    addEvent(function()
        local caster = Creature(casterId)
        if not caster or not caster:isCreature() then
            return
        end
        local target = caster:getTarget()
        if not target or not target:isCreature() then
            return
        end
        
        local targetPos = target:getPosition()
        local explosionCombat = Combat()
        explosionCombat:setParameter(COMBAT_PARAM_TYPE, COMBAT_HOLYDAMAGE)
        explosionCombat:setParameter(COMBAT_PARAM_EFFECT, CONST_ME_HOLYAREA)
        explosionCombat:setFormula(COMBAT_FORMULA_DAMAGE, -5000, 0, -9000, 0)
        
        local explosionRadius = 3
        for dx = -explosionRadius, explosionRadius do
            for dy = -explosionRadius, explosionRadius do
                local pos = Position(targetPos.x + dx, targetPos.y + dy, targetPos.z)
                explosionCombat:execute(caster, positionToVariant(pos))
                doSendMagicEffect(pos, CONST_ME_HOLYAREA)
            end
        end
    end, 2500)
    
    return true
end

celestialJudgment:name("Judgment")
celestialJudgment:words("##judgment")
celestialJudgment:isAggressive(true)
celestialJudgment:blockWalls(true)
celestialJudgment:needLearn(true)
celestialJudgment:register()

 

[Gesior.pl]

These checks are useless (or checks):

        local caster = Creature(casterId)
        if not caster or not caster:isCreature() then
            return
        end
        local target = caster:getTarget()
        if not target or not target:isCreature() then
            return
        end

it can be just:

        local caster = Creature(casterId)
        if not caster then
            return
        end
        local target = caster:getTarget()
        if not target then
            return
        end

Creature(x) and creature:getTarget() already return Creature object or nil, so if creature does not exist or caster has no target, it will return nil, which is false in if.

It should not crash engine. Only variable from outer scope inside addEvent is local origin = creature:getPosition() passed as origin and it should be safe to use Position object in Lua. At least on TFS, idk if canary made some changes to Position, but it should not. Position is just a table with 3 values (x,y,z).

To make code easier to read, you should pass all variables as parameters to delayed functions and name delayed functions ex.:

local celestialJudgment = Spell("instant")

local function delayedFunctionOne(casterId, origin)
    local caster = Creature(casterId)
    if not caster or not caster:isCreature() then
        return
    end
    local target = caster:getTarget()
    if not target or not target:isCreature() then
        return
    end
    
    local combatBeam = Combat()
    combatBeam:setParameter(COMBAT_PARAM_TYPE, COMBAT_HOLYDAMAGE)
    combatBeam:setParameter(COMBAT_PARAM_EFFECT, CONST_ME_HOLYAREA)
    combatBeam:setFormula(COMBAT_FORMULA_DAMAGE, -4000, 0, -9000, 0)
    doSendDistanceShoot(origin, target:getPosition(), CONST_ANI_HOLY)
    combatBeam:execute(caster, positionToVariant(target:getPosition()))
end

local function delayedFunctionTwo(casterId)
    local caster = Creature(casterId)
    if not caster or not caster:isCreature() then
        return
    end
    local target = caster:getTarget()
    if not target or not target:isCreature() then
        return
    end
    
    local targetPos = target:getPosition()
    local explosionCombat = Combat()
    explosionCombat:setParameter(COMBAT_PARAM_TYPE, COMBAT_HOLYDAMAGE)
    explosionCombat:setParameter(COMBAT_PARAM_EFFECT, CONST_ME_HOLYAREA)
    explosionCombat:setFormula(COMBAT_FORMULA_DAMAGE, -5000, 0, -9000, 0)
    
    local explosionRadius = 3
    for dx = -explosionRadius, explosionRadius do
        for dy = -explosionRadius, explosionRadius do
            local pos = Position(targetPos.x + dx, targetPos.y + dy, targetPos.z)
            explosionCombat:execute(caster, positionToVariant(pos))
            doSendMagicEffect(pos, CONST_ME_HOLYAREA)
        end
    end
end

function celestialJudgment.onCastSpell(creature, var)
    local origin = creature:getPosition()
    creature:say("Judgment!", TALKTYPE_MONSTER_SAY)
    doSendMagicEffect(origin, CONST_ME_HOLYAREA)
    
    local casterId = creature:getId()
    -- Stage 1: Fire a holy projectile at the target after 500ms.
    addEvent(delayedFunctionOne, 500, casterId, origin)
    
    -- Stage 2: After 2500ms, cause a ring explosion around target's current position.
    addEvent(delayedFunctionTwo, 2500, casterId)
    
    return true
end

celestialJudgment:name("Judgment")
celestialJudgment:words("##judgment")
celestialJudgment:isAggressive(true)
celestialJudgment:blockWalls(true)
celestialJudgment:needLearn(true)
celestialJudgment:register()

It does not make your code 100% safe, because you can still declare some 'global variable' somewhere (any file) and use same name in addEvent callback function, it will work.. until player logouts, then it will crash, but it's much safer and easier to read.