[vJASS] [System] Illusion

Have you a lil updated version which some notes from above by AGD?

JASS:
//Refresh Rate of triggers
private constant real REFRESH_RATE = 120
^what can user exactly expect being refreshed?
The system might just handle it internaly imo, each few minutes, but having it configurable is of course also good.

System seems to work good. I tend to agree that you could consider just taking duration as create() argument, then user can't even try to change it. But it doesn't really matter.
 
Alternatvly I thought about to suggest it might be also okay to or to use instance specific triggers and always destroy them,
or to take advantage of Trigger v1.1.0.2 library. Nes's trigger lib seems really cool, but it has also deep structures and requires a bunch of libs.
In "Trigger" there also exist something like ClearEvents().
But periodicly refreshing is just also a fine method for here imo.

What would could be cool, if I could define absolsut numbers for damage making/taking, instead of "only" relative factors.

852274 is clear what I expect it to be, thouh for readability it could be a constant integer, so it's just 100% clear what order command it is.

Instead of using Table/hashtable UDex could be used, and then instead of onDeath onDeindex could fire. It's a tiny bit better imo.
And if it was imported, then also just a normal array can be used to bind the instance to a unit.

The system doesn't need to recreate it onDestroy if group is empty, but rather if a new unit is added and the timer is paused.
(then also no onDeath trigger operations need to be done before a unit registered actually)

Everything mentioned is just suggestions, or thoughts, and nothing required to be changed.
 

AGD

AGD

Level 14
Joined
Mar 29, 2016
Messages
678
Tried that before, if I recall correctly, that event will only run when DUMMY_OWNER is summoned not the summoner.
JASS:
private static method onInit takes nothing returns nothing
    local trigger t = CreateTrigger()
    set thistype.dummy = CreateUnit(DUMMY_OWNER, DUMMY_ID, 0, 0, 0)
    call TriggerRegisterUnitEvent(t, thistype.dummy, EVENT_UNIT_SUMMON)
endmethod
I tried using a specific unit event and it works.

Also, maybe you could add a SetUnitAnimation(thistype.illu, "stand") in case the illusion is a building so that users won't have to do it themselves because buildings play their birth animation upon being summoned.

And then under the onDamage method, you don't need the local thistype this. thistype.get() is only referred to once.

