Ice Dance

This bundle is marked as useful / simple. Simplicity is bliss, low effort and/or may contain minor bugs.
  • Like
Reactions: Dark_Dragon
Remade of my old GUI not MUI spell... <3
Now it's MUI and JASS and Leakless. <3
Use it with Pleasure... <3

Credits :
JetFangInferno for his/her AquaSpikev2 model =)
DarkDragon For teaching me something =)

Updates :
22 April 2009 = It's now MUI and Leakless! >=)

Keywords:
Ice, Dance, s4nji, stress, Spell, Director's Cut, Recommended, Creative, Amazing, Number one
Contents

Ice Dance (Map)

Reviews
4 November 2015 Bribe: Rejecting due to the status of this resource being "needs fix" for years. 11:34, 24th Aug 2009 Hvo-busterkomo: Please optimize the code. Switch from locations to coordinates, give the local variables an initial value...

Moderator

M

Moderator

4 November 2015
Bribe: Rejecting due to the status of this resource being "needs fix" for years.

11:34, 24th Aug 2009
Hvo-busterkomo: Please optimize the code. Switch from locations to coordinates, give the local variables an initial value instead of setting them instantly, rework the conditions function, and remove the BJs.
 
Level 15
Joined
Jul 19, 2007
Messages
618
hmm this is one of your first jass spells and its awesome...

well since it is first and i know it is since you converted it from GUI any quickly edited to jass or maybe you just see function names from GUI and used them in jass code since i see you use BJ-s

wll don't worry thats not the problem increasing speed of your spell by some amount of few nanoseconds really does not metter...

what i can see is that you removed every leak gz as well as null handle extended types well done!

you give credits which is nice and as well spell idea is nice


now what i dont like is that you pause caster, used UnitDamagePointLoc since it damages even your units thats why i don't like it as well as buildings and everything... i would suggest you to add group and then add filter which checks is unit ally, building, spell immune...

second thing is:

JASS:
    set Ice[1] = null
    set Ice[2] = null
    set Caster = null
    set Owner = null
    call RemoveUnit (Ice[1])
    call RemoveUnit (Ice[2])

this is part of your ending code well this units are not removed coz they are nulled allready however before that you did:

JASS:
    call UnitApplyTimedLifeBJ( 4.00, 'BTLF', Ice[1] )
    call UnitApplyTimedLifeBJ( 4.00, 'BTLF', Ice[2] )

which ofc fixes the leak since game will remove this units after time but i would suggest you to just remove this last two lines RemoveUnit() or simple remove them before nulling and if you remove them you dont need to apply timed life

spell is MUI but its its hard for GUI-ers to edit try to make it easier

well in init function you leak 16 boolexpr tables but thats blizzard problem of there BJ function if you want to fix that make function which takes nothing and returns true then simple store in one variable local boolexpr bx = Filter(function yourDummyBoolFunc)

and input not null but that var at end don't destroy it but null it... then you will fix things GUI can't well but many people just even don't know about it so if you don't want to leave it as it is.

My rating now would be:

Idea: 8.5/10 -> i would say its cool

Coding: 6.6/10 -> for beginner jass coder its really good, but try to fix some of things i said

Documentation: 4.7/10 -> well you wrote a little comment at end of trigger but you could do it at beginning of trigger so people can easier found it...

Effects: 8/10 -> its simple effect/dummy usage causes no lag delay and such as well looks good but maybe reduce distance between them

Test map: 8.2/10 -> Really good test map

MUI: True -> spell is mui and you get extra points

all in all it deservs 4/5 but plz fix main things i said, best regards!
~Dark Dragon
 
Level 15
Joined
Jul 19, 2007
Messages
618
really nice spell! well you did not need to give me credits but thanks! and what you can improve now is maybe not pause caster and null Exef (effect variable). if you choose to pause caster the maybe it will be good idea to add this line of code after pausing the caster:

JASS:
call SetUnitAnimation(Caster, "spell")

anyway gj all other leaks are fixed and spell is nice. performances are nice much faster then GUI at least...

so 4.5/5 from me and +rep!
 
Level 14
Joined
Nov 18, 2007
Messages
815
No, no, no, no, and no. This fucking isnt a very very good spell. It sucks coding wise, and thats all that matters to me.

  • DestroyGroup()
  • TriggerSleepAction()
  • Global variablesTheyre actually local. Meh.
  • Direct GUI->Jass code leftovers
  • Destroying boolexprs created via Condition() or Filter()
  • GetUnitAbilityLevelSwapped()
  • Using locations
  • pausing units
  • affecting a players camera
  • no maintainability (balancing options) whatsoever, not even a function (or a global) for the spell rawcode
  • integer damage and radius?
  • IsUnitType() needs ==true or ==false
  • GetWidgetLife()>0.405 doesnt exactly tell you if a units alive (just noticed you even use GetUnitState for that, seriously...)
  • GetLastCreatedEffectBJ() doesnt work in response to natives (bug!)
