Gravity Spellpack v1.2

[Moderator]

12th Dec 2015
IcemanBo: Too long as NeedsFix. Rejected.


11 Nov 2011
Bribe: Instead of StringHash("count") you could use the child key "-1" for example.

FirstOfGroup loop here instead of group filtering.

You should use pathing checks to make sure that units don't get knocked out of bounds and crash the game.

What's the point in saving 0.0 in a hashtable? It is already 0.0 by default.

Since you only use "key 0" in the hashtable you should use Table, or even better a unit indexer for this. Wait you have AIDS in your map, so why are you using a hashtable and getting the handle Id in the first place? You do this in both "GravityFlux" and "GravityMatrix".

Just use "null" for weapon type for spells.

For global groups just use "bj_lastCreatedGroup".

 

[88WaRCraFT3]

Sounds great gotta test.

 

[Rexp]

I tested it there where no test enemies so i added and then i tryed to launch again then evrything just went wrong all triggers disabled and such.... think it needs a fix there
EDIT: Didn't see that i could press down arrow for spawns but anyways u should be able to put out unit's without the spell crashing

 

[watermelon_1234]

Code Check:

Inversion Field

• There's really no need to use a local variable to refer to tempData.
  This applies to other group enumerations.
• In static method GroupFilter:
  

  set dx = GetUnitX(u) - this.x
  set dy = GetUnitY(u) - this.y

  That way, you can do this
  

  set a = Atan2(dy, dx)

  This applies to other instances like GroupFunc.
• I think the number of special effects that are spawned should be configurable.
• There is no need for a when you loop to create the special effects.
  dx and dy can be set like this:
  

  set dx = this.x + this.aoe * Cos(bj_PI*2/count * i)
  set dy = this.y + this.aoe * Sin(bj_PI*2/count * i)

• if GetUnitAbilityLevel(u, ABILDUMID) > 0 and GetUnitAbilityLevel(t, ABILDUMID) > 0 then
  elseif GetUnitAbilityLevel(u, ABILDUMID) > 0 then

  ?
• You could use local integer variables to store the ability level in DamageReduction.
• There is no need for the local effect variable e.
  

  call DestroyEffect(AddSpecialEffectTarget(DAMAGEART, t, DAMAGEATTACH))

• You don't have to use another local trigger variable for the damage detection. You could just do something like this:
  

  set t = CreateTrigger()
  call Damage_RegisterEvent(t)
  call TriggerAddAction(t, function DamageReduction)

  This applies to Gravity Matrix as well.

Gravity Matrix

• Attacktype, damagetype, and weapontype could be configurable.
• There is no need for the local effect variable e.
  

  call DestroyEffect(AddSpecialEffectTarget(MANAART, u, MANAATTACH))

• You could have used Tempunit all along to refer to GetTriggerUnit instead of having to declare the local unit variable t.
• 1000.0 should be configurable.

Gravity Flux

• Yes, you destroyed special effects correctly! ^o^
• When you set check in FilterFunc, you can do something like this:
  

  set check = IsUnitAlly(u, GetOwningPlayer(this.caster)) and GetUnitState(u, UNIT_STATE_MANA) > 0

• Using static ifs to determine if an ally or an enemy unit can be affected is a bit too much. I would provide a configuration function at the top to allow the user to customize the targets. (Know that your current method can bug though if both ALLY and ENEMY are set to true.)
• unitmana could have been set earlier since it's going to be used to determine the check in FilterFunc.
• In the Actions method, you could null t after TimerStart

Graviton Acceleration

• Use radians directly.
  

  set this.angle = bj_PI*2* (INTERVAL / SPINRATE)

  That way, you don't need to convert from degrees in GroupFunc.
• No need for local unit u in Actions.
• You could preload the dummy unit.

 

[The_Swamp_Warrior]

could this have been inspired by mine?

 

[MortAr]

I smell a BlackRose apprentice.

 

[Lambdadelta]

I smell a BlackRose apprentice.

o.o?

 

[iAyanami]

Updated the codes.

Code Check:

Inversion Field

• There's really no need to use a local variable to refer to tempData.
  This applies to other group enumerations.

True. But I just like using the "this" keyword. I don't really want to name everything tempData.caster, etc.

I smell a BlackRose apprentice.

?

 

[Nestharus]

Don't use KT2... use http://www.hiveworkshop.com/forums/submissions-414/snippet-timerqueue-187502/ instead ; |. It makes a world of difference.

private static method expire takes nothing returns nothing
//this only runs once, but it can be set to repeat by adding the timer back into the queue
            local thistype this = thistype(0).nextTimer
            local real offset = timeOffset
            local thistype next
            local integer prev = currentTimer
            loop
                set next = nextTimer
                set nextTimer = recycle
                set recycle = this
                set expired = true
                set currentTimer = this
                //code
                set this = nextTimer
                exitwhen timeOffset != offset or this == 0
            endloop
            set currentTimer = prev
            set thistype(0).nextTimer = this
            if (this != 0) then
                call TimerStart(expireTimer, timeOffset-recycle.timeOffset, false, function thistype.expire)
            endif
endmethod

Add in code

if (lastTimer == 0) then
    call TimerStart(expireTimer, INTERVAL, false, function thistype.expire)
endif
set timeOffset = INTERVAL-TimerGetRemaining(expireTimer)
set lastTimer.nextTimer = this
set lastTimer = this
set nextTimer = 0

->AIDS

Try Unit Indexer instead (linked in sig). Read the top of the thread to read a compare/contrast on the three indexing systems.

->Event

Event linked in my sig is a lot faster.

