[Trigger] My first Hash spell - Is it good?

[HammerFist]

I think am I make it leakless and removed all what I do not need, or have something to much
Here are triggers

• Spinning Blade Start
  • Events
    • Unit - A unit Starts the effect of an ability
  • Conditions
    • (Ability being cast) Equal to Spinning Blade
  • Actions
    • Set SBCaster_Player = (Owner of (Casting unit))
    • Set SBCaster_Point = (Position of (Casting unit))
    • Set SBTargets_UnitGroup[0] = (Units within 250.00 of SBCaster_Point matching ((((Matching unit) is dead) Equal to False) and (((Matching unit) belongs to an enemy of SBCaster_Player) Equal to True)))
    • Unit Group - Pick every unit in SBTargets_UnitGroup[0] and do (Actions)
      • Loop - Actions
        • Special Effect - Create a special effect attached to the chest of (Picked unit) using Abilities\Weapons\LavaSpawnMissile\LavaSpawnBirthMissile.mdl
        • Special Effect - Destroy (Last created special effect)
        • Unit - Add Spinning Blade (Slow) to (Picked unit)
        • Unit - Turn collision for (Picked unit) Off
        • Unit - Cause (Casting unit) to damage (Picked unit), dealing (2.20 x (Real((Strength of (Casting unit) (Exclude bonuses))))) damage of attack type Spells and damage type Normal
        • Set SBTimer_Real = 5.00
        • Set SBTarget_Point[0] = (Position of (Picked unit))
        • Set SBAngle_Real = (Angle from SBCaster_Point to SBTarget_Point[0])
        • Hashtable - Save SBAngle_Real as 1 of (Key (Picked unit)) in SB_Hash
        • Hashtable - Save SBTimer_Real as 2 of (Key (Picked unit)) in SB_Hash
        • Unit Group - Add (Picked unit) to SBTargets_UnitGroup[1]
    • Custom script: call DestroyGroup(udg_SBTargets_UnitGroup[0])
    • Custom script: call RemoveLocation(udg_SBCaster_Point)
    • Custom script: call RemoveLocation(udg_SBTarget_Point[0])

• Spinning Blade Loop
  • Events
    • Time - Every 0.03 seconds of game time
  • Conditions
  • Actions
    • Unit Group - Pick every unit in SBTargets_UnitGroup[1] and do (Actions)
      • Loop - Actions
        • Set SBTimer_Real = (Load 2 of (Key (Picked unit)) from SB_Hash)
        • If (All Conditions are True) then do (Then Actions) else do (Else Actions)
          • If - Conditions
            • SBTimer_Real Greater than 0.00
          • Then - Actions
            • Set SBTarget_Point[1] = (Position of (Picked unit))
            • Set SBTarget_Point[2] = (SBTarget_Point[1] offset by 15.00 towards (Load 1 of (Key (Picked unit)) from (Last created hashtable)) degrees)
            • Unit - Move (Picked unit) instantly to SBTarget_Point[2]
            • Hashtable - Save (SBTimer_Real - 0.30) as 2 of (Key (Picked unit)) in SB_Hash
          • Else - Actions
            • Special Effect - Create a special effect attached to the chest of (Picked unit) using Abilities\Weapons\LavaSpawnMissile\LavaSpawnBirthMissile.mdl
            • Special Effect - Destroy (Last created special effect)
            • Unit - Remove Spinning Blade (Slow) from (Picked unit)
            • Unit - Turn collision for (Picked unit) On
            • Unit Group - Remove (Picked unit) from SBTargets_UnitGroup[1]
            • Hashtable - Clear all child hashtables of child (Key (Picked unit)) in SB_Hash
    • Custom script: call RemoveLocation(udg_SBTarget_Point[0])
    • Custom script: call RemoveLocation(udg_SBTarget_Point[1])

Any working suggestion for making thing what low fps removed +REP

 

[defskull]

Set SBCaster_Player = (Owner of (Casting unit))

Change to Triggering player as it only call 1 function instead of 2

Set SBCaster_Point = (Position of (Casting unit))

You should save the caster in a variable first and use it like this

