[Wurst] CombatState

[Chaosy]

Somewhat simple creation.
Honestly a fair lot of questionmarks regarding how to use events and whatnot but at least it works.

CombatState.wurst

package CombatState
import DamageDetection
import LinkedList
import HashMap
import RegisterEvents
import TimerUtils
/*
    Simple system to check if a unit has recently been in combat or not.
    public function getTriggerCombatUnit() returns unit
    public function registerOnCombatEnter(code c)
    public function registerOnCombatExit(code c)
    public function unit.isInCombat() returns boolean
*/
@configurable
constant LOOP_SPEED = ANIMATION_PERIOD //Standard Global
constant COMBAT_DURATION = 3
constant FRIENDLY_FIRE = true
@endconfigurable
LinkedList<unit> instances = new LinkedList<unit>()
LinkedList<onCombat> onCombatListeners = new LinkedList<onCombat>()
LinkedList<offCombat> offCombatListeners = new LinkedList<offCombat>()
HashMap<integer, real> time = new HashMap<integer, real>()
HashMap<integer, boolean> state = new HashMap<integer, boolean>()
abstract class onCombat
    abstract function callback(unit u)
abstract class offCombat
    abstract function callback(unit u)
public function registerOnCombatEnter(onCombat c)
    onCombatListeners.push(c)
public function registerOnCombatExit(offCombat c)
    offCombatListeners.push(c)
public function unit.isInCombat() returns boolean
    return state.get(this.getHandleId())
function unit.getTime() returns real
    return time.get(this.getHandleId())
function unit.setTime(real val)
    time.put(this.getHandleId(), val)
function unit.setState(boolean inCombat)
    if(this.isInCombat() != inCombat)
        state.put(this.getHandleId(), inCombat)
function unit.setCombatState(boolean inCombat)
    if(this.isInCombat() != inCombat)
        if(inCombat == false)
            for listener in offCombatListeners
                listener.callback(this)
            cleanup(this)
        else   
            for listener in onCombatListeners
                listener.callback(this)
            instances.add(this)
            this.setTime(0)
    else if inCombat
        this.setTime(0)
    state.put(this.getHandleId(), inCombat)
function unit.isAllyWith(unit u) returns boolean
    return this.getOwner().isAllyOf(u.getOwner())
function onDamage()
    unit target = GetTriggerUnit()
    unit source = GetEventDamageSource()
    if((target.isAllyWith(source) and FRIENDLY_FIRE == true) or target.isAllyWith(source) == false)
        GetTriggerUnit().setCombatState(true)
        GetEventDamageSource().setCombatState(true)
function cleanup(unit u)
    integer h = u.getHandleId()
    state.remove(h)
    time.remove(h)
    instances.remove(u)
function onLoop()
    real time
    for unit u in instances
        time = u.getTime()
        if(time >= COMBAT_DURATION)
            for listener in offCombatListeners
                listener.callback(u)
            cleanup(u)
        else
            u.setTime(time + LOOP_SPEED)
   
function onDeath()
    cleanup(GetTriggerUnit())
init
    registerPlayerUnitEvent(EVENT_PLAYER_UNIT_DEATH, function onDeath)
    enableDamageDetect() // Not sure if needed
    addOnDamageFunc(function onDamage)
    getTimer().startPeriodic(LOOP_SPEED, function onLoop)

Demo

package Hello
import CombatState
import OnUnitEnterLeave
import UnitIndexer
group g = CreateGroup()
function onMapEnter()
    g.addUnit(getEnterLeaveUnit())
function onMapLeave()
    g.removeUnit(getEnterLeaveUnit())
function onLoop()
    for unit u in g
        if(u.getHP() < u.getMaxHP() and u.isInCombat() == false)
            u.setHP(u.getHP() + 1)
            CreateTextTag()
            ..setPos(u.getPos3Fly())
            ..setColor(0, 255, 0, 255)
            ..setLifespan(0.5)
            ..setFadepoint(0.25)
            ..setPermanent(false)
            ..setText("+1", 10)
            ..setVelocity(0, 0.4)
init
    registerOnCombatEnter() u ->
        print(u.getName() + " entered combat!")
    registerOnCombatExit() u ->
        print(u.getName() + " left combat!")
    onUnitIndex(function onMapEnter)
    onUnitDeindex(function onMapLeave)
    CreateTimer().startPeriodic(0.1, function onLoop)

 

[Cokemonkey11]

Random thoughts:

- Don't use InitHashtable directly - use HashMap to save memory
- there is a global constant called `ANIMATION_PERIOD` ~ 0.03
- A lot of constants should be `configurable` but aren't?
- Consider using OnIndex and OnDeindex instead of checking for unit death. (Or even better: provide an API that allows the user to select which one they want)

@Frotty may have further thoughts

 

[Frotty]

The main critical error here tho is in function onLoop().
You are leaking iterators, because the "for from" loop does not close them. Use for in or the static itr variant with from, but not this.

Other than that:

- Check code conventions
- Perhaps replace code as listener with a closure
- Use registerEvents or ClosureEvents for the death listener

if(instances.indexOf(u) != -1)
        instances.remove(u)

The condition seems pointless? Also a slow operation on a LinkedList. You are iterating through the entire list twice here.

Sorry for late replies. Hive seems to have to way of notifying one of threads tagged with wurst.

 

[Chaosy]

Random thoughts:

- Don't use InitHashtable directly - use HashMap to save memory
- there is a global constant called `ANIMATION_PERIOD` ~ 0.03
- A lot of constants should be `configurable` but aren't?
- Consider using OnIndex and OnDeindex instead of checking for unit death. (Or even better: provide an API that allows the user to select which one they want)

1. Done
2. Done
3. Done
4. Looking into it
(doing other changes, wont upload the updates yet)

The main critical error here tho is in function onLoop().
You are leaking iterators, because the "for from" loop does not close them. Use for in or the static itr variant with from, but not this.

Other than that:

- Check code conventions
- Perhaps replace code as listener with a closure
- Use registerEvents or ClosureEvents for the death listener

if(instances.indexOf(u) != -1)
        instances.remove(u)

The condition seems pointless? Also a slow operation on a LinkedList. You are iterating through the entire list twice here.

Can you show how to use a closure that way? as I know how to use a closure API but I cannot make one myself x)

 

[Frotty]

1. Done
Can you show how to use a closure that way? as I know how to use a closure API but I cannot make one myself x)

Replace everywhere you use code with the type of a new abstract class or interface that has one abstract function. The abstract function is the callback you call on the passed closure.
A parameter could be used to pass data, e.g. the combat state.

 

[Chaosy]

Got it, closures are actually mind blowing it feels like magic compared to the jass approaches I have seen.
Thanks, I'll update this soon TM

 

[Cokemonkey11]

> Got it, closures are actually mind blowing it feels like magic compared to the jass approaches I have seen.

Someone's new hive signature text

 

[Chaosy]

Updated.

 

[Cokemonkey11]

Do you have a diff? It's not really practical to read everything from scratch again

 

[Chaosy]

I don't remember honestly.

(doing other changes, wont upload the updates yet)

Though I suppose everything included in this post along with updated callback API, previously I did not use an abstract class for callbacks.

 

[Cokemonkey11]

Ok, I took a skim:

[optional] Your library can provide an API `getUnitsInCombat() -> group` to simplify your demo.

[optional] You could simplify the API to something like:

```
onUnitEnterLeaveCombat(
(u) -> print(u.getName() + " entered combat!"),
(u) -> print(u.getName() + " left combat!")
)
```

LGTM. Will let Frotty give the final approve/decline

 

[Chaosy]

Good suggestions. I'll try to implement them within a day or two.

 

[Cokemonkey11]

@Chaosy did you end up updating this? I think it can be approved without it too

 

[Cokemonkey11]

@Chaosy sorry to ping you again - just wondering if this needs another review (because you made changes) or if I should simply approve as-is