Instead of periodic event refresh, I think it would be better to use a wasted event counter which increments everytime an illusion dies then refresh the trigger when the counter cap is reached because there're cases where the trigger needs not be refreshed within a certain duration (Like if an illusion is a map is only seldom used). I think a unit specific death event is more efficient than a generic death event.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
private static method onInit takes nothing returns nothing
local trigger t = CreateTrigger()
set thistype.[COLOR=color: #666666]dummy[/COLOR] = CreateUnit(DUMMY_OWNER, DUMMY_ID, 0, 0, 0)
call TriggerRegisterUnitEvent(t, thistype.[COLOR=color: #666666]dummy[/COLOR], EVENT_UNIT_SUMMON)
endmethod
I'll check again.

Also, maybe you could add a
SetUnitAnimation(thistype.[COLOR=color: #666666]illu[/COLOR], "stand")
in case the illusion is a building so that users won't have to do it themselves because buildings play their birth animation upon being summoned.
Ok.

And then under the onDamage method, you don't need the local thistype this.
thistype.[COLOR=color: #666666]get[/COLOR]()
is only referred to once.
Right.

Instead of periodic event refresh, I think it would be better to use a wasted event counter which increments everytime an illusion dies then refresh the trigger when the counter cap is reached because there're cases where the trigger needs not be refreshed within a certain duration (Like if an illusion is a map is only seldom used). I think a unit specific death event is more efficient than a generic death event.
I'm not sure if it's worth doing a specific unit event over a generic event though. Using a generic event would be as simple as
JASS:
if IsUnitInGroup(GetTriggerUnit(), thistype.g) then
    call thistype.get(GetTriggerUnit()).destroy()
endif
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
Updated, I've implemented the event counter instead of the generic event. Other suggestions are applied as well except a few like:

What would could be cool, if I could define absolsut numbers for damage making/taking, instead of "only" relative factors.
This could be done by a DDS.

852274 is clear what I expect it to be, thouh for readability it could be a constant integer, so it's just 100% clear what order command it is.
Forgot to do this, but I've already updated so I'm just gonna leave it at that.

Instead of using Table/hashtable UDex could be used, and then instead of onDeath onDeindex could fire. It's a tiny bit better imo.
And if it was imported, then also just a normal array can be used to bind the instance to a unit.
Not a fan of Unit Indexer. Plus I don't want to force users to import a Unit Indexer.
 

Submission:
[System] Illusion

Date:
5 Feb 2017

Status:
Approved
Note:

System is written good and is definitly useful.

small notes:
  • order id might be put into a variable
  • the if thistype.count >= REFRESH_COUNT then statement could be put in the create function, instead of destroy.. but it doesn't really matter in the end, just a thought concept.
  • the duration method seems not too useful for me, and I would just define the duration just inside create function
  • DisplayTextToPlayer function for debug purpose is in my opinion suboptimal, as it's duration is not very high. I think the default duration, 60 seconds for debug messages is good minimum. (as the user should actually never face a situation where we can't look at a printed debug message anymore, if screen was not cleared, etc)
  • the onDeath function could be somewhere on top, as JassHelper will internaly have to create a copy function of it so it can be properly called, if it's under the caller function in the struct. And the original, actual onDeath function may become not used.
 

AGD

AGD

Level 14
Joined
Mar 29, 2016
Messages
678
@Flux, you forgot to remove this check on onEnter
if GetSummoningUnit() == thistype.dummy then

And this call SetUnitOwner(thistype.dummy, DUMMY_OWNER, false) I think should be directly after the issue illusion order to the dummy since if the issue fails or if illusion is null, it will return 0 before the dummy will be set back to the neutral owner.
 
Level 22
Joined
Feb 6, 2014
Messages
2,468
you forgot to remove this check on onEnter
if GetSummoningUnit() == thistype.dummy then
Of course, how could I miss that :/

And this call SetUnitOwner(thistype.dummy, DUMMY_OWNER, false) I think should be directly after the issue illusion order to the dummy since if the issue fails or if illusion is null, it will return 0 before the dummy will be set back to the neutral owner.
Come to think of it, if that dummy does not return to DUMMY_OWNER, I don't see any game-breaking side effects.

the onDeath function could be somewhere on top, as JassHelper will internaly have to create a copy function of it so it can be properly called, if it's under the caller function in the struct. And the original, actual onDeath function may become not used.
I'll test using plain JASS if that's the case
EDIT: You're right. I thought JassHelper should automatically handle and restructure that for me.
EDIT2: Ugh, it generates trigger evaluations...

EDIT3: Also wasn't expecting this would be approved knowing a dependency is not yet approved
 
Last edited:

MyPad

Spell Reviewer
Level 20
Joined
May 9, 2014
Messages
1,600
Hey, @Flux (Sorry for repost, faulty internet). I found something odd:

JASS:
        private static method onDeath takes nothing returns boolean
            call thistype(thistype.get(GetTriggerUnit())).destroy()
            return false
        endmethod
      
        method destroy takes nothing returns nothing
            if UnitAlive(this.unit) then
                call KillUnit(this.unit)
            endif
            static if LIBRARY_Table then
                call thistype.tb.remove(GetHandleId(this.unit))
            else
                call RemoveSavedInteger(thistype.hash, GetHandleId(this.unit), 0)
            endif
            call GroupRemoveUnit(thistype.g, this.unit)
            //Death trigger refresh
            set thistype.count = thistype.count + 1
            if thistype.count >= REFRESH_COUNT then
                call DestroyTrigger(thistype.deathTrg)
                set thistype.deathTrg = CreateTrigger()
                call TriggerAddCondition(thistype.deathTrg, Condition(function thistype.onDeath))
                call ForGroup(thistype.g, function thistype.reAdd)
                set thistype.count = 0
            endif
            set this.unit = null
            call this.deallocate()
        endmethod

I think that part will generate a trigger-evaluation.
 
Level 37
Joined
Sep 26, 2009
Messages
8,446
Ok this doesn't work at all is what the problem is. The unit != null debug comparison is incorrect - if you check if GetUnitTypeId(this.unit) != 0 you will find that the unit actually does not exist yet, and is why the demo map is completely busted. This could be due to the way Blizzard changed thread sequencing in one of their updates.

JASS:
//TESH.scrollpos=0
//TESH.alwaysfold=0
library Illusion /*
                         Illusion v1.33
                            by Flux
            
            Allows easy creation of Illusion with any damage factor.
    
    */ requires DamageEvent, DamageModify, UnitIndexerGUI /*
    http://www.hiveworkshop.com/threads/damagepackage.287101/
    Required to manipulate damage given and damage taken by illusions.
    
    
    */ optional Table /*
    If not found, the system will create a hashtable. You cannot create more than 256 hashtable
    per map.
    
    ********************************************************************************
    *************************************** API ************************************
    ********************************************************************************
    
    Illusion.create(player, unitSource, x, y)
        - Create an Illusion based on <unitSource>, owned by <player>, positioned at (<x>, <y>) 
        
    this.duration = <timedLife>
        - Add a timer to an illusion.
        - Cannot be overwritten once set.
        
    Illusion.get(unit)
        - Return the 'Illusion instance' based on <unit> parameter.
          
    this.unit
        - Refers to the actual illusion unit
        
    this.damageGiven
        - Determines damage dealt factor.
        
    this.damageTaken
        - Determines damage received factor.  
    
    
    CREDITS:
        Bribe         - Table
        Flux          - DamageEvent and DamageModify
        
*/
    //===================================================================
    //========================= CONFIGURATION ===========================
    //===================================================================
    globals
        //Rawcode of Illusion Ability based on "Item Illusions"
        private constant integer ILLUSION_SPELL = 'AILS'
        
        private constant integer DUMMY_ID = 'dumi'
        
        private constant integer REFRESH_COUNT = 30
        //Dummy unit owner
        private constant player DUMMY_OWNER = Player(PLAYER_NEUTRAL_PASSIVE)
    endglobals
    //===================================================================
    //======================= END CONFIGURATION =========================
    //===================================================================
    
    native UnitAlive takes unit u returns boolean
    
    struct Illusion extends array
        
        public real damageTaken
        public real damageGiven
        
        private static trigger deathTrg = CreateTrigger()
        private static group g = CreateGroup()
        private static timer t = CreateTimer()
        private static integer count = 0
        private static unit dummy
        private static unit illu
        private static thistype illuId
        
        method operator unit takes nothing returns unit
            return GetUnitById(this)
        endmethod
        
        static method get takes unit u returns thistype
            return GetUnitUserData(u)
        endmethod
        
        private static method reAdd takes nothing returns nothing
            call TriggerRegisterUnitEvent(thistype.deathTrg, GetEnumUnit(), EVENT_UNIT_DEATH)
        endmethod
        
        method destroy takes nothing returns nothing
            if UnitAlive(this.unit) then
                call KillUnit(this.unit)
            endif
            call GroupRemoveUnit(thistype.g, this.unit)
            //Death trigger refresh
            set thistype.count = thistype.count + 1
            if thistype.count >= REFRESH_COUNT then
                call DestroyTrigger(thistype.deathTrg)
                set thistype.deathTrg = CreateTrigger()
                call TriggerAddCondition(thistype.deathTrg, Condition(function thistype.onDeath))
                call ForGroup(thistype.g, function thistype.reAdd)
                set thistype.count = 0
            endif
        endmethod
        
        private static method onDeath takes nothing returns boolean
            call thistype(thistype.get(GetTriggerUnit())).destroy()
            return false
        endmethod
        
        private static method onDamage takes nothing returns nothing
            //If source is illusion
            if IsUnitInGroup(Damage.source, thistype.g) then
                set Damage.amount = Damage.amount*thistype.get(Damage.source).damageGiven
            endif
            //If target is illusion
            if IsUnitInGroup(Damage.target, thistype.g) then
                set Damage.amount = Damage.amount*thistype.get(Damage.target).damageTaken
            endif
        endmethod
        
        private static method entered takes nothing returns nothing
            set thistype.illu = GetIndexedUnit()
            set thistype.illuId = udg_UDex
        endmethod
        
        method operator duration= takes real time returns nothing
            call UnitApplyTimedLife(this.unit, 'BTLF', time)
        endmethod
        
        static method create takes player owner, unit source, real x, real y returns thistype
            local thistype this
            set thistype.illu = null
            //Create the Illusion Unit
            if source != null and UnitAlive(source) then
                call SetUnitX(thistype.dummy, GetUnitX(source))
                call SetUnitY(thistype.dummy, GetUnitY(source))
                call SetUnitOwner(thistype.dummy, GetOwningPlayer(source), false)
                if IssueTargetOrderById(thistype.dummy, 852274, source) then
                    if GetUnitTypeId(thistype.illu) != 0 then
                        call SetUnitOwner(thistype.illu, owner, true)
                        if IsUnitType(source, UNIT_TYPE_STRUCTURE) then
                            call SetUnitPosition(thistype.illu, x, y)
                        else
                            call SetUnitX(thistype.illu, x)
                            call SetUnitY(thistype.illu, y)
                        endif
                    else
                        call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, 3600, "[Illusion] No illusion created")
                        call SetUnitOwner(thistype.dummy, DUMMY_OWNER, false)
                        return 0
                    endif
                else
                    debug call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, 3600, "[Illusion] Issued illusion create order failed")
                    call SetUnitOwner(thistype.dummy, DUMMY_OWNER, false)
                    return 0
                endif
                call SetUnitOwner(thistype.dummy, DUMMY_OWNER, false)
            else
                debug call DisplayTimedTextToPlayer(GetLocalPlayer(), 0, 0, 3600, "[Illusion] Source unit dead or non-existing")
                return 0
            endif
            //Initialize struct
            set this = thistype.illuId
            set this.damageTaken = 1.0
            set this.damageGiven = 1.0
            call SetUnitAnimation(thistype.illu, "stand")
            call GroupAddUnit(thistype.g, this.unit)
            call TriggerRegisterUnitEvent(thistype.deathTrg, this.unit, EVENT_UNIT_DEATH)
            set thistype.illu = null
            return this
        endmethod
        
        private static method onInit takes nothing returns nothing
            set thistype.dummy = CreateUnit(DUMMY_OWNER, DUMMY_ID, 0, 0, 0)
            call OnUnitIndex(function thistype.entered)
            call TriggerAddCondition(thistype.deathTrg, Condition(function thistype.onDeath))
            call UnitAddAbility(thistype.dummy, ILLUSION_SPELL)
            call Damage.registerModifier(function thistype.onDamage)
        endmethod
        
    endstruct
    
endlibrary
 

Attachments

  • Illusion v1.33 (1).w3x
    71.9 KB · Views: 30
Top