Spell Temporal Vortex

[saw792]

I suggest you leave the offensive subskill as enemy and allied units and make it toggleable.

 

[WolfieeifloW]

Multiple leaks
1) GetUnitsInRangeOfLocAll not set to a variable
2) Enumerating function with a null boolexpr leaks IIRC (check the Function List and you'll see it)
3) GetUnitLoc

Fixed, as far as I know.

That GroupClear is unnecessary - group is going to be destroyed anyway, why spend time emptying it

Removed, thanks.

Also, move those leak-removing/nulling stuff outside the if's - if these conditions aren't met

Done.

What's the point of the integer? I can't see anything else using it

Removed.

Why? Quite pointless to have those there, nobody needs to modify them anyway

Removed.

Why not make that into a formula function, rather than restricting it to a multiplier of ability level e.g.

function GetChance takes integer stat returns nothing
  return 5 + stat * 5
endfunction

and let people devise their own method of determining the % chance

I don't get this part, sorry.

I suggest you leave the offensive subskill as enemy and allied units and make it toggleable.

What do you mean?
The skill is an autocast skill, how would I make it toggleable between enemies and allies? (if i understood you correctly)


UPDATE

Changelog
v1.4c
- Code cleanup
Credits to Flare
- Fixed map description
- Fixed tooltip not showing level 4 for Offense
- Fixed code not showing description for Offense

 

[saw792]

As in toggleable on and off so that you can control when it can move units to you, if you get what I mean. Basically you would base the ability on immolation and enable the offensive triggers when it's on (detect the "immolation" order) and disable the offensive triggers when it is turned off ("unimmolation"). I can imagine that skill pissing me off a great deal if it kept randomly sucking units towards me of which I have no control. If you can at least choose when you want it to pull units (with the % chance still, of course) it adds more depth and skill to the ability.

 

[WolfieeifloW]

It's toggleable as it is right now;
It just sucks enemies and allies alike.
I'm hoping to make it only suck in enemies in the next version.

 

[saw792]

Probably should have tested that first.

Leave it as both so that there is actually some skill involved in using it.

 

[WolfieeifloW]

Alrighty !
Thanks for the comments nonetheless !

 

[Flare]

I don't get this part, sorry.

What you have is (ability level) * (chance multiplier), and it doesn't allow for much freedom with your values. Let's say you want 10% chance at Level 1. With your formula, you have to have 20% with Lv2, 30% with Lv3, 40% with Lv4 - those values are quite large, and may not be desirable.

What I'm suggesting is implementing a configuration function that lets the end-user determine their own formula e.g.

function GetChance takes integer stat returns nothing
  return 7 + stat * 3
endfunction

(to give 10-19% chance, based on 4 levels of the ability)

Then, when your doing the chance check, just replace the (GetUnitAbilityLevel * ChanceMultiplier) with

GetChance (GetUnitAbilityLevel (...))

The only con would be the fact that 'stat' would be fixed at GetUnitAbilityLevel (unless someone wants to look through the code and change it), but the same already applies to what you're currently doing (i.e. you can't decide what ChanceMultiplier is multiplied by), but it's a bit of extra hassle to do it any other way

 

[WolfieeifloW]

Alrighty.
I'm pretty sure I did everything right (i seemingly did it right) .


UPDATE

Changelog
v1.5
- Major code revamp
Certain configs (can) use formulas now
Credits to Flare

EDIT: I'm looking to make this (v1.5b) the final version.
UPDATE

Changelog
v1.5b
- Code touchup
Replaced a location with X/Y

 

[perkeyone]

nice spell
i loved how the ability has both offense and defense styles
would be a good ability in rpg's
useful for both the tanks and the more fragile units

 

[WolfieeifloW]

!
Thanks.
Maybe this will find it's way into someone's map someday !

 

[Kenny]

Nice spell. The offensive and defensive aspects of the spell made it a lot better in general and far more feasible for a variety of different situations. Good work! Ill give this 4 stars because of how far the spell has come.

 

[WolfieeifloW]

Nice spell. The offensive and defensive aspects of the spell made it a lot better in general and far more feasible for a variety of different situations. Good work! Ill give this 4 stars because of how far the spell has come.

!
Thank you very much kenny!.
Glad you liked the spell !

Keep the comments/suggestions/ratings coming people !

 

[Tyrande_ma3x]

Seems good, no apparent code mistakes or anything. +rep, also rated 5.

 

[WolfieeifloW]

Thanks for looking it over.
And extra thanks for the rating !

Anymore comments/ratings TH.netters ?

 

[Lumograph090]

Another fantastic ability! great effects as always.

 

[WolfieeifloW]

Another fantastic ability! great effects as always.

Thanks !
Glad you liked it.

Keep the comments/ratings coming guys!

 

[Kenny]

loop
                    set pickedUnit = FirstOfGroup(offGroup)
                    exitwhen pickedUnit == null
                        set pickedUnit = FirstOfGroup(offGroup)
                        call DestroyEffect(AddSpecialEffectTarget(pullStart, pickedUnit, startAttach))
                        call SetUnitPosition(pickedUnit, tTVX, tTVY)
                        call DestroyEffect(AddSpecialEffectTarget(pullEnd, pickedUnit, endAttach))
                        call GroupRemoveUnit(offGroup, pickedUnit)
                        set pickedUnit = null
                    endloop

Setting pickedunit = FirstOfGroup(offGroup) twice. And you really dont need to set pickedunit = null, but i think that has already been addressed.

call AddSpecialEffectTarget(OFFSFX, offU, SFXAttach)
            call DestroyEffect(bj_lastCreatedEffect)

Why not just use DestroyEffect(AddSpecialEffectTarget()) ? Or did i miss something?

And just out of curiosity, what does:

private function DefTRYActions takes nothing returns nothing
        local unit tryU = GetTriggerUnit()
        call IssueImmediateOrder(tryU, "holdposition")
        call TriggerSleepAction(0.01)
        call SetUnitAnimation(tryU, "walk")
        set tryU = null
    endfunction

Do?

 

[WolfieeifloW]

Setting pickedunit = FirstOfGroup(offGroup) twice. And you really dont need to set pickedunit = null...

Yeah it has.
Thought I fixed it .

Why not just use DestroyEffect(AddSpecialEffectTarget()) ? Or did i miss something?

The SFX I used can't be done by that action.

And just out of curiosity, what does: ... Do?

Stops the unit from casting the spell (so it's only autocastable) .


Update

Changelog
v1.6b
- Fixed loop pickedUnit setting
Credits to kenny!

 

[Kenny]

The SFX I used can't be done by that action.

Yes but that doesn't mean other peoples won't work either. Using that just seems un-necessary, at least make it a variable or something.

Stops the unit from casting the spell (so it's only autocastable) .

Then why play the "walk" animation? what if you are standing still? I think i simple Pause, "stop", Unpause should work just fine. And if you want to make it awesome, chuck in a SimError() .

 

[WolfieeifloW]

The SFX is pwn, who wouldn't want to use it .
And the walk animation I play so that the unit doesn't continue his casting animation.
I don't know what a SimError() is but what I have works perfectly fine so !
Thanks for the comments.