Listen to a special audio message from Bill Roper to the Hive Workshop community (Bill is a former Vice President of Blizzard Entertainment, Producer, Designer, Musician, Voice Actor) 🔗Click here to hear his message!
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...
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.
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:
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
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...
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
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.
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
This site uses cookies to help personalise content, tailor your experience and to keep you logged in if you register.
By continuing to use this site, you are consenting to our use of cookies.
Approved
