System In Combat Status

[Prometheus]

I wouldn't compeltely trust the syntax checker, I've had infinite loops and it misses them. =(

 

[PurgeandFire]

That's odd, maybe CSData was already placed before the library. I don't know. @Kc: It isn't supposed to check for infinite loops. It just checks for errors in the syntax.

Anyway, I need to take a huge crap at the moment so I'll be back later to go over the code.

 

[Prometheus]

o.o
Okay.....

 

[Darthfett]

That's odd, maybe CSData was already placed before the library. I don't know.

Well the trigger was already in the map, and placed before the other trigger, so I guess it could be. I guess it makes sense to place all the functions in order first.

I'll be back later to go over the code.

I'm just posting this to make it a bit easier for you to go over. The conditions are a bit confusing. Here's the way the trigger works:

It runs whenever a unit is attacked or a unit starts the effect of an ability.

The conditions go through the units, and checking them against the booleans in the first global functions.

If the trigger was fired by a unit casting a unit target spell, it will check if they are allies, and if they are, it returns whether allied casts will make units in combat.

If it was an attack that fired the trigger, it checks if the owners are allies, and if they are, returns whether allied attacks will make units in combat.

Then, if it was an instant cast spell, or the caster/attacker and target are enemies, it will return true.
--Conditions are over--

Then the Actions part of the trigger runs the SetUnitInCombat function (which does the bulk of the trigger)

This is the part that I think might be a little screwed up...
It checks to see if there is already a timer running on the unit, make sure that if the unit is already in combat, it is just restarting the timer.

The SetUnitCombatState runs when the timer expires, and sets the attached combat state to false (out of combat) It then destroys the timer and nulls it (nulling it so that it's not trying to reuse a destroyed timer)

I have only actually tested this 2-3 times or so.

 

[Darthfett]

-BUMP-

System, testmap, information has been updated with a newer, more efficient vJASS version. It only uses CSData.

 

[Tukki]

Now this is what I'm talking 'bout, simplicity and user-friendly! This will be really useful when making different types of maps

Glad you also learned vJass, good job :thup:

Will comment on the code when I've read it

EDIT: Okey code-commenting time ...

1) I see you are using 1 timer per struct (unit). This not a big deal but could be swapped to using index arrays and a single timer looping through a main "loop" function, lowering the unit's "combat status duration".
2) I've not tested the attached map, but how will your system handle multiple tries to set a unit "In Combat"? (I'm a little lost with timers )

Else it looks fine + Rep

 

[Darthfett]

1) I see you are using 1 timer per struct (unit). This not a big deal but could be swapped to using index arrays and a single timer looping through a main "loop" function, lowering the unit's "combat status duration".

That kind of defeats the purpose of the timer. If I used a global timer, it would either have to be a very fast timer and looping through a lot of units, or it would be an inaccurate timer.

2) I've not tested the attached map, but how will your system handle multiple tries to set a unit "In Combat"? (I'm a little lost with timers )

I've done a little debugging on timers before, and it helped me find a few things out.

If you start a timer that is already started, it will simply restart the timer. Additionally, if you pause a timer, and try to unpause it, it will not run the function when it expires. You have to start it again using the TimerStart function.

So, I made the function reuse the timer from when a unit enters into combat, so that it will restart the duration. That way, each time the unit attacks or casts a spell, the timer is reset, until the unit goes for 4 seconds without attacking, being attacked, casting a spell, or being cast upon.

 

[Romek]

set this.u = null
set this.t = null

Struct members don't need to be nulled, they're globals.

Also, I think the "Conditions" function should go at the top, so it's easier to find and edit.

Overall though, this is really, really useful.
Good Job. +Rep =]

 

[Tukki]

That kind of defeats the purpose of the timer. If I used a global timer, it would either have to be a very fast timer and looping through a lot of units, or it would be an inaccurate timer

Yeah, it would need to loop through several units instead --> but you could do more things in between. And no, it shouldn't be an inaccurate timer -- unless you make some 3 sec interval with it..