->Recycle

Recycling shouldn't really be done to minimize memory. Speed is good, but so is minimal memory. This is why I recycle very rarely now (only recycle units).

->Damage
Check out AdvDamageEvent ;o.

 

[illegalpie]

Don't use KT2... use http://www.hiveworkshop.com/forums/submissions-414/snippet-timerqueue-187502/ instead ; |. It makes a world of difference.

private static method expire takes nothing returns nothing
//this only runs once, but it can be set to repeat by adding the timer back into the queue
            local thistype this = thistype(0).nextTimer
            local real offset = timeOffset
            local thistype next
            local integer prev = currentTimer
            loop
                set next = nextTimer
                set nextTimer = recycle
                set recycle = this
                set expired = true
                set currentTimer = this
                //code
                set this = nextTimer
                exitwhen timeOffset != offset or this == 0
            endloop
            set currentTimer = prev
            set thistype(0).nextTimer = this
            if (this != 0) then
                call TimerStart(expireTimer, timeOffset-recycle.timeOffset, false, function thistype.expire)
            endif
endmethod

Add in code

if (lastTimer == 0) then
    call TimerStart(expireTimer, INTERVAL, false, function thistype.expire)
endif
set timeOffset = INTERVAL-TimerGetRemaining(expireTimer)
set lastTimer.nextTimer = this
set lastTimer = this
set nextTimer = 0

->AIDS

Try Unit Indexer instead (linked in sig). Read the top of the thread to read a compare/contrast on the three indexing systems.

->Event

Event linked in my sig is a lot faster.

->Recycle

Recycling shouldn't really be done to minimize memory. Speed is good, but so is minimal memory. This is why I recycle very rarely now (only recycle units).

->Damage
Check out AdvDamageEvent ;o.

Don't advertise your shit here. All I need to say.


@iAyanami Edit: I like the spells, only a few comments.
Graviton Acceleration: I don't like how the movements are so choppy, it's like they're blinking around the sphere, not moving around it, it ruins that spell. Other than that, good job, only other thing I could suggest is that you add some more special effects on say the mana shield.

 

[Nestharus]

Don't advertise your shit here. All I need to say.

I'll ignore that statement and ignore any future refs as if they were never posted ; ).

So yea, update to TimerQueue asap ;D. The rest were just suggestions, but not much of a deal, just the TimerQueue one. You shouldn't be using KT2 for this, and in fact, using it for this is wrong ;P.

 

[iAyanami]

I'll ignore that statement and ignore any future refs as if they were never posted ; ).

So yea, update to TimerQueue asap ;D. The rest were just suggestions, but not much of a deal, just the TimerQueue one. You shouldn't be using KT2 for this, and in fact, using it for this is wrong ;P.

Yeah, actually, I've stopped using KT myself. This was made before I actually stopped using KT. Will update it soon. I'm now currently using TimerUtils, T32 or TimerQueue for timer systems.

Also, I'm using AIDS because it's needed for Damage. And I'm using Damage because I want it to support the Damage system. I might add support for just the AddDamageEvent though.

EDIT
Updated the Spellpack to version 1.2

Changelogs

Version 1.2

- Reworked code using Timer32, GTrigger and GroupUtils, instead of KeyTimers2 and Recycle

More feedback will be appreciated. Thanks

 

[baassee]

2PI should be stored into a global constant.

In the Graviton Acc spell you still use squareroot while in all the other spells you avoided it. this.aoe * this.aoe will fix it
This line

if dist > this.aoe or not UnitAlive(u) then

Why don't you just use Damage's SpellWrapper instead of UnitDamageTargetEx? Seems like a spell to me

That was just a quick check, haven't tested it in-game yet.

 

[Nestharus]

I might add support for just the AddDamageEvent though.

Well, currently AdvDamageEvent has 2 bugs, one having to do with the wc3 engine itself and the other having to do with the fact that nobody's found a good way to determine whether or not a unit is flagged to explode. At the moment, no units explode (think back to artillery attacks like canon where the unit explodes). Some maps do depend on that flag, but not the end of the world. I'll eventually get it working tho >: ). The last post on the thread actually discusses a way to find out whether a unit will make another unit explode or not (yes, me n Bribe finally figured it out), but there are issues with it ;P.

The other bug has to do with the fact that damage is applied from chains of expiring attacks, but this one can be fixed with attack indexing, which I'll eventually get back to.

I guess AdvDamageEvent is safe to use on most maps (most maps don't rely on the explode flag) =). The API for it won't change, just the back code, so if you do add support for it, you won't have to worry about it later on when I implement the explosion fix (once I solve it).

TimerUtils, T32 or TimerQueue for timer systems

Good suite ; )

Also, Event by j4l does run a lot slower than the one at THW (it's been proven), so just a thought ;P. There's no way to support both libs.

 

[baassee]

Isn't it possible to support both Events? LOL I thought that's what our static ifs are for.

 

[Nestharus]

They both have the same lib name and they have some same method names that take different arguments, so no.

 

[Bribe]

I like this one a lot better than the previous spellpack I reviewed from you. It's more dynamic and gives players some extra mechanics in-game.

Improvements:

- When units start to be damaged by the black hole, if you changed the lightning's alpha color to red that would really help the effect.
- The effect for Inversion Field is really bad. You can only see half of the dark summoning missile because it's on the ground. Also, when units reach the edge, they just look like they're glitching the whole time making it look really weird. If you made some kind of a constant "push/pull" effect on the respective units (moving them over time instead of instantly) this would look better.

 

[SpectreD]

very impressive spellpack keep up the good work 5/5