I mean seriously. I havent seen code that bad for a while now.

Personal suggestion: Learn vJass. A _LOT_. Then use it.
 
Level 15
Joined
Jul 19, 2007
Messages
618
Deaod i appreciate your way of thinking.

It sucks coding wise, and thats all that matters to me.

yes you pointed out many things that could be fixed and ofc while i give him some tips i did not wanted to give him to much of staff which is really not so important (in my option)

main thing about his spell was to make it MUI, leakless...

you pointed out few like destroying group, trigger sleep actions BJ-s, widget life...
k first of all destroying group might be the only thing he could fix since blizzard coded it bad but it wont crash the game...

other (trigger sleep) -> np with that its MUI and its random wait rather then constant timer... ofc i as well suggest timers but its easier for him now

Is unit type needs "==" only if you call and only if you call it once and nothing else and only in boolexpr form

JASS:
function bug takes nothing returns boolean
return IsUnitType(GetFilterUnit(), UNIT_TYPE_DEAD)
endfunction

set bx = Condition(function bug)

this will return boolean 64 which is not 0 nor 1 (in fact) true or false but is something... other lets say

GetWidgetLlife is better then IsUnitType(dead) and using both of them is best

IsUnitType(dead) checks is unit dead but if unit dies right now and if its life is (dead life) that function will return false but GetWidgetLife will always return true (not always) not before we find out that we have to check > 0.405 and not > 0

GetLastCreatedEffectBJ() is inlined

using locations (np he nulled and removed them) so np


Direct GUI->Jass code leftovers
really i do not care for that nor BJ-s since spell works and is fast with less time spent on it... you are right ofc that it would be better but its not worth wasting time for nothing...

//-------------------------------------------------
# pausing units
# affecting a players camera
# no maintainability (balancing options) whatsoever, not even a function (or a global) for the spell rawcode

thats deff right and thats what effects spell from not being 5/5 ofc


all i said was my own way of thinking since i know how you think because i stoped thinking like you like few months ago...

i now care for leaks and (true performances of spell) like will code lag or not. too much loops and timers? will it cause lag?... thats what i care about


I am sry i did not want to assault you in any way, you have right on your own thinking as anyone else so i as well just told my own...

if you have any questions feel free to ask!
~Dark Dragon
 
Level 14
Joined
Nov 18, 2007
Messages
815
actually, destroying groups makes it non-leakless.

He should go for a timer, whether hes a beginner or not. Do it right, from the start.

If you didnt notice, hes using IsUnitType in a boolexpr. Without ==true or ==false.

A unit dies instantly, after its life becomes smaller or equal to 0.405. Thus, once its life has reached 0.405, its flagged as dead. And there are no errors with UNIT_TYPE_DEAD to my knowledge.

GetLastCreatedEffectBJ() being inlines doesnt make it any better. Its still a bug.

Actually, i was talking about the first function. Then theres CreateNUnitsAtLoc combined with GetLastCreatedUnit or something like that. If he had been writing the spell in Jass from ground up, he would have chosen natives and more efficient ways to code.

True performance...what a load of bullshit. Do you really think you have the WHOLE VM for your spell? Do you really think there are NO other things going on each frame? If yes, i pity you, if no, stop wasting everyones time and write efficient code, not something half-assed that works, but hogs up too much CPU-time.

Oh, and your way of thinking is superior to mine? I dont think so.

Edit: Theres something i wanted to say about locations...oh, yeah, theyre like 50 times slower than coordinates and not as easy to use.
 
Level 15
Joined
Jul 19, 2007
Messages
618
actually, destroying groups makes it non-leakless.

correct but blizzard coded bad DestroyGroup function thats what i mean

He should go for a timer, whether hes a beginner or not. Do it right, from the start.

he could but that requires some knowledge of timer which some people from wc3c discoverd... like expired timers which are nulled may cause bugs and its too complicated to explain to him currently... but correct as well he could learn timers and i am sure he will...

If you didnt notice, hes using IsUnitType in a boolexpr. Without ==true or ==false.

sry i explained it bad... it must be set to "==" only when its alone example:

JASS:
function nobug takes nothing returns boolean
return IsUnitType(u, UNIT_TYPE_DEAD) or IsUnitEnemy(u, p)
endfunction