• Set SBCaster = (Triggering unit)
• Set SBCaster_Point = (Position of SBCaster)

Set SBTargets_UnitGroup[0] = (Units within 250.00 of SBCaster_Point matching ((((Matching unit) is dead) Equal to False) and (((Matching unit) belongs to an enemy of SBCaster_Player) Equal to True)))

You can choose either to create a Unit Group or not for this, but since it's only temporary, you should not create Unit Group, just use this above the Unit Group action you want to use

• Custom script: set bj_wantDestroyGroup = true

That way, you don't need extra variable to be used (this is useful if you use a Unit Group that is single-time only)

Hashtable - Save SBAngle_Real as 1 of (Key (Picked unit)) in SB_Hash

You should use this to set as (Picked unit) (create a new Unit variable named PickedUnit)

set udg_HandleID = GetHandleId(udg_PickedUnit)
This is more efficient than what you are doing now as it call less function

Create a new Integer variable named HandleID then do as above function
Then, you should save your data like this

• Hashtable - Save SBAngle_Real as 1 of HandleID in SB_Hash

Also, you should name your data, not as 1 or 2 or 3, just name them appropriate with the data they carry
If above data, I suggest you should name it "Angle", this improve readability


In the loop trigger, do the same for set udg_HandleID = GetHandleId(udg_PickedUnit)

And after you have done this, load the data as usual.

Also, once you remove all those blades (the spell ended and there is no activity of this spell occur in the map), you should turn off the trigger (for efficiency) by checking is there anyone left in the Unit Group

• (udg_SBTargets_UnitGroup[1] is empty) Equal to True

Then turn off the trigger (will make your game less lag by giving your computer to perform less activity)

I suggest that you should download spells at the Spells Section and browse through their codes first
That way you can have a better understanding

 

[HammerFist]

Thank u dude +Rep. Will fix all things today and post spell once more time

 

[MortAr]

Hash is bad. Use dynamic indexing.

 

[Starquizer]

Hash is bad. Use dynamic indexing.

No and yes, take a look at this.

 

[MortAr]

Nothing to read.
Indexing > Hash.

 

[Starquizer]

Nothing to read.
Indexing > Hash.

But indexing has a limit.

 

[HammerFist]

when i would know what is indexing xD

 

[defskull]

Indexing is the use of array for up to 8192 and that's it...
Hashtable also has its usage limit for up to 255 (something like that)

You can't see the efficient of these 2 methods which is the best with our eyes, the only thing that we see is MUI-ness, that's all that matters.

 

[Starquizer]

Indexing is the use of array for up to 8192 and that's it...
Hashtable also has its usage limit for up to 255 (something like that)

True, and if you need to make a 2D variable using array that 8192 is reduced by the array size. But on the other hand you can use the hashtable as a 2D array freely.

 

[HammerFist]

I was using arrays but they are somehow hard formaking mui spells and I never could make spell like this in array, on the other hand in hash I have created this spell in few sec

 

[CoLd Bon3]

Just asking - are you using my Spinning Blade spell and remake it? My variables were almost the same (check ma signature to see the spell)

 

[HammerFist]

Honestly not. I swear. I was thinking about spell for solo mini maping contest for one wow model based on warrior. I was thinking about paladin's spell from wow. And I was looking for name. This was start ''Bladestorm....er...that's used million times. Whrilwind thats to ahem what about splitting blade ......er somehow sux what about split weapon.....er no that to...spliting strike noooooo......spinning blade that's cool''
Honesty not this is just big and funny accident. I do not know you spell. My is based on Attack walk stand spin animation and knockback enemy units xD

 

[CoLd Bon3]

Honestly not. I swear. [...]

OMG, man... using my spell is not a crime, bro! This is why i have created it - for other people to use it. I'm just proud of me.

 

[mckill2009]

Hash is bad. Use dynamic indexing.

I preffer UnitIndexer over DI in GUI...
DI is messy but faster than hash and cant save/load things outside its function...
Learn jass/vjass for more efficient method of indexing called structs and you
can create unlimited number of hashes called Table(by Bribe)...