For character abilities that have a certain trigger condition, eg. “OnAttack”, “OnJump”, “OnDamaged” etc…

Currently each of these triggers is a signal. When a signal fires, the character loops through all of its abilities and activates each one with that specific condition, so it just runs an if statement for every ability, regardless of whether it has that condition or not:

if ability.trigger_condition == Triggers.OnAttack:
  ability.activate()

My issue is that this could get a little unscalable with many characters on-screen each having many abilities of their own. A character could have 1 OnDamaged ability and 19 OnAttack abilities, but when an “OnDamaged” signal is received, it will still loop through all 19 OnAttack abilities.

Any advice on this is appreciated, thank you all.

  • @Kylamon1
    5•6 months ago

    Without knowing your specific situation, it seems like each of you signals should not be connecting to one master “abilities” function to dole out the effects.

    Instead each signal should connect to its own function and that function is responsible for only its specif effect.

    ====== Another thought would be if you like your setup, change the if statement to a match/case. For many simple if checks the match is more optimized.

    • @JozzoOP
      1•6 months ago

      I think I understand…

      Instead of the player iterating through and calling all of its abilities, the ability just connects directly to whichever signal it needs on the player?

      My current setup is to add each Ability as a node to the player, so right now it follows the “call down, signal up” adage that I hear everyone say. What would be a good way to implment the other way? I assume I should rework my current setup otherwise it’d be “signal down, signal up”?

      • @Kylamon1
        2•6 months ago

        So player has all these nodes that provide abilities. Each node has a signal that the ability is activated. This is correct. What you do after the signal was your question.

        The two options i described were:

        #1 don’t just connect to one omnibus function. Don’t connect them all to a _gave_ability() function. This is what it sounds like you are doing. Instead seperate into seperat smaller functions. Connect ability As signal to functionA(), and abilityB signal to functionB(). Then yiu are not checking all 19 cases everything a signal is called.

        #2 if you are using the omnibus function _give_ability(ability), set an input parameter for the signal saying which specific signal was emitted. This can be done by code or the inspector when connecting the signal.

        Then on _give_ability(ability) do:

        match ability: abilityA: Give ability

  • @[email protected]
    3•6 months ago

    When you add a new ability you could register it in a OnAttack or whatever list for that character.

    Then you can just loop that list and activate them all.

    • @JozzoOP
      2•6 months ago

      I patched together some version of this using nested dictionaries:

      var abilities: Dictionary = {
	AbilityData.Trigger.BEFORE_ATTACK : {},
	AbilityData.Trigger.ON_ATTACK : {},
	AbilityData.Trigger.ON_HIT : {},
	AbilityData.Trigger.ON_KILL : {},
	AbilityData.Trigger.ON_DEATH : {},
	AbilityData.Trigger.ON_JUMP : {},
	AbilityData.Trigger.PASSIVE : {}
}

      with each value being another key:value pair of { "ability_id": <ability-node> } so I can keep a reference to the Ability node and use dictionary functions like .has() to check if a character has a specific ability:

      func has_ability(ability_data: AbilityData) -> bool:
	if abilities[ability_data.trigger_type].has(ability_data.id):
		return true
	return false

      Then when a trigger fires, it calls this (I omitted the return code):

      // Activates all abilities with the specified trigger type. Returns an array containing each ability that was activated this way.
//trigger_type is an enum
//data is just a resource containing things like position, target, ability owner, etc
func trigger(trigger_type: AbilityData.Trigger, data: AbilityActivationData) -> Array[Ability]:
	var abilities_to_activate: Dictionary = abilities.get(trigger_type)
	
	// Loops through the list of Ability nodes.
	for ability in abilities_to_activate.values():
		ability.activate(data)
		abilities_activated.append(ability)

      This seems to work, but it still gives me that tickling sensation that it could be a little cleaner.

  • @[email protected]English
    2•6 months ago

    It’s hard to give advice about how code should be structured, since there’s many ways of accomplishing the same things, but you’re doing the right thing by thinking about scalability before you get too deep to change it.

    You could try separating eacg trigger condition into their own functions, so that if an OnAttack gets triggered it will only check and loop through OnAttack abilities.

    Something like:

    OnAttack.connect( CheckOnAttack )
OnDamaged.connect( CheckOnDamaged )

func CheckOnAttack( ATTACK_TYPE ):
          match ATTACK_TYPE:
                  ....

func CheckOnDamaged( DAMAGE_TYPE ):
         match DAMAGE_TYPE:
                  ....