function bug takes nothing returns boolean
return IsUnitType(u, UNIT_TYPE_DEAD)
endfunction

well code should explain it...

A unit dies instantly, after its life becomes smaller or equal to 0.405. Thus, once its life has reached 0.405, its flagged as dead. And there are no errors with UNIT_TYPE_DEAD to my knowledge.

correct he dies when life <= 0.405 and UNIT_TYPE_DEAD has no errors it checks is unit dead in way when he becomes corpse -> while playing animation and little time after it will returns false so basically its safe to use both but well widget life is fine as well...

GetLastCreatedEffectBJ() being inlines doesnt make it any better. Its still a bug.

doesnt make it any better? i thought you care for 1 extra function call but k...
its a bug? what is that supposed to mean its a global variable not a bug!
so he stores in local variable that effect and then destroys it after... so no leak nor bug

Actually, i was talking about the first function. Then theres CreateNUnitsAtLoc combined with GetLastCreatedUnit or something like that. If he had been writing the spell in Jass from ground up, he would have chosen natives and more efficient ways to code.

well he started learning jass as i did from GUI... he has not any other programming language knowledge.
i am sure he will become better!

True performance...what a load of bullshit. Do you really think you have the WHOLE VM for your spell? Do you really think there are NO other things going on each frame? If yes, i pity you, if no, stop wasting everyones time and write efficient code, not something half-assed that works, but hogs up too much CPU-time.

well what i meant with "true performances", by that i mean example if spell lags as well as will it lag if i cast some other spell (or this spell two times at same time), will it cause lag? cast it 6-8 times at same time will it lag? if answer is no then spell performances are good. you can as well type /fps in game to check out by yourself k?

i think you understand now that i ofc do care and always care about not using whole VM for spell and ofc with whole map as well.

the problem is i never wasted anyones time i teached him and i by myself write that what you call "efficient code" but his code which you call "unefficient" is faster then your spells which ofc i checked out "/fps" why is that? your spells are more advanced use timers and ofc take cpu time. his spell is at top performances, "could be improved by few MS" and it would not change anything else than making you happy!

half-assed that works, but hogs up too much CPU-time

as i said his spell woks perfectly 100% correct and you can check that out by casting spell yourself u know? you dont need me or anyone else to tell you "you can see that spell is lag-free by casting it yourself 10x times at same time"

do it yourself...

Oh, and your way of thinking is superior to mine? I dont think so.

i really do not know how much you know... maybe you are a genius... i am not i just like math, physics, programming... and such as well i hate history and such. i am good at things i like and bad at things i hate...

even if i like programming there will always be someone who is better then i am but i learn through life and dont care to ask myself every time "hey you are you better then i am?"

Theres something i wanted to say about locations...oh, yeah, theyre like 50 times slower than coordinates and not as easy to use.

correct they are slower ("but ofc not 50x times")
locations are coded in c++ and is a class which ofc is pointer generated in memory and (dynamically) allocated then cleared by RemoveLocation and finally struct in VM is cleared by nulling...

and ofc you are 100% correct when it comes that they are harder to use...


After all i hope i answerd all of your questions as you would like it to be answerd as well if i pissed you i did not ment to as i said before. its just that we think in different way nothing else... i respect your thinking and thinking of people on wc3c. they all think in way like you do and like i did. however i saw it was pointless and talking that to other people was boring and so i quit... thats all i wanted to say nobody is telling you that you are wrong you are correct he could improve performances.

i just said in my option its fine as it is...

if you still did not understand something or have some other questions fell free to ask!
Greets!
~Dark Dragon
 
Level 14
Joined
Nov 18, 2007
Messages
815
Lets start again. Heres a list of things I would fix, if I wanted to get this spell approved.

  • DestroyGroup()
  • TriggerSleepAction()
  • Global variablesTheyre actually local. Meh.
  • Direct GUI->Jass code leftovers
  • Destroying boolexprs created via Condition() or Filter() (bug!)
  • GetUnitAbilityLevelSwapped()
  • Using locations
  • pausing units
  • affecting a players camera
  • no maintainability (balancing options) whatsoever, not even a function (or a global) for the spell rawcode
  • integer damage and radius?
  • IsUnitType() needs ==true or ==false Vex's Optimizer can take care of that.
  • GetWidgetLife()>0.405 doesnt exactly tell you if a units alive (just noticed you even use GetUnitState for that, seriously...)
  • GetLastCreatedEffectBJ() doesnt work in response to natives (bug!)

Why fix those? Simple: You want to submit something for other people to use (and/or learn from). So make sure what you submit is top-notch quality.
 
Top