if you pause a timer, and try to unpause it,

Good to know.

 

[Darthfett]

Yeah, it would need to loop through several units instead --> but you could do more things in between. And no, it shouldn't be an inaccurate timer -- unless you make some 3 sec interval with it..

Multiple units would need to be a fast interval timer, since it would have to set the duration for how many times the timer has run. Then, it would be doing a loop each time the timer expired, through all the possible units that could have a struct attached to it (which would probably be an array of 8190 units, or a group, which would involve copying the group each callback, removing all the units from it, and destroying the group). The way I did it is pretty simple, because the timer only goes off once, and it lasts for four seconds. very accurate, no looping, no lag.

Struct members don't need to be nulled, they're globals.

Also, I think the "Conditions" function should go at the top, so it's easier to find and edit.

Fixed.

 

[Tukki]

It's probably fine with multiple timers but;

through all the possible units that could have a struct attached to it

Yeah, and that timer would only need to be executed every 0.1 second, then it checks if "duration" is >= MaxDuration, if it is then destroy else add + 0.1 to duration. And it would only need to loop until "UnitsInCombat" is reached, not 8190 times ^^

 

[Ghost-X]

Question I dont understand how this system works, i copied the cs data and the incombat file, do i have to do anything more?

 

[Darthfett]

Question I dont understand how this system works, i copied the cs data and the incombat file, do i have to do anything more?

As long as you have both in your map in libraries, with Newgen installed, everything should work just fine.

Now, simply call "GetUnitCombatState" with a unit to determine whether or not a unit is in combat (it will return true if the unit is in combat, or false if it is not).

If you want to know how the system works:

The system attaches a struct to a unit when it is attacked, targeted by a spell, or casts a spell. This marks the unit as "in-combat". Then, it starts a 4 second timer (you can change the duration in the configuration), which will remove the attached struct when it expires.

If the unit already has the struct attached to it, the 4 second timer starts over.

When you ask the function "GetUnitCombatState" if the unit is in combat, it returns "true" if there is an attached struct, and "false" if there is no attached struct.

 

[Darthfett]

I'll bump this with an update:

The system is now safer for removed units, and uses both CSData and PUI.

The system now allows you to attach multiple structs to units, instead of monopolizing CSData on units.

 

[Flare]

Decided to get around to looking at this

1) Requiring CSData -AND- PUI? Not really sure why CSData is needed, a single timer on a 1 sec interval (or even 0.5s interval, if you're worried about accuracy, even though the greatest discrepancy in actual time spent flagged in combat is TIMER_INTERVAL/2, and is anyone going to notice a half or quarter of a second?) with a group (which could be used for your InCombat condition as well) e.g.

function GetUnitInCombat takes unit u returns boolean
  return IsUnitInGroup (UnitsInCombat, u)
//or
  return CombatState<u> != 0
endfunction

function GroupCallback takes nothing returns nothing
  local CombatState a = CombatState[GetEnumUnit ()]
  set a.combatdur = a.combatdur + TIMER_INTERVAL
  if a.combatdur &gt;= COMBAT_DURATION then
    call a.release ()
    call GroupRemoveUnit (UnitsInCombat, a.u)
    if IsGroupEmpty (UnitsInCombat) then
      call PauseTimer (GlobalTimer)
    endif
  endif
endfunction

function TimerFunc takes nothing returns nothing
  call ForGroup (UnitsInCombat, function GroupCallback)
endfunction

function SetUnitInCombat takes unit u returns nothing
  local CombatState a = CombatState<u>
  if a == 0 then
    set a = CombatState.create (...)
    if IsGroupEmpty (GlobalTimer) then
      call TimerStart (GlobalTimer, TIMER_INTERVAL, true, function TimerFunc)
    endif
    call GroupAddUnit (UnitsInCombat, u)
//rest of the add-to-system stuff
  else
//Reset the combat duration
    set a.combatdur = 0
endfunction</u></u>

But, that's just me having a dislike for importing stuff when it can be done otherwise

2)

    local CombatState cs = CombatState<u>
    return cs != 0</u>

Why not inline that to

return CombatState<u> != 0</u>

Seems a bit pointless declaring a variable when you're only calling the method operator once

3) Not a big deal, but I'd still like to point it out

call SetCSData(this.t,0)

There's no need to set the data to 0 when you're done - CSData doesn't require data to be flushed/released/whatever, so it's just extra stuff that isn't required

4) There doesn't appear to be any discrimination between 'friendly' and hostile spells, from the looks of things. If that's true, it'd be great if this only responded to damage (since a unit shouldn't be flagged for combat if they cast a healing spell on a friendly unit that's also out of combat ), and if spells were cast on a unit in combat (such as a healing spell), the caster would be flagged for combat, but only if the target was flagged for combat

 

[Darthfett]

@Flare

Thanks a lot for the constructive criticism!

1) I never thought about using a global timer and a group to contain the units. I'll experiment, and see what I come up with. (Maybe some new features? ) (The only real reason I'm considering the group, is because of your fourth point. If I have multiple methods of putting a unit in-combat, I can't use TriggerSleepAction instead of -sometimes- inaccurate timers. )

2) This is actually some old functionality I forgot to change (when the struct was actually attached to all units, and a boolean told whether or not a unit was in combat). I'll inline it in the next version, thanks!

3) Another remnant of an old way of doing things. Originally, I was using CSData to attach the struct to the unit and the timer. I was having problems with units still being 'In Combat' (because I didn't clear the attached unit data), and figured out it was because I wasn't clearing the data.

I didn't figure out until later that I didn't actually need to clear the timer data.

4) I designed it with spells so that units buffing other units that are in-combat would put them in combat as well. However, I think I'm going to redesign it so any sort of damage puts a unit in-combat, and any cast on a friendly unit (that is in-combat) puts a unit in-combat.

even though the greatest discrepancy in actual time spent flagged in combat is TIMER_INTERVAL/2

Actually, it would be anything less than the TIMER_INTERVAL, since a unit could technically enter combat immediately before the Timer loop starts incrementing unit's time spent in combat. Then, the timer loop would increment the unit's time by the full TIMER_INTERVAL.

 

[Flare]

Maybe some new features?

New features? I don't really see why new features would be needed. It's a system which detects combat, it does that (albeit slightly flawed at the moment, but that's rectifiable), it doesn't need to be able to make me breakfast (although that would be great ). It's a simple concept, but it's the kind of simplicity I like - it does one particular thing, but it does it right (or, at least, it will, if/when you bring in damage events), does it really need anything more than that?

Actually, it would be anything less than the TIMER_INTERVAL, since a unit could technically enter combat immediately before the Timer loop starts incrementing unit's time spent in combat. Then, the timer loop would increment the unit's time by the full TIMER_INTERVAL.

Bleh, wasn't thinking straight Not sure where I got that idea. Either way, 0.5s is safe enough I would imagine (if a player has an issue with half a second, they're a bit of a nerd to be honest )

But nice system overall (only particular flaws would be the healing issue and UNIT_ATTACKED, which would be particularly breakable with Artillery units), I see no reason why it shouldn't be approved once that is sorted out, and it's seen to be bug-free

EDIT: Nothing to do with the code (well, not really), but

3. Rename it "InCombat"

I don't think you need to rename the trigger to the library's name, the initializer should work regardless of what the trigger name is (at least it does for me )

 

[Darthfett]

Graveyarding this because it uses CSData (incompatible with newest patch), and because it is easily obtainable using AIDS and a timer of some sort.

 

[Narks]

Ever gonna fix it?

 

[Darthfett]

Ever gonna fix it?

It could be easily done with an AIDS struct and a timer system, I don't see much reason to fix it.

If there is demand, I will consider it.