Kinetic Ring v1.3

[Moderator]

11:28, 29th Jul 2012
Magtheridon96: 4/5. This is a pretty neat spell.

Comments

• Like I said in the first review, you could use the values of the coordinate variables you're setting directly inside the AddLightningEx call.
  call AddLightningEx(udg_KR_LModel, true, GetUnitX(...), GetUnitY(...), GetUnitFlyHeight(...), GetUnitX(...), GetUnitY(...), GetUnitFlyHeight(...))

Other than that, good job, there doesn't seem to be any major issues. You still need to fix the above issues though. Only then would it be approved.

 

[-Derp-]

Pretty good looking spell. I found a funny effect that occurs if you cast the Kinetic Ring twice by the hero. Instead of resetting the spell, the effects double in size.

bloop

 

[Kingz]

Spells that use hashtables in general aren't properly useable when it comes to the same unit casting the spell multiple times.
That is what indexing is for.

 

[-Derp-]

Yep. It's caused from not using a dummy unit substitute and just saving the hero's ID.

 

[Almia]

Wow,i didn't notice that one! ill fix it

 

[Losam]

Nice Spell.

 

[ap0calypse]

• Set KR_Distance = (Distance between KR_CasterLoc and KR_PickedLoc)

The variable "KR_CasterLoc" doesn't exist in the trigger "KR_Loop", so it's just the middle of the map.

On a related note: that's a shitton of variables.
The X/Y-variables don't carry over to other maps (because the editor doesn't copy variables that are only used in JASS - or am I mistaken?) and they can just as well be removed entirely.
I always go by the rule of thumb that you never need more than 3 temporary locations, but you're using 6 of them.

Edit: there's more! You should take the Location Z into account as well, because I just had the problem that my terrain z was about -700 and the lightning effects were completely off.
Don't worry, you've got all the locations you need . You do need to keep the 2 Z-variables though, otherwise the custom script will be too long.
This is what I did to fix it:

• Custom script: set udg_TempReal1 = GetUnitFlyHeight( udg_TempUnit1) + GetLocationZ( udg_TempLoc1)+75.
• Custom script: set udg_TempReal2 = GetUnitFlyHeight( udg_TempUnit2) + GetLocationZ( udg_TempLoc2)+75.
• Custom script: set bj_lastCreatedLightning = AddLightningEx( "AFOD", true, GetUnitX(udg_TempUnit1), GetUnitY(udg_TempUnit1), udg_TempReal1, GetUnitX(udg_TempUnit2), GetUnitY(udg_TempUnit2), udg_TempReal2 )

(You can use "bj_lastCreatedLightning in your spell as well).
Endedit


(also, I want people to start using the variable "GlobalHashtable" for all spells/systems and use a "HashtableId" which is an integer like 'KiR' - may upgrade it to 4 digits though
I do accept that it's just a silly dream and it will probably never happen, but a man can have dreams!)

 

[Almia]

Thanks
but im a lazy coder,i dont update approved spells

 

[DESTROYER123]

I won't judge it by how the triggers are working (because I suck at that) but the spell looks really cool and performs perfect!!!
Good job!!!
5/5

 

[Almia]

Thanks

 

[Zwiebelchen]

Spells that use hashtables in general aren't properly useable when it comes to the same unit casting the spell multiple times.
That is what indexing is for.

Please... if you have no clue what you're talking about, then don't be a smartass.

Hashtables (when used right) work just as well as dynamic indexing.

 

[Almia]

Please... if you have no clue what you're talking about, then don't be a smartass.

Hashtables (when used right) work just as well as dynamic indexing.

What he meants is that Hashtables generally use GetHandleId when it comes to an MUI spell, apparently (just like UnitIndexer) doesn't support multiple instances of spells on the same unit.

Dynamic Indexing prevents that from happening.

 

[Zwiebelchen]

What he meants is that Hashtables generally use GetHandleId when it comes to an MUI spell, apparently (just like UnitIndexer) doesn't support multiple instances of spells on the same unit.

Dynamic Indexing prevents that from happening.

Nobody uses GetHandleId(unit) for the parent key except in systems that have their own hashtable. Why would you do that? You usually attach your spell to a timer, which can be uniquely created and destroyed (and in 99% of cases you need a timer anyway).

 

[IcemanBo]

Nobody uses GetHandleId(unit) for the parent key except in systems that have their own hashtable.

It is/was used often that the indexid or handleid of a unit serves as instance key for a spell.
And as Almia explained it won't allow the unit to have multiple instances at said spell or system.

Please... if you have no clue what you're talking about, then don't be a smartass.

Talking offensivly about this several years old post just out of nowhere... for what >.<

 

[Wasabee]

Dude this is a great idea.
A very good disabler spell.

 

[KILLCIDE]

Some global variables are strictly used in JASS scripts, so they will not be created when you import them into another map. For those getting undeclared variable errors or something similar, create the following variables and their type:

Variable Name	Type
KR_UX	Real
KR_UY	Real
KR_UZ	Real
KR_PickedX	Real
KR_PickedY	Real
KR_PickedZ	Real

I went ahead and updated the submission so that the error will no longer occur during import. You can either manually create these variables in your map, or re-import the submission with the new update.

 

[Keiu]

Some global variables are strictly used in JASS scripts, so they will not be created when you import them into another map. For those getting undeclared variable errors or something similar, create the following variables and their type:

Variable Name	Type
KR_UX	Real
KR_UY	Real
KR_UZ	Real
KR_PickedX	Real
KR_PickedY	Real
KR_PickedZ	Real

I went ahead and updated the submission so that the error will no longer occur during import. You can either manually create these variables in your map, or re-import the submission with the update.

Thanks for responding, I didn't see your reply before I delete my post. I exported the map script and open the script file with notepad, so I can see the JASS scripts and add missing variables, now it works!

 

[KILLCIDE]

Cool, but next time, get in the habit of posting the solution instead of just deleting your post. Had I not been here, a lot of people would have been experiencing the same problem, and having your solution here will save a lot of headaches.