Vjass Code Problems (Simple)

Squll2

je'ne sais pas
Hey everyone. Begginner Vjasser here, with a very simple code that is playing up on me.

Would anyone care to explain to me why those code does not work at all :( and tell me how I can improve it, and make it faster... cleaner as such?

Thanks in advance!! :)

The simple aim of the spell, currently, is to create lightning from enemy units around the target point to the caster.

Also, with my GroupEnumUnitsInRange, how do I filter it through a condition? :nuts:

JASS:
scope CounterEnergy initializer Init

//Changeable / Alterable functions

private function ID takes nothing returns integer
  return 'A000'
endfunction

private function Unit_ID takes nothing returns integer
  return 'h000'
endfunction

private function Boo takes nothing returns boolexpr
   return null
endfunction

private function Radius takes nothing returns real
   return 350.00
endfunction

//Changeable / Alterable functions End.

private function Cons takes nothing returns boolean
   return GetSpellAbilityId() == ID
endfunction

private function Actions takes nothing returns nothing
//Local Setting
 local unit u     = GetTriggerUnit()
 local unit e
 local unit d
 local group g               = CreateGroup()
 local group g2              = CreateGroup()
 local player p              = GetOwningPlayer(u)
 local integer a             = 0
 local integer b             = 0
 local location l            = GetSpellTargetLoc()
 local lightning array Light
 local real x                = GetLocationX(l)
 local real y                = GetLocationY(l)
 local real X
 local real Y
//Local Setting

 call GroupEnumUnitsInRange(g, x, y, 500.00, null)
 call GroupAddGroup(g, g2)
 
loop

 set e = FirstOfGroup(g2)
 exitwhen e == null
 set X = GetUnitX(e)
 set Y = GetUnitY(e)
 
 set Light[a] = AddLightning( "CLPB", true, X, Y, x, y)
 
 set a = a+1
 call GroupRemoveUnit(g2, e)
 
endloop

 call PolledWait(2.00)
 
loop

 exitwhen a < 0
 
 call DestroyLightning(Light[a])
 
 set a = a-1
 
endloop
 
endfunction

//===========================================================================
private function Init takes nothing returns nothing
    local trigger trig = CreateTrigger(  )
    call TriggerRegisterAnyUnitEventBJ( trig, EVENT_PLAYER_UNIT_SPELL_EFFECT )
    call TriggerAddCondition( trig, Condition( function Cons ) )
    call TriggerAddAction( trig, function Actions )
endfunction

endscope
 

kingkingyyk3

Visitor (Welcome to the Jungle, Baby!)
1) remove your leak.
2)
JASS:
private function ID takes nothing returns integer
  return 'A000'
endfunction

You can make it as constant globals, since variable is faster than calling a function.
3) Create a function like
JASS:
private function Cons takes nothing returns boolean
   return GetSpellAbilityId() == ID
endfunction

to filter units.
 

Exide

I am amazingly focused right now!
Your trigger is quite difficult to read. You should indent it.

Why are these functions?
JASS:

private function ID takes nothing returns integer
  return 'A000'
endfunction

private function Unit_ID takes nothing returns integer
  return 'h000'
endfunction

private function Boo takes nothing returns boolexpr
   return null
endfunction

private function Radius takes nothing returns real
   return 350.00
endfunction



Why are you using two groups?
Remove g2, and use g only.

EDIT: g should be a private global variable.
If you do this, you don't need to 'call DestroyGroup(g)'


JASS:
call GroupEnumUnitsInRange(g, x, y, 500.00, null)

instead of 'null', add a function:

JASS:

function A takes nothing returns boolean
    //return Some Condition Here
endfunction


    //Lots of actions and whatnot..

    call GroupEnumUnitsInRange(g, x, y, 500.00, Filter(function A))
 

Squll2

je'ne sais pas
I was told that GroupAddGroup() and using two groups is a better method then just using one group?

Those functions are there, exide, for easy customastion, so I can add them down below and instead of searching for them I will just change their values there, but in this version of the trigger I havn't used them ><

Thanks for the help exide +rep, kingking, when your talking to a noob, please evaluate what you mean.. I know what i have to do I just don't know where to put it and how.. >.>

Edit: Exide what do you mean Why are these functions? Should they be something else?
 

Exide

I am amazingly focused right now!
Yea, they should be variables.
Calling a function is unnecessary work.
Using private globals you can declare them at the top of your trigger, so it's equally easy to modify the values.

>I was told that GroupAddGroup() and using two groups is a better method then just using one group?
I never heard anything about that. It sounds silly, though.
Check out this thread: http://www.thehelper.net/forums/showthread.php?t=131720
 

Zaraf

New Member
For your purposes, you actually don't even need a group.

I've modified your code to show you what I mean

JASS:
scope CounterEnergy initializer Init
        
        globals
            private constant integer ID      = 'A000'
            private constant integer Unit_ID = 'h000'
            private constant real    Radius  = 350.0
            
            private real Xfilter
            private real Yfilter
            
            private boolexpr LightActions
            private lightning array Light
            
            group ENUM_GROUP = CreateGroup()
        endglobals

private function Cons takes nothing returns boolean
   return GetSpellAbilityId() == ID
endfunction

private function LightActionsAdded takes nothing returns boolean
    local unit f = GetFilterUnit()
    local real x = Xfilter
    local real y = Yfilter
    
    set Light[a] = AddLightning( &quot;CLPB&quot;, true, GetUnitX(f), GetUnitY(f), x, y)

    return false
endfunction

private function DestroyLight takes nothing returns nothing
    local integer a = 0
    
    loop
        exitwhen DestroyLightning(Light[a]) == null
        call DestroyLightning(Light[a])
        set Light[a] = null
        set a = a+1
    endloop
    
    call DestroyTimer(GetExpiredTimer())
endfunction

private function Actions takes nothing returns nothing
    //Local Setting
    local unit u                = GetTriggerUnit()
    local player p              = GetOwningPlayer(u)
    local location l            = GetSpellTargetLoc()
    local real x                = GetLocationX(l)
    local real y                = GetLocationY(l)
    local timer t               = CreateTimer()
    //Local Setting
    
    set Xfilter = x
    set Yfilter = y
    call GroupEnumUnitsInRange(ENUM_GROUP, x, y, 500.00, LightActions)

    call TimerStart(t, 2.00, false, function DestroyLight)

    call RemoveLocation(l)
    set t = null
    set l = null
    set u = null
endfunction

//===========================================================================
private function Init takes nothing returns nothing
    local trigger trig = CreateTrigger()
    call TriggerRegisterAnyUnitEventBJ(trig, EVENT_PLAYER_UNIT_SPELL_EFFECT)
    call TriggerAddCondition(trig, Condition( function Cons))
    call TriggerAddAction(trig, function Actions)
    
    set LightActions = Condition(function LightActionsAdded)
endfunction

endscope


Basically, you can perform your actions in the filter function.

The code turned out to be a bit messy since I was trying to avoid using a struct. If you use a struct, then this works flawlessly. But as it is right now, it's not MUI, and there are other problems. But this is more to illustrate to you how you can completely avoid the group altogether, and make your code run faster and better.

I've explained this no group concept in more detail here:

http://www.thehelper.net/forums/showpost.php?p=1076839&postcount=3

I encourage you to learn this technique, and if you'd like, I can speak with you further on it to better explain, and show you how to rewrite your spell using structs to improve it.
 
General chit-chat
Help Users
  • No one is chatting at the moment.
  • The Helper The Helper:
    what is this Ghan? https://enterprise.ghannet.com/ you are totally holding out on us my friend
  • Ghan Ghan:
    Not mine, I just ran the server. They let the domain expire but it was a cool project so I kept it alive there.
  • Ghan Ghan:
    Used to be enterpriserpg.com I think.
  • The Helper The Helper:
    If you go to the world editor tutorials site and click the starcraft link it takes you there :)
  • Ghan Ghan:
    lol
  • Ghan Ghan:
    I forgot about that alias name. (The full URL still went to the correct site.) That should be fixed.
  • Ghan Ghan:
    Silkroad Forums site really needs help.
  • Ghan Ghan:
    Its software has been out of support since 2017.
  • Ghan Ghan:
    The woes of using custom styles....
  • The Helper The Helper:
    does anyone still use silk road forums?
  • Ghan Ghan:
    It's about as active as here I'd say. Not much going on.
  • Wizard Wizard:
    I think it doesn't help that the silk road game isn't as popular any more I think. The only MMO I play these days is FFXIV like most other people I know.
  • The Helper The Helper:
    I checked a status page for players and it says the game has 500 daily players.
  • The Helper The Helper:
    I cannot believe that site is still going I wonder if Ryoko is involved with it at all would love to talk to him again and see what he is up to
  • The Helper The Helper:
    my oldest daughter failed her driving test today for her license she is 22 and yes I have been driving her everywhere her whole life
  • The Helper The Helper:
    Now they are blaming me on Facebook for her failure LOL I am accepting it because it is my failure I am her driving teacher even though she had certified driving instruction from the best place in town I for some reason cannot teach her how to drive this is frustrating
  • jonas jonas:
    I didn't get my license until I was 25
    +1
  • The Helper The Helper:
    Back in the old days when I was getting a license people were getting them at 16 yrs old and many were getting hardships so they could get them earlier. Almost nobody did not drive when they were able. Nowadays though....
  • Ghan Ghan:
    Why drive when there's Uber Eats?
    +2
  • The Helper The Helper:
    I hope Varine is OK have not heard from him in a while.
    +1
  • Ghan Ghan:
    Diablo 2 Resurrected launching in 9 days.
  • The Helper The Helper:
    Just got a new to me computer hope it will run it
  • jonas jonas:
    I would buy if it had better modding support than D2, like a small lua script engine or much less hardcoded stuff
  • Ghan Ghan:
    Lots of folks are upset that they aren't supporting mods.

    Members online

    No members online now.

    Affiliates

    Hive Workshop NUON Dome
    Top