• 🏆 Texturing Contest #33 is OPEN! Contestants must re-texture a SD unit model found in-game (Warcraft 3 Classic), recreating the unit into a peaceful NPC version. 🔗Click here to enter!
  • It's time for the first HD Modeling Contest of 2024. Join the theme discussion for Hive's HD Modeling Contest #6! Click here to post your idea!

How unoptimized is my Dynamic Tooltip Trigger?

Regno

Regno

Regno

Level 7
Joined
Sep 16, 2016
Messages
185
I do not feel proud at all for making this, how unoptimized is it, I don't think its use-able is it? Everything works but...

The script updates the dynamic tooltip for the players hero, Q W E R T F D without having to use abilcode
Players only have 1 hero, MOBA map, max 9 players. So at most, 9 people can proc this with their hero

JASS: 
// Dynamic setting for abilities
constant string COLOR_GOLD = "|cffffd700"
constant string COLOR_RED = "|cffff3b3b"
constant string COLOR_BLUE = "|cff1e90ff"
constant string COLOR_GREEN = "|cff03cf64"
constant string COLOR_GRAY = "|cffc0c0c0"
constant string COLOR_RESET = "|r"
ability array abil_Q
ability array abil_W
ability array abil_E
ability array abil_R
ability array abil_T
ability array abil_F
ability array abil_D
string array abil_Q_Normal
string array abil_Q_NormalExtended
string array abil_W_Normal
string array abil_W_NormalExtended
string array abil_E_Normal
string array abil_E_NormalExtended
string array abil_R_Normal
string array abil_R_NormalExtended
string array abil_T_Normal
string array abil_T_NormalExtended
string array abil_F_Normal
string array abil_F_NormalExtended
string array abil_D_Normal
string array abil_D_NormalExtended
init
    dynamicUpdateTooltip_init()
function abilityLevel(unit u, ability abil_) returns int
    return GetUnitAbilityLevel(u, BlzGetAbilityId(abil_))
function dynamicTooltip_Cond() returns boolean
    return IsUnitType(GetTriggerUnit(), UNIT_TYPE_HERO)
function dynamicUpdateTooltip_Act() // (Dynamic Tooltip) updates on hero level up, skill learn/upgrade or item gain/drop
    let u = GetTriggerUnit()
    let id = u.getOwner().getId()
    let str = u.getStr(true)
    let agi = u.getAgi(true)
    let intel = u.getInt(true)
    string array abilNormal
    string array abilNormalExt
    ability array abilArray
    ability spell
    for i = 0 to 6
        spell = BlzGetUnitAbilityByIndex(u, i)
        if BlzGetAbilityPosX(BlzGetAbilityId(spell)) == 0 and BlzGetAbilityPosY(BlzGetAbilityId(spell)) == 2 and spell != null // Q
            abil_Q[id] = spell
            abilArray[0] = spell
        if BlzGetAbilityPosX(BlzGetAbilityId(spell)) == 1 and BlzGetAbilityPosY(BlzGetAbilityId(spell)) == 2 and spell != null // W
            abil_W[id] = spell
            abilArray[1] = spell
        if BlzGetAbilityPosX(BlzGetAbilityId(spell)) == 2 and BlzGetAbilityPosY(BlzGetAbilityId(spell)) == 2 and spell != null // E
            abil_E[id] = spell
            abilArray[2] = spell
        if BlzGetAbilityPosX(BlzGetAbilityId(spell)) == 3 and BlzGetAbilityPosY(BlzGetAbilityId(spell)) == 2 and spell != null // R
            abil_R[id] = spell
            abilArray[3] = spell
        if BlzGetAbilityPosX(BlzGetAbilityId(spell)) == 3 and BlzGetAbilityPosY(BlzGetAbilityId(spell)) == 1 and spell != null // T
            abil_T[id] = spell
            abilArray[4] = spell
        if BlzGetAbilityPosX(BlzGetAbilityId(spell)) == 2 and BlzGetAbilityPosY(BlzGetAbilityId(spell)) == 1 and spell != null // F
            abil_F[id] = spell
            abilArray[5] = spell
        if BlzGetAbilityPosX(BlzGetAbilityId(spell)) == 1 and BlzGetAbilityPosY(BlzGetAbilityId(spell)) == 1 and spell != null // D
            abil_D[id] = spell
            abilArray[6] = spell
    // ----------------------------- Variables Config -----------------------------
    // Ability Q is: abil_Q[id]
    // Ability W is: abil_W[id]
    // Ability E is: abil_E[id]
    // Ability R is: abil_R[id]
    // Ability T is: abil_T[id]
    // Ability F is: abil_F[id]
    // Ability D is: abil_D[id]
    // ----------------------------- Variables Config -----------------------------
    // ------------------------------------------------ Hero Ver. 1 ------------------------------------------------
    if u.getTypeId() == 'H049'                                                            // Hero Ver. 1
        let DMG_Q = 100*abilityLevel(u, abil_Q[id])                                       // 100, 200, 300, 400, 500
        let DMG_W = (1+abilityLevel(u, abil_W[id]))*agi                              // 2, 3, 4, 5, 6
        let DMG_E = ((0.5*(abilityLevel(u, abil_E[id])-1)+1) * agi).toInt()           // 1, 1.5, 2, 2.5, 3
        let DMG_R = (9+abilityLevel(u, abil_R[id]))*agi                               // 10, 11, 12
        let DMG_T = 15*agi                                                            // 15
        let DMG_F = (9+abilityLevel(u, abil_F[id]))*agi                              // 10, 11, 12
        let DMG_D = 0                                                                     // 0
        // --------------------------------------------- Ability Translations --------------------------------------------
        abil_Q_Normal[id] = COLOR_BLUE+"Fireball Spitter"+COLOR_RESET
        abil_Q_NormalExtended[id] = COLOR_GOLD+"The Fireball Spitter is a very potent ability, it spits so hard Gajeel wants to fall over and fart.|n|n"+
        "This powerful ability deals ("+COLOR_GREEN+(DMG_Q).toString()+COLOR_GOLD+") damage.|n"+
        "Fireball spit your enemies!"+COLOR_RESET
        abil_W_Normal[id] = COLOR_BLUE+"Ass Smash 1"+COLOR_RESET
        abil_W_NormalExtended[id] = COLOR_GOLD+"The Ass Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+ (DMG_W).toString() +COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_E_Normal[id] = COLOR_BLUE+"Ass Smash 2"+COLOR_RESET
        abil_E_NormalExtended[id] = COLOR_GOLD+"The Ass Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+ (DMG_E).toString() +COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_R_Normal[id] = COLOR_BLUE+"Ass Smash 3"+COLOR_RESET
        abil_R_NormalExtended[id] = COLOR_GOLD+"The Butt Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+ (DMG_R).toString() +COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_T_Normal[id] = COLOR_BLUE+GetAbilityName(BlzGetAbilityId(abil_T[id]))+COLOR_RESET
        abil_T_NormalExtended[id] = COLOR_GOLD+"The Ass Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+(DMG_T).toString()+COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_F_Normal[id] = COLOR_BLUE+"Ass Smash 5"+COLOR_RESET
        abil_F_NormalExtended[id] = COLOR_GOLD+"The Ass Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+(DMG_F).toString()+COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_D_Normal[id] = GetAbilityName(BlzGetAbilityId(abil_D[id]))
        abil_D_NormalExtended[id] = "Normal Extended. This unit has "+str.toString()+" strength and "+intel.toString()+" intelligence, and does Damage: "+DMG_D.toString()
        // --------------------------------------- Ability Translations --------------------------------------
    // ------------------------------------------------ Hero Ver. 2 ------------------------------------------------
    if u.getTypeId() == 'H045'                                                            // Hero Ver. 2
        let DMG_Q = 100*abilityLevel(u, abil_Q[id])                                       // 100, 200, 300, 400, 500
        let DMG_W = (1+abilityLevel(u, abil_W[id]))*intel                              // 2, 3, 4, 5, 6
        let DMG_E = ((0.5*(abilityLevel(u, abil_E[id])-1)+1) * intel).toInt()           // 1, 1.5, 2, 2.5, 3
        let DMG_R = (9+abilityLevel(u, abil_R[id]))*intel                               // 10, 11, 12
        let DMG_T = 15*intel                                                            // 15
        let DMG_F = (9+abilityLevel(u, abil_F[id]))*intel                              // 10, 11, 12
        let DMG_D = 0                                                                     // 0
        // --------------------------------------------- Ability Translations --------------------------------------------
        abil_Q_Normal[id] = COLOR_BLUE+"Fireball Spitter"+COLOR_RESET
        abil_Q_NormalExtended[id] = COLOR_GOLD+"The Fireball Spitter is a very potent ability, it spits so hard Gajeel wants to fall over and fart.|n|n"+
        "This powerful ability deals ("+COLOR_GREEN+(DMG_Q).toString()+COLOR_GOLD+") damage.|n"+
        "Fireball spit your enemies!"+COLOR_RESET
        abil_W_Normal[id] = COLOR_BLUE+"Ass Smash 1"+COLOR_RESET
        abil_W_NormalExtended[id] = COLOR_GOLD+"The Ass Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+ (DMG_W).toString() +COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_E_Normal[id] = COLOR_BLUE+"Ass Smash 2"+COLOR_RESET
        abil_E_NormalExtended[id] = COLOR_GOLD+"The Ass Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+ (DMG_E).toString() +COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_R_Normal[id] = COLOR_BLUE+"Ass Smash 3"+COLOR_RESET
        abil_R_NormalExtended[id] = COLOR_GOLD+"The Butt Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+ (DMG_R).toString() +COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_T_Normal[id] = COLOR_BLUE+GetAbilityName(BlzGetAbilityId(abil_T[id]))+COLOR_RESET
        abil_T_NormalExtended[id] = COLOR_GOLD+"The Ass Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+(DMG_T).toString()+COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_F_Normal[id] = COLOR_BLUE+"Ass Smash 5"+COLOR_RESET
        abil_F_NormalExtended[id] = COLOR_GOLD+"The Ass Smash has such destructive force, ones ass cease to exist when hit by this.|n|n"+
        "Ass Smash deals ("+COLOR_GREEN+(DMG_F).toString()+COLOR_GOLD+") damage.|n"+
        "Smash any enemies you come across!"+COLOR_RESET
        abil_D_Normal[id] = GetAbilityName(BlzGetAbilityId(abil_D[id]))
        abil_D_NormalExtended[id] = "Normal Extended. This unit has "+str.toString()+" strength and "+intel.toString()+" intelligence, and does Damage: "+DMG_D.toString()
        // --------------------------------------- Ability Translations --------------------------------------
    // ------------ Update translations ------------
    abilNormal[0] = abil_Q_Normal[id]
    abilNormalExt[0] = abil_Q_NormalExtended[id]
    abilNormal[1] = abil_W_Normal[id]
    abilNormalExt[1] = abil_W_NormalExtended[id]
    abilNormal[2] = abil_E_Normal[id]
    abilNormalExt[2] = abil_E_NormalExtended[id]
    abilNormal[3] = abil_R_Normal[id]
    abilNormalExt[3] = abil_R_NormalExtended[id]
    abilNormal[4] = abil_T_Normal[id]
    abilNormalExt[4] = abil_T_NormalExtended[id]
    abilNormal[5] = abil_F_Normal[id]
    abilNormalExt[5] = abil_F_NormalExtended[id]
    abilNormal[6] = abil_D_Normal[id]
    abilNormalExt[6] = abil_D_NormalExtended[id]
    // ------------ Update translations ------------
    // ------------------------------------ Apply Translations ------------------------------------
    for i = 0 to 6
        if abilArray[i] != null and abilNormal[i] != null and abilNormalExt[i] != null
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL, 0, abilNormal[i])
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL, 1, abilNormal[i])
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL, 2, abilNormal[i])
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL, 3, abilNormal[i])
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL, 4, abilNormal[i])
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL_EXTENDED, 0, abilNormalExt[i])
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL_EXTENDED, 1, abilNormalExt[i])
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL_EXTENDED, 2, abilNormalExt[i])
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL_EXTENDED, 3, abilNormalExt[i])
            BlzSetAbilityStringLevelField(abilArray[i], ABILITY_SLF_TOOLTIP_NORMAL_EXTENDED, 4, abilNormalExt[i])
    // ------------------------------------ Apply Translations ------------------------------------
function dynamicUpdateTooltip_init()
    CreateTrigger()
    ..registerAnyUnitEvent(EVENT_PLAYER_HERO_LEVEL)
    ..registerAnyUnitEvent(EVENT_PLAYER_UNIT_PICKUP_ITEM)
    ..registerAnyUnitEvent(EVENT_PLAYER_UNIT_DROP_ITEM)
    ..registerAnyUnitEvent(EVENT_PLAYER_HERO_SKILL)
    ..registerAnyUnitEvent(EVENT_PLAYER_UNIT_SPELL_CAST)
    ..addCondition(Condition (function dynamicTooltip_Cond))
    ..addAction(function dynamicUpdateTooltip_Act)
 

Nichilus

Nichilus

Nichilus

Level 25
Joined
Sep 26, 2009
Messages
2,379
It may work in your map at the moment, but it may very well cause issues in the future when you add new spells.
As for what I think about the script, I would say it's suboptimal at best, breaking at worst.

It looks like the script is neither vjass nor lua, so I cannot say whether you could write this script better using some functionality offered by the chosen language, but I can say some things about your code approach and some potential issues I see:

1. The entire script is correlating abilities, units, abilities' descriptions and abilities' damage calculations using a very weak relation: assumption
  • Basically, for hero 'H049' you assume that the ability at [0,2] is Fireball Spitter and you assume that the damage is calculated the way you have it there.
2. The script assumes that the only dynamic thing is damage calculation. How will it work if something else can also change? For example you would add an ability that has longer duration based on unit's INT, etc. This script will tie your hands for no reason

3. You are querying unit's abilities using "spell = BlzGetUnitAbilityByIndex(u, i)" and you assume that the index range 0..6 will be enough.
  • this is potentially breaking for your script
  • While the list includes some abilities by default, any new ability is pushed to the start of the list, to index 0
  • Abilities on this list include abilities like 'Attack' and 'Move', learned hero abilities, non hero abilities but also buffs
    • For example, in a fresh new map the list of abilities for Paladin will look like this:
      Code: 
      0 = Inventory
1 = Attack
...
    • and after the Paladin learns Devotion Aura the list looks like this:
      Code: 
      0 = Devotion Aura buff (BHad)
1 = Devotion Aura ability (AHad)
2 = Inventory
3 = Attack
...
  • Same thing happens if you add unit ability to the hero midgame
    • for example to give hero Evasion via buff, you would have dummy buff + give the hero a hidden Evasion ability for the duration of the buff
  • That means the list of hero abilities may be pushed outside the range 0..6, so this script would not update all descriptions.

---
I say you should decouple the ability text from the [x,y] ability position, decouple it from a specific unit-type, adhere to single responsibility principle and keep a single source of truth:
  • Ability description is related only to a specific ability, not to the ability's position, nor the hero having that ability. Keep it that way.
  • Keep a single place where you calculate ability's damage and other parameters: in the ability itself (or its script). If needed, encapsulate the damage calculation in a function and call that function when executing the ability and when updating its description.
  • If needed, create a function for each ability that would return a formatted description
    • this will free your hands if different abilities have different dynamic values than just damage
    • this function could be called from the script you have
  • Build a list of hero abilities for each hero - this can be done dynamically during game by reacting to 'Hero learns a skill' event and storing the ability code of (Learned hero ability)
    • Use this list to update descriptions instead of ability indexes that change throughout the game
  • I am not sure what you want to achieve with events like 'EVENT_PLAYER_UNIT_CAST' in terms of updating descriptions, but perhaps you could have two versions of those triggers:
    • One version targets only a single ability (for example when you learn/level up the spell)
    • Second version would update all abilities (for example on hero level up)
  • Unless you have huge amount of items and abilities that affect hero's stats (and thus require description update), I think you should use conditions to avoid unnecessary operations
 

Uncle

Uncle

Uncle

Warcraft Moderator
Level 64
Joined
Aug 10, 2018
Messages
6,557
What Nichilus said.

One simple change would be instead of checking multiple Unit-types in dynamicUpdateTooltip_Act() you could run a specific function for that unit-type:
vJASS: 
// Call a function dedicated to this hero unit-type:
if abilHero[u.getTypeId] != null
    abilHero[u.getTypeId]()
The function that gets called will contain all of the unit-type specific data like damage values and strings. That way you can define the settings for these different heroes outside of dynamicUpdateTooltip_Act().

Then you can adjust your Condition to prevent unregistered Heroes from using the system:
vJASS: 
function dynamicTooltip_Cond() returns boolean
    return abilHero[GetUnitTypeId(GetTriggerUnit())] != null
 
Last edited:

Regno

Regno

Regno

Level 7
Joined
Sep 16, 2016
Messages
185
Nichilus said:
It may work in your map at the moment, but it may very well cause issues in the future when you add new spells.
As for what I think about the script, I would say it's suboptimal at best, breaking at worst.

It looks like the script is neither vjass nor lua, so I cannot say whether you could write this script better using some functionality offered by the chosen language, but I can say some things about your code approach and some potential issues I see:

1. The entire script is correlating abilities, units, abilities' descriptions and abilities' damage calculations using a very weak relation: assumption
  • Basically, for hero 'H049' you assume that the ability at [0,2] is Fireball Spitter and you assume that the damage is calculated the way you have it there.
2. The script assumes that the only dynamic thing is damage calculation. How will it work if something else can also change? For example you would add an ability that has longer duration based on unit's INT, etc. This script will tie your hands for no reason

3. You are querying unit's abilities using "spell = BlzGetUnitAbilityByIndex(u, i)" and you assume that the index range 0..6 will be enough.
  • this is potentially breaking for your script
  • While the list includes some abilities by default, any new ability is pushed to the start of the list, to index 0
  • Abilities on this list include abilities like 'Attack' and 'Move', learned hero abilities, non hero abilities but also buffs
    • For example, in a fresh new map the list of abilities for Paladin will look like this:
      Code: 
      0 = Inventory
1 = Attack
...
    • and after the Paladin learns Devotion Aura the list looks like this:
      Code: 
      0 = Devotion Aura buff (BHad)
1 = Devotion Aura ability (AHad)
2 = Inventory
3 = Attack
...
  • Same thing happens if you add unit ability to the hero midgame
    • for example to give hero Evasion via buff, you would have dummy buff + give the hero a hidden Evasion ability for the duration of the buff
  • That means the list of hero abilities may be pushed outside the range 0..6, so this script would not update all descriptions.

---
I say you should decouple the ability text from the [x,y] ability position, decouple it from a specific unit-type, adhere to single responsibility principle and keep a single source of truth:
  • Ability description is related only to a specific ability, not to the ability's position, nor the hero having that ability. Keep it that way.
  • Keep a single place where you calculate ability's damage and other parameters: in the ability itself (or its script). If needed, encapsulate the damage calculation in a function and call that function when executing the ability and when updating its description.
  • If needed, create a function for each ability that would return a formatted description
    • this will free your hands if different abilities have different dynamic values than just damage
    • this function could be called from the script you have
  • Build a list of hero abilities for each hero - this can be done dynamically during game by reacting to 'Hero learns a skill' event and storing the ability code of (Learned hero ability)
    • Use this list to update descriptions instead of ability indexes that change throughout the game
  • I am not sure what you want to achieve with events like 'EVENT_PLAYER_UNIT_CAST' in terms of updating descriptions, but perhaps you could have two versions of those triggers:
    • One version targets only a single ability (for example when you learn/level up the spell)
    • Second version would update all abilities (for example on hero level up)
  • Unless you have huge amount of items and abilities that affect hero's stats (and thus require description update), I think you should use conditions to avoid unnecessary operations
Click to expand...

I am not entirely sure what kind of answer I was expecting but I am extremely grateful, I tried a "lazy" approach of not having to use abilcode for every hero, and you're also right that there is a possibility it'll get pushed out of the 0-6 range. I will take what you said into account and change it up a bit.

The script heavily relies on assumption, which could make maintaining this a bother in the future when it starts to give issues. I just tried to find a way of not having to refer to every single ability using abilcode, but this "Learned hero ability" method could actually be very ingenious. The reason is that there is more than 700 abilities, and having to refer to each and every one with their abilcode, especially when they aren't ordered in Q W E R T F D format would make it a bit annoying 😓

Uncle said:
What Nichilus said.

One simple change would be instead of checking multiple Unit-types in dynamicUpdateTooltip_Act() you could run a specific function for that unit-type:
Lua: 
// Call a function dedicated to this hero:
if abilHero[u.getTypeId] != null
    abilHero[u.getTypeId]()
The function that gets called will contain all of the unit-type specific data like damage values and strings. That way you can define the settings for these different heroes outside of dynamicUpdateTooltip_Act().

Then you can adjust your Condition to prevent unregistered Heroes from using the system:
Lua: 
function dynamicTooltip_Cond() returns boolean
    return abilHero[GetUnitTypeId(GetTriggerUnit())] != null
Click to expand...

Hello Uncle, I am not sure why I didn't think of that xD, preventing unregistered heroes is definitely something I want, thank you. :grin:
 
You must log in or register to reply here.

Similar threads

shadowvzs
  • Locked
No ideea why stop/ignored the trigger if no any known mistake
Replies
3
Views
736
shadowvzs
shadowvzs
Nightmare Moon
  • Locked
EGUI problem
Replies
5
Views
836
Sevion
Sevion
Frotty
[Wurst] Ability Tooltip Generator
Replies
17
Views
15K
Overkane
Overkane
ToolorDie
  • Locked
[Solved] Bpower Equipment System (The unapproved one)
Replies
0
Views
2K
ToolorDie
ToolorDie
Maker
  • Locked
[JASS] My 1 armor = 1 damage reduction system
Replies
3
Views
825
Bribe
Bribe
Top