[1.24 compatible] Firy Stones v1.01

[Moderator]

10:10, 20th Dec 2009
TriggerHappy:

Review for Spell

The only thing stopping me from approving this is the public structs. Please make your structs private. You don't need to null u in Dealdamage. FirstOfGroup Loops are the slowest method of group retrieving AFAIK. It would be better to use ForGroup or use the Filter directly.

Status

Feel free to message me here if you have any issues with my review or if you have updated your resource and want it reviewed again.

Rejected

 

[Deuterium]

Well, although triggering looks decent, I don't see anything special about the idea...
Still, job well-done

 

[cedi]

I said it, its only a small spell for a map. Nothing special... But I think there're comming others soon.

cedi

 

[Cheezeman]

private function OutControl takes nothing returns nothing
    //If you're only using the timer once, there's no need to create a local.
    //GetExpireTimer() doesn't leak. Actually declaring a local out of it
    //will make it leak, so it's better both for processing and 'leakage' if you don't.
    //This is especially important if the timer has a low expire time, like 0.02
    //(By the way, timers has a minimum of 0.035, but that's not important)
    local timer t = GetExpiredTimer()   
    local FiryStone data = GetTimerData( t )
    set t = null
    call data.Main()
endfunction

    //You're also using alot of "unsafe" loops aswell
    //Like this:
        loop
            exitwhen i == MISSILE_COUNT
            if .There[i] == true then
                call .stones[i].Control()
            endif
            set i = i + 1
        endloop
    //It's 'unsafe' because IF (note the if) the CPU
    //miss the exitwhen, you're stuck with an infinite loop
    //which crash the game
    //It's alot safer using
    exitwhen i >= MISSILE_COUNT
    //Because even if the CPU miss the exitwhen,
    //it won't crash the game

    //Other than that, I can't say much more
    //I'm new to structs so there are several things
    //I do not understand, and I can't give you guides
    //on how to improve them.

    //Hope it helped
    //- Cheezeman

 

[cedi]

Fixes done...

 

[Kricz]

Nice spell again cedi
Will use it in "the" map *huuuuust* ^^

 

[Ranger21]

Spell very awesome, i love this spell.

Maybe it is look simple, but it is effects and minor stuff done very good.

GJ

 

[Lambdadelta]

        loop
            exitwhen i == MISSILE_COUNT
            set data.There[i] = true
            set data.stones[i] = Stone.create( data, angle * I2R( i ), i )
            set i = i + 1
        endloop

Don't need I2R.

You don't need to convert integers to real, but you do need to convert reals to integer.
You don't need to set t to null in init trigger, but meh, it doesn't matter.

            if .There[i] == true then

if .There[i] then

Don't need == true. Though that is quite small thing as well.

The spell is done nicely, I like it. Although I don't know why it doesn't damage buildings.... fire burns buildings >.>

 

[NFWar]

After using spell 1st time my PC with duo core CPU 2.33 GH and memory 2 GB laged for ~0.75 seconds. Meaby make it more lagless; meaby any code optimize?
The spell do hight damage. It is like spell cause damage more than one time at period when unit get close to hero.

I know zero of jass coding, even how you people make triggers like codelines...
4/5 cuz of lag

 

[Lambdadelta]

... every spell will most likely lag on first cast... you can always preload it. Deducting points because of something common on a lot of spells is really silly.

 

[Deuterium]

Umm yeah, even preloading seems not to "totally" remove lag.... so as BlackRose said, it's silly to deduct point on that... unless the lag is really terrible, which in this case, it's not.

 

[Cheezeman]

Deducting points because of something common on a lot of spells is really silly.

So if something can be fixed, but the author havn't, it shouldn't be counted?

 

[Deuterium]

First cast lag usually can only be minimized and not fixed...

 

[NFWar]

First cast lag usually can only be minimized and not fixed...

At previos coment i wrote my PC memory and speed. How can you "rate" lag of this spell? Hight? Medium? Low?

EDIT: Deuterium is right. I dont blame Cedi at lag enymore.
My PC do strange things sometimes, like lag at any game or application for 0.5-1 sec
Cedi spell is well done. 5/5 And sry for blaming you Cedi.

 

[Deuterium]

Well, even after preloading the SFX, the same lag still occured.
So it's not his fault.

And it's actually not something to nag about, as the SFX can be changed in the globals... actually the globals are very detailed.

So again I say, there's nothing that can be done about the lag except changing SFX, which can be done by any user. So don't blame the cedi

 

[cedi]

There is one thing you can do:

call KillUnit( CreateUnit( Player( 0 ), DUMMY_ID, 0.00, 0.00, 0.00 ) )

Writh it in the init function, that could decrease the lag a bit. ( Preload the dummy unit )

WARNING! Its free-hand, so may I made a mistake in one of the parameter...

cedi

 

[Gronis93]

Hi

I have downloaded your spell and added it to my map. but for some reason, i do not get the model. everything works, except that you cant see any stones spinning auround your character. help?

Edit: Maked it work. I changed the model art of the dummy to the fire rocks lovely spell thought 5/5