[vJASS] Hero Revive System

[Kithio]

Hello, I was wondering if anyone could give me some insight in how I can make this system better. Right now it feels very wasteful and inefficient as every Player has a unique respawn timer and dialog attached to them. Which means that I have 12 "Timer Expired" events hooked up in the background to revive every player's hero when their respective timer runs out :/

Note: My code makes use of PlayerUtils

My code for the primary scope is as follows

scope HeroRevive

scope HeroRevive initializer onInit
    globals
        timer array HeroReviveTimer
        timerdialog array HeroReviveTimerDialog
    endglobals
   
    private function Conditions takes nothing returns boolean
        return GetTriggerUnit() == PlayerHero[GetPlayerId(GetOwningPlayer(GetTriggerUnit()))]
    endfunction

    private function Actions takes nothing returns nothing
        local real respawnTime = 5.00
        local User p = User(GetPlayerId(GetOwningPlayer(GetTriggerUnit())))
       
        call StartTimerBJ(HeroReviveTimer[p.id], false, respawnTime)
        call CreateTimerDialogBJ(HeroReviveTimer[p.id], PlayerColor[p.id] + p.name + "|r")
        set HeroReviveTimerDialog[p.id] = bj_lastCreatedTimerDialog
       
        call DisplayTimedTextToPlayer(GetOwningPlayer(GetTriggerUnit()), 0.0, 0.0, 2.0, "Your hero will be respawned in |c00DFFB4F" + I2S(R2I(respawnTime)) + "|r seconds.")
        call NormalToAlpha(GetTriggerUnit(), 0.15)
    endfunction

    private function onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        local User p = User.first
       
        loop
            exitwhen p == User.NULL
           
            call TriggerRegisterPlayerUnitEvent(t, p.toPlayer(), EVENT_PLAYER_UNIT_DEATH, null)
           
            set p = p.next
        endloop
       
        call TriggerAddCondition(t, Condition(function Conditions))
        call TriggerAddAction(t, function Actions)
    endfunction
endscope

Code for X Timer Expired is as follows

scope TimerHeroRevive1

scope TimerHeroRevive1 initializer onInit
    private function Actions takes nothing returns nothing
        local location loc = null
        local User p = User(GetPlayerId(0))
   
        call PauseTimer(GetExpiredTimer())
        call DestroyTimerDialog(HeroReviveTimer[p.id])
        set loc = GetStartLocationLoc(GetPlayerStartLocation(p.toPlayer()))
        call ReviveHeroLoc(PlayerHero[p.id], loc, true)
        if (User.fromLocal().id == p.id) then
            call ClearSelection()
            call SelectUnit(PlayerHero[p.id], true)
           
            call PanCameraToTimed(GetLocationX(loc), GetLocationY(loc), 0.50)
        endif
        call RemoveLocation(loc)
        call AlphaToNormal(PlayerHero[p.id], 0.07)
       
        set loc = null
    endfunction

    private function onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterTimerExpireEvent(t, HeroReviveTimer[0])
        call TriggerAddAction(t, function Actions)
    endfunction
endscope

 

[Nordmar]

i once saw a system where no timers were used but the custom value of a unit. Each second the custom value gets reduced by 1 and when it hits 0 the her revives. Now you dont need to use custom values but can store the integer values in an array for example so you don't need multiple timer objects.

 

[Dehua_Darbuya]

Couldn't you call another function instead of using a trigger event to detect when the timer has expired and then revive the hero?

 

[Kithio]

Couldn't you call another function instead of using a trigger event to detect when the timer has expired and then revive the hero?

I'm by no means comfortable with how Jass / vJass works yet as I'm only just getting started with it now after being away for years from warcraft maps in general. That's why I'm looking for whatever information I can get about how to make my code more efficient

 

[MyPad]

Efficiency, you say?

You could jam all of your actions in your condition, like this

Before Sole Conditions: (Pseudocode)

function Cond... returns boolean
    return IsUnitType(GetTriggerUnit(), UNIT_TYPE_HERO) == true
endfunction

function Act..
    // actions here
endfunction

function InitTrig..
    local trigger t = CreateTrigger()
    ...
    call TriggerAddCondition(t, Condition(function Cond))
    call TriggerAddActions(t, function Act)
    set t = null

After Conditions: (Pseudocode)

function Cond... returns boolean
    if IsUnitType(GetTriggerUnit(), UNIT_TYPE_HERO) == true then
        .. do actions
    endif
    return false
endfunction

function InitTrig..
    local trigger t = CreateTrigger()
    ...
    call TriggerAddCondition(t, Condition(function Cond))
    // saved a handle here
    set t = null

EDIT:

I did some optimization with your code, and here's the output

scope HeroRevive initializer onInit

    globals
        timer array HeroReviveTimer
        timerdialog array HeroReviveTimerDialog
    endglobals
 
    private function GetExpiredTimerUser takes timer t returns User
        local User p = User.first
        loop
            exitwhen p == User.NULL or HeroReviveTimer[p.id] == t
            set p = p.next
        endloop
        return p
    endfunction
 
    private function OnCallback takes nothing returns nothing
        local real l_x = 0
        local real l_y = 0
        local timer t = GetExpiredTimer()
        local User p = GetExpiredTimerUser(t)
     
        set t = null
     
        call PauseTimer(t)
        call DestroyTimerDialog(HeroReviveTimerDialog[p.id])
        set l_x = GetStartLocationX(GetPlayerStartLocation(p.toPlayer()))
        set l_y = GetStartLocationY(GetPlayerStartLocation(p.toPlayer()))
        call ReviveHero(PlayerHero[p.id], l_x, l_y, true)
        if (User.fromLocal().id == p.id) then
            call ClearSelection()
            call SelectUnit(PlayerHero[p.id], true)
         
            call PanCameraToTimed(GetLocationX(loc), GetLocationY(loc), 0.50)
        endif
        call AlphaToNormal(PlayerHero[p.id], 0.07)
    endfunction
 
    private function Conditions takes nothing returns nothing
        local real respawnTime
        local User p
        local unit u = GetTriggerUnit()
        if u == PlayerHero[GetPlayerId(GetOwningPlayer(u))] then
            set respawnTime = 5.
            set p = User(GetPlayerId(GetOwningPlayer(u)))
         
            call TimerStart(HeroReviveTimer[p.id], false, respawnTime, function OnCallback)
            set HeroReviveTimerDialog[p.id] = CreateTimerDialog()
            call TimerDialogSetTitle(HeroReviveTimerDialog[p.id], PlayerColor[p.id] + p.name + "|r")
            call TimerDialogDisplay(HeroReviveTimerDialog[p.id], true)
         
            call DisplayTimedTextToPlayer(p.toPlayer(), 0.0, 0.0, 2.0, "Your hero will be respawned in |c00DFFB4F" + I2S(R2I(respawnTime)) + "|r seconds.")
            call NormalToAlpha(u, 0.15)
        endif
        set u = null
    endfunction

    private function onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        local User p = User.first
        loop
            exitwhen p == User.NULL
            call TriggerRegisterPlayerUnitEvent(t, p.toPlayer(), EVENT_PLAYER_UNIT_DEATH, null)
            set p = p.next
        endloop
        call TriggerAddCondition(t, Filter(function Conditions))
    endfunction
endscope

 

[Kithio]

Efficiency, you say?

Your code works perfectly and doesn't have me split the code up into multiple triggers which saves me some structure issues

 

[MyPad]

Wait a second, I spotted an error with the code:

local timer t = GetExpiredTimer()
local User p = GetExpiredTimerUser()

set t = null

// this part is unnecessary, so you can remove it.
call PauseTimer(t)

 

[Serenity09]

i once saw a system where no timers were used but the custom value of a unit. Each second the custom value gets reduced by 1

wait a second...

first off

private function onInit takes nothing returns nothing
        local trigger t = CreateTrigger()
        call TriggerRegisterTimerExpireEvent(t, HeroReviveTimer[0])
        call TriggerAddAction(t, function Actions)
    endfunction

i think your event is only firing when player 0's timer expires. it's fine to throw every player's expiration event onto the same trigger like you're doing there. just loop through your active players to register the event on that same trigger. NOTE that you should still only add the action once, outside the loop, so that it doesn't fire multiple times per event

i like how you've made/imported a User struct that easily lets you iterate it for active players. abstracting that will come in handy on every map you do

in terms of optimization, i see a few things
most importantly: what you're doing isn't that heavy efficiency wise. every time i come to this site, i leave caring about things i shouldn't. i'd say that you shouldn't worry so much about performance until it starts being problematic

that said, academically here are a few things that you could look at:
change your local real respawnTime to a constant global. it'll be a tiny bit faster, mostly from shaving a local variable declaration, and more importantly, it'll be easier to read and configure going forward

using a number of timers equal to the number of players seems a little unnecessary. why do we need to keep track of these timers all separately -- they are all doing the same thing once they expire, and more importantly what they're doing is pretty quick to execute anyways. the only thing you get by having them separate is they keep track of the remaining death time for you. why not just have one timer that expires every little bit. you would associate this timer with the collection of dead heroes, and their remaining respawn time. have it expire every .5 (or so) seconds and on expire iterate the full collection of dead heroes. handle any heroes that finished respawning and just decrement the remaining time on the others. with only 1 timer on a relatively slow timestep i doubt you'd even want to worry about pausing and unpausing it

creating and destroying timers without recycling them can be heavy on a map when done a lot. i can't remember if 'a lot' is hundreds or thousands of timers, but either way you're not in danger there with just this functionality. recycling refers to when you don't destroy the actual object (timer in this instance) but instead put it on a recycled (timer) stack, waiting to be used. There are many timer recycling frameworks out there, TimerUtils is very simple and commonly used

 

[Kithio]

i think your event is only firing when player 0's timer expires.

As stated in the OP, That is just one of the (X) ones as I originally had them all split up into multiple files so every player's expiration had it's own trigger. which is what made it feel super inefficient to begin with and what prompted me to make this thread.

i like how you've made/imported a User struct that easily lets you iterate it for active players. abstracting that will come in handy on every map you do

Also stated in the first post I used PlayerUtils by TriggerHappy with a link to the library if you click on the text in OP

what you're doing isn't that heavy efficiency wise. every time i come to this site, i leave caring about things i shouldn't. i'd say that you shouldn't worry so much about performance until it starts being problematic

I prefer worrying about performance before it even begins to show an issue. I like having a neat and clean structure and will always optimize what I can when I can.

I've been reworking all of my GUI triggers into pure vJass over the last day or so. I just finished them all this morning

change your local real respawnTime to a constant global. it'll be a tiny bit faster, mostly from shaving a local variable declaration, and more importantly, it'll be easier to read and configure going forward

I would make the respawnTime into a global if it was only one set time, But it changes from player to player and is only used in one file at this point so I didn't see the point in having it a global anymore (It was originally)

TimerUtils is very simple and commonly used

I'll look into TimerUtils as there are other things in my code using timers in ways that doesn't feel too efficient the way it's made right now.

I appreciate the insight you've given in your comment though, Even if some of it seems a little redundant as I'm currently using a slightly adjusted version of the one shown in Post #5

 

[MyPad]

why not just have one timer that expires every little bit. you would associate this timer with the collection of dead heroes, and their remaining respawn time. have it expire every .5 (or so) seconds and on expire iterate the full collection of dead heroes.

A timer queue, although a bit painful to code, can technically be the fastest option. (I'm sure that's not what you're talking about)
What I mean to say is this;

Explanation

First instance starts the timer, making it terminate in 5 seconds.

After 0.5 seconds, second instance needs to terminate in 4 seconds, leaving us with (-0.5) seconds to spare.

Thus, we push first instance and make second instance the first instance.

After 4 seconds, we remove the head.. which is the new first instance (formerly second instance). After 0.5 seconds, we pause the timer because we no longer have any instances left to remove.

 

[Serenity09]

As stated in the OP, That is just one of the (X) ones as I originally had them all split up into multiple files so every player's expiration had it's own trigger. which is what made it feel super inefficient to begin with and what prompted me to make this thread.

oh okay, well there you go. solving that was probably the biggest improvement you could make in terms of cutting down on code repetition

Also stated in the first post I used PlayerUtils by TriggerHappy with a link to the library if you click on the text in OP

as stated in my original, i don't care who you got it from, abstracting a player is a good idea. i was trying to get at: why is it a really good idea and can we apply that good idea here (and other places)?

I prefer worrying about performance before it even begins to show an issue. I like having a neat and clean structure and will always optimize what I can when I can.

do you think performance and good structure are the same? sometimes a good structure can lead to performance gains, but prioritizing performance always comes at the cost of code structure if you take it far enough. it's obviously your preference as the lone developer on your own project, but i have a hard time believing people who say their (essentially) minified and barebones code makes more sense to them

I've been reworking all of my GUI triggers into pure vJass over the last day or so. I just finished them all this morning

what a good way to learn vJASS. now that you've done that, when do you think you'd make a GUI trigger instead of a JASS trigger? both options are viable, when compared to eachother for certain tasks

I would make the respawnTime into a global if it was only one set time, But it changes from player to player and is only used in one file at this point so I didn't see the point in having it a global anymore (It was originally)

i see, thanks for simplifying your code a bit for us. what do you think about using structs? you already have abstracted players into User via TriggerHappy's framework, why not abstract your heroes into a Hero struct. to be clear, this isn't a performance gain (it's marginally worse), but it is a much more OOP design (most people view that as "better"). this is sort of an alternative to the DeadHero struct i suggested before, you'd probably want to just have all the 'Dead' functionality be within your Hero struct

I'll look into TimerUtils as there are other things in my code using timers in ways that doesn't feel too efficient the way it's made right now.

i think that's an easy way to improve performance while keeping your code design the same. it's worth remembering that this only effects timers that are created and destroyed over and over. before you pick TimerUtils, it's also worth looking at a few of the alternatives because you'll use the same timer recycling framework for all your timer recycling needs per map. if i remember right, the most popular merging framework is TimerTools and Timer32 is good for when you have a few timers that expire with the minimum timeout

I appreciate the insight you've given in your comment though, Even if some of it seems a little redundant as I'm currently using a slightly adjusted version of the one shown in Post #5

you're welcome, but considering you're asking strangers to take time to understand and help you, you could at the least keep your post up to date

A timer queue, although a bit painful to code, can technically be the fastest option. (I'm sure that's not what you're talking about)

you're right that this would probably be the most efficient approach if he keeps going with 1 timer per death countdown. 'probably' because this isn't enough timers to make a whole merging strategy worth it, but the rest of his map will probably make it a net gain once plugged into the same timer merging framework. still, unless wc3 multithreads timers since last i looked, executing 5 timers to run X code, with merges or not, can't be more efficient than executing 1 timer to run X code. isn't that the whole philosophy behind that framework: merging more timers into less is always better

 

[Kithio]

do you think performance and good structure are the same? sometimes a good structure can lead to performance gains, but prioritizing performance always comes at the cost of code structure if you take it far enough. it's obviously your preference as the lone developer on your own project, but i have a hard time believing people who say their (essentially) minified and barebones code makes more sense to them

I never said anything about Performance and Structure being the same. I always find a way to get a neat-ish structure for any kind of code base even while I'm prioritizing performance.

This statement has nothing to do with the fact that I am current a lone developer on my project. I have had the same ideals in all projects I've had in the past few years which are C# or Java based. I adapt to what is needed while still keeping my own stance on things.

what a good way to learn vJASS. now that you've done that, when do you think you'd make a GUI trigger instead of a JASS trigger?

Honestly, based on the crap that GUI tends to spit out when compiled into Jass... I'd prefer writing all my code myself instead of relying on that. I'm very used to writing all my code from scratch and can somewhat easily make sense of just about any code put infront of me once I get a steady read into what native functions do what and what library functions do what. So writing Jass instead of using GUI seems like a more logical step to me since I get to 100% control the code in each trigger.

you're welcome, but considering you're asking strangers to take time to understand and help you, you could at the least keep your post up to date

I haven't had a lot of time at the computer lately so I never had the chance to update the OP to include this details. I have been doing a little bit of code then walking off to do other things around the place I live every day for the last week or so. I am sorry for not keeping the OP up to date with the details and will try to do so in the future.

However I also think it's up to the person responding to look at some of the comments, at the very least the latest comments, in any given thread, doesn't take a lot of times to just skim through

 

[Serenity09]

I never said anything about Performance and Structure being the same. I always find a way to get a neat-ish structure for any kind of code base even while I'm prioritizing performance.

when you say prioritize performance, i take that to mean you are putting that above other things. i agree that it's a reasonable aim for everyone to get neat-ish code that doesn't run sloppily or excessively, but performance tuning is specifically the process of making something worse at most things so it can be better at a few things. its a game about finding the pros that matter most to you while picking up cons that don't. you don't just optimize a program, you optimize a program for a specific task(s). i think what you're saying though is that you want clean code that runs efficiently, which i think is a great goal for every developer to shoot for during initial development. im just starting a discussion about what optimization really means because i think its very interesting, though ill advised for your current needs

This statement has nothing to do with the fact that I am current a lone developer on my project. I have had the same ideals in all projects I've had in the past few years which are C# or Java based. I adapt to what is needed while still keeping my own stance on things.

i wasn't very clear. trying to argue the strengths and weaknesses of one implementation of a program is a slippery slope. sure there are objective details, like runtime, that make it tempting to assess one implementation as being better, but these differences in whats better are so specific to the context the program is used that they might not matter at all. if two implementations work the same on the outside, then id argue that its more about how the internals work for the development team, whatever their priorities might be

Honestly, based on the crap that GUI tends to spit out when compiled into Jass... I'd prefer writing all my code myself instead of relying on that. I'm very used to writing all my code from scratch and can somewhat easily make sense of just about any code put infront of me once I get a steady read into what native functions do what and what library functions do what. So writing Jass instead of using GUI seems like a more logical step to me since I get to 100% control the code in each trigger.

that's an interesting way to look at it, i can see why the granularity would seem like a big plus. but what if i told you that you don't actually get 100% control over what the compiled JASS looked like. and, even if you did, it's still not a universe away from controlling how that compiled JASS gets read in and executed by the machine. that all said, you'd probably still lean towards JASS because the syntax is similar enough to other languages you know right? i do, for that reason. i'm not arguing anything here, i'm just interested in when JASS or GUI is the better option for someone

However I also think it's up to the person responding to look at some of the comments, at the very least the latest comments, in any given thread, doesn't take a lot of times to just skim through

i agree, but i did skim actually. it seems that you like assuming that i haven't done something just because the end result isn't what you expect. i just didn't read through the code revisions MyPad benevolently supplied, because there's no point till it becomes the code you've adopted. you don't have to update everything, just keep the code in your OP accurate to what you want to discuss