Organic Bomb v1.0.3 [vJASS][UPDATE]

Moderator · Aug 14, 2010

19:01, 16th Aug 2010
The_Reborn_Devil:

The coding looks better now and it's more configurable too ^^

Status: Approved
Rating: Recommended

Inferior · Aug 14, 2010

ok finally a vJass submission

.

Feedback:

1. This:
[jass=Feedback]private function filter takes nothing returns boolean
return (GetWidgetLife(GetFilterUnit())>.415) and (not(IsUnitType(GetFilterUnit(), UNIT_TYPE_STRUCTURE)))
endfunction

method explode takes nothing returns nothing
local group g = CreateGroup()
local boolexpr e = Condition(function filter)
local unit u
call GroupEnumUnitsInRange(g, GetUnitX(.t), GetUnitY(.t), this.radius, e)
call DestroyEffect(AddSpecialEffect(BOMB_EFFECT_STR, GetUnitX(.t), GetUnitY(.t)))
loop
set u = FirstOfGroup(g)
exitwhen u == null
if IsUnitEnemy(u, GetOwningPlayer(.u)) then
call UnitDamageTarget(.u, u, .dmg, false, false, BOMB_A_TYPE, BOMB_D_TYPE, WEAPON_TYPE_WHOKNOWS)
endif
call GroupRemoveUnit(g, u)
endloop
call DestroyBoolExpr(e)
call DestroyGroup(g)
set e = null
set g = null
endmethod[/code]

Can be replaced with this:
[jass=new]
globals
private unit TempUnit
private real TempDamage
private constant group TempGroup = CreateGroup()
endglobals

static method filter takes nothing returns boolean
local unit enum=GetFilterUnit()
if (GetWidgetLife(enum)>.415) and (not(IsUnitType(enum, UNIT_TYPE_STRUCTURE))) and IsUnitEnemy(enum,GetOwningPlayer(TempUnit)) then
call UnitDamageTarget(TempUnit,enum , TempDamage, false, false, BOMB_A_TYPE, BOMB_D_TYPE, WEAPON_TYPE_WHOKNOWS)
set enum=null
return false
endif
set enum=null
return false
endmethod

method explode takes nothing returns nothing
set TempUnit=.u
set TempDamage=.dmg
call GroupEnumUnitsInRange(TempGroup, GetUnitX(.t), GetUnitY(.t), this.radius,Condition(function thistype.filter))
call DestroyEffect(AddSpecialEffect(BOMB_EFFECT_STR, GetUnitX(.t), GetUnitY(.t)))
call GroupClear(TempGroup)
endmethod[/code]

With this you get rid of destroying groups, boolexpr and you avoid the slower FirstOfGroupLoop.

2 .This: GetWidgetLife(AnyUnit)>0.415 should be this GetWidgetLife(AnyUnit)>0.405

If your not sure with it you could use native UnitAlive takes unit id returns boolean

3. Maybe its just the post you made but fix this globals indenting pls, its horrible.

AND:

private constant boolean NEED_COOKIE = true
//Wether to give Prince.Zero a cookie for this

Remove this pls

. has nothing to do with your spell.

That's all i found right now.

Prince.Zero · Aug 15, 2010

just woke up ;p
well ok np
But other than that the spell works fine.

Edit: Update the spell as you asked me too

Fixed the ugly lining... Dunno what happened, it looked ok in the editor.

Mantis_Style · Aug 15, 2010

Good Points

Seems to be Mui and Leakless.

Able to customize.

Sheeps are epic.

Suggestions

Uniqueness
I think there is missing something what makes this spell outstanding.
Adding a tick sound for counting down would be very nice.
And of course I know what sound to use:
Sound --> Interface --> BattleNetTick.wav

Eye Candy
The Special Effect looks really tiny compared to the AoE.
And maybe you should add Effects to the damaged units, too.

Rating

Rating 3/5 [Useful]

Vote for Approval.

If updated, I'll rate again.

Prince.Zero · Aug 15, 2010

i guess i was never good with eye candy ;P

I'll see what i can do my friend.
Thanks for the feedback.

Mantis_Style · Aug 15, 2010

New Rating

I'll give it 4 / 5 now. This may not get a higher rating because it's just too special to be used very often.

watermelon_1234 · Aug 16, 2010

~~I haven't tested this spell yet in-game. If I do, I'll edit this post.~~

Coding Review:

You can use static ifs in checking for BOMB_PAUSE, BOMB_INVULNERABLE, BOMB_SHOW_TIME, etc.
Perhaps allow text size for the text tag to be configurable?

Your filter method could look like this instead:

JASS:

    static method filter takes nothing returns boolean
    local unit enum=GetFilterUnit()
    if (GetWidgetLife(enum)>.405) and (not(IsUnitType(enum, UNIT_TYPE_STRUCTURE))) and IsUnitEnemy(enum,GetOwningPlayer(TempUnit)) then
           call UnitDamageTarget(TempUnit,enum , TempDamage, false, false, BOMB_A_TYPE, BOMB_D_TYPE, WEAPON_TYPE_WHOKNOWS) 
           call DestroyEffect(AddSpecialEffectTarget( BOMB_SECEFFECT_STR, enum, "chest"))
       endif
       set enum=null
       return false
    endmethod

Optional:

You might want to make it easier for people to change which units are affected.
Allow weapontype to be configurable.

You don't need to call GroupClear on TempGroup since no units should actually be stored inside the group. (You're returning false in the filter regardless of what's happening)

Since you're not really doing anything in the Actions function, you could do everything in the Bomb function:

JASS:

private function Bomb takes nothing returns boolean
    if GetSpellAbilityId() == BOMB_ID then
        call boom.create(GetTriggerUnit(), GetSpellTargetUnit())
    endif
    return false
endfunction

Edit:
~~0_0~~
Anyway, the only thing I would suggest is to remove the texttag before it starts showing 0 since that seems kind of strange. That's only my opinion so no need to do so if you disagree. Other than that, the spell looks nice.

Prince.Zero · Aug 16, 2010

performance wasn't an issue, but thanks for the feedback.

BoringEquations · Aug 17, 2010

WOW THIS OUGHT TO BE THE BEST EXPLOSIVE SPELL I'VE SEEN IN A WHILE!!!
i mean look at it! It's like old times in WoW doing ragnaros and getting hit by his spell and knocking everyone off!! This is almost like it, but a ticking bomb?>? SICK

and it's very damn configurable which tempts me to give it a 5/5 for it's awesomeness

Effects and Eye Candy - 5/5
Coding - 5/5
Originality - 5/5
Uniqueness - 5/5

+rep

Atami · Aug 17, 2010

Hi,

i found a Bug. If a unit have more than one timer on itself and the first timer Explode, every other timer explode on the same same at other location. I hope you can fix this...

BTW: the spell looks realy cool! :>

-Atami

Enjoy Mapping...

Prince.Zero · Aug 17, 2010

Atami said:
Hi,

i found a Bug. If a unit have more than one timer on itself and the first timer Explode, every other timer explode on the same same at other location. I hope you can fix this...

BTW: the spell looks realy cool! :>

-Atami

Enjoy Mapping...

meh... a unit isn't supposed to have more than one timer attached

will fix it

edit-

ok fixed it, it seems the damn hot weather makes me not notice things lol
A unit without a corpse doesn't leave coordinates X_x

thanks for feedback!

usfmohy · Aug 18, 2010

That is very useful even if i didnt use it
i can still learn
- how to change the hero color to red
-how to make dynamic text over unit
-knock-back and jump

Atami · Aug 18, 2010

Hi,

Nice that u changed the bug which i found. But now if the targetet unit have more timer on it, all unit around the target fly realy far far far away.
1 Timer= Normal
2 Timer= Normal * 2
3 Timer= Normal * 3

etc.

-atami

Prince.Zero · Aug 18, 2010

Atami said:
Hi,

Nice that u changed the bug which i found. But now if the targetet unit have more timer on it, all unit around the target fly realy far far far away.
1 Timer= Normal
2 Timer= Normal * 2
3 Timer= Normal * 3

etc.

-atami

yea thats normal, its the same in real physics as well ;P

Atami · Aug 19, 2010

Prince.Zero said:
yea thats normal, its the same in real physics as well ;P

Yeah sure thats normal but i think it be better if

X Timer = Normal

I think it look better! :>

Prince.Zero · Aug 22, 2010

maybe i should just adjust it to not be able to cast it again if the unit has a bomb in it.

Atami · Aug 25, 2010

Prince.Zero said:
maybe i should just adjust it to not be able to cast it again if the unit has a bomb in it.

Japp its a alternative...

enjoy! :>

indomitable1319 · Sep 4, 2010

Are those sheeps instead of spheres?! Lol, good spell. 5/5.

jk2pach · Nov 3, 2010

Used in Restricted Complex 601, 5/5

Kam · Jun 30, 2014

Setting private constant boolean SHOW_TIME = false does not remove the text.

Organic Bomb v1.0.3 [vJASS][UPDATE]

Organic Bomb v1.0.3 (Map)

Moderator

Moderator

Similar threads