[Help] My Attack System Only Deals Damage Sometimes -->Unity 2D Combat System

Unity Engine
, , ,
1

Hello everyone,

I’ve been working on my own 2D combat system for the past three days, spending around five hours on it. Since there aren’t many good tutorials on YouTube, I tried to figure everything out myself using my own knowledge and creativity. Overall, everything seems to be working as expected: Attacks can be executed, combos are detected correctly, and the system runs smoothly.

The Problem:

  • If I attack once and then stop, the first attack deals damage correctly, but the second one does not.
  • If I spam the attack button and chain combos together, no damage is dealt after the first hit.

I’m about 99% sure that I’m correctly calling my methods inside the animation events, and I have already debugged everything. All methods triggered by animations are marked with comments in my code.

Here is my complete script:

using UnityEngine;
using UnityEngine.InputSystem;

public class PlayerCombatSystem : MonoBehaviour
{
    [SerializeField] private bool StartTimerForComboWindow = false; 
    [SerializeField] private ComboAttackData comboAttackDataScribtible;  
    [SerializeField] private PlayerAnimationsCombatManager playerAnimationsCombatManagerScriptRef;
    [SerializeField] private PlayerAnimationsScript playerAnimationsScriptRef;
    [SerializeField] private Camera_System camera_SystemScriptRef;
    [SerializeField] private PlayerAttackData playerAttackDataScriptible;
    [SerializeField] private PlayerAttackStates playerAttackStatesScriptRef;
    private PlayerInput playerInput;  // Referenz zum neuen Input System

    [Header("Combat Settings")]
    [SerializeField] private PolygonCollider2D AttackColliderFirstAttack;
    [SerializeField] private PolygonCollider2D StrikeDownCollider;
    [HideInInspector] public bool isCurrentlyAttacking = false; 


    void Awake()
    {

        if (!playerAttackStatesScriptRef) playerAttackStatesScriptRef = GetComponent<PlayerAttackStates>();
        if (!comboAttackDataScribtible) comboAttackDataScribtible = GetComponent<ComboAttackData>();
        if (!playerAnimationsCombatManagerScriptRef) playerAnimationsCombatManagerScriptRef = GetComponent<PlayerAnimationsCombatManager>();
        if (!playerAnimationsScriptRef) playerAnimationsScriptRef = GetComponent<PlayerAnimationsScript>();
        if (!camera_SystemScriptRef) camera_SystemScriptRef = GetComponent<Camera_System>(); 
        if (!playerAttackDataScriptible) playerAttackDataScriptible = GetComponent<PlayerAttackData>();


        // Setze den aktuellen State auf inAttack1
        playerAttackStatesScriptRef.currentAttackState = PlayerAttackStates.PlayerAttackState.inAttack1;
 

        //Folgender Code, deaktivert einfach nur alle verfügbaren Colliders. 
        if (AttackColliderFirstAttack != null)
            AttackColliderFirstAttack.enabled = false;

        if (StrikeDownCollider != null)
            StrikeDownCollider.enabled = false;

        playerInput = GetComponent<PlayerInput>();
    }

    void Update()
    {
        if (StartTimerForComboWindow)
        {
            comboAttackDataScribtible.InputComboTimer += Time.deltaTime; // Timer hochzählen

            // Überprüfen, ob der Spieler innerhalb des Zeitfensters angreift
            if (playerInput.actions["Attack"].WasPressedThisFrame()) 
            {
                //Starte die Angriffs Animation
                playerAnimationsCombatManagerScriptRef.HandleCombatAttacks(); 

                playerAttackStatesScriptRef.currentAttackState = PlayerAttackStates.PlayerAttackState.inAttack2; //im zweiten Angriffs State (Wichtig, immer im Überblick bahalten!)

                StartTimerForComboWindow = false; // Deaktiviere den Timer
                comboAttackDataScribtible.InputComboTimer = 0f; // Timer zurücksetzen, für bugs die ich schon hundertmal gemacht habe....
            }
        }

        // Falls der Timer überschritten wird
        if (comboAttackDataScribtible.InputComboTimer > comboAttackDataScribtible.MaxTimeForComboAttack)
        {
            StartTimerForComboWindow = false; // Timer deaktivieren
            comboAttackDataScribtible.InputComboTimer = 0f; // Timer zurücksetzen
            
            playerAttackStatesScriptRef.currentAttackState = PlayerAttackStates.PlayerAttackState.inAttack1; //Spieler befindet sich diesesmal aber jetzt im ersten Zustand

        }

        MirrorAttackDetection(); //Das spielgelt beide verfügbaren Colliders, damit der Spieler auch nach Rechts schlageb kann. 
    }

    
    public void OnAttack()
    {

        if (AttackColliderFirstAttack == null)
        {
            DebugLimiter.LogError("Der Angirff Collider wurde nicht gesetzt, Fixen");
            return;
        }
        if (!camera_SystemScriptRef)
            camera_SystemScriptRef = GetComponent<Camera_System>(); 

        isCurrentlyAttacking = true; //Eine Extrem Wichtig, es sorgt für flüßiege Animationen. 

        AttackColliderFirstAttack.isTrigger = true;
        StrikeDownCollider.isTrigger = true; 
    }

    // Methode wird von Animation Event aufgerufen, Der Timer der dann für den zweiten Schlag fenster offen ist. 
    private void SetTimerFlagTrue() 
    {
        StartTimerForComboWindow = true;
        comboAttackDataScribtible.InputComboTimer = 0f; // Timer starten
    }

    // Das wird im Animation Window aktivert, und deaktviert immer nur den benötigten Collider für den Angriff
    public void ActivateAttackCollider()
    {
        Debug.Log("Spieler hat etwas getroffen! Aktueller Angriffszustand: " + playerAttackStatesScriptRef.currentAttackState);
        if(playerAttackStatesScriptRef.currentAttackState == PlayerAttackStates.PlayerAttackState.inAttack1)
        {
            AttackColliderFirstAttack.enabled = true;
            StrikeDownCollider.enabled = false; 
        }
        else if(playerAttackStatesScriptRef.currentAttackState == PlayerAttackStates.PlayerAttackState.inAttack2)
        {
            AttackColliderFirstAttack.enabled = false;
            StrikeDownCollider.enabled = true;
            
        }
    }

    // Wird im Animation Evnt aufgeuffen, und deatkviert hier nur alles, WICHRTG Auch der isCurrentlyAttacking = false; wird auf falsch!!!
    public void DeactivateAttackCollider()
    {
        AttackColliderFirstAttack.enabled = false;
        StrikeDownCollider.enabled = false; 
        isCurrentlyAttacking = false;
    }


     // Hier wird der Collider gespielt, zusammenkomminaktions Script ist hier bei -playerAnimationsScriptRef-
    private void MirrorAttackDetection()
    {
        if (playerAnimationsScriptRef.lookingToRight)
        {
            if (AttackColliderFirstAttack.enabled)
                AttackColliderFirstAttack.transform.rotation = Quaternion.Euler(0f, 180f, 0f);

            if (StrikeDownCollider.enabled)
                StrikeDownCollider.transform.rotation = Quaternion.Euler(0f, 180f, 0f);
        }
        else
        {
            if (AttackColliderFirstAttack.enabled)
                AttackColliderFirstAttack.transform.rotation = Quaternion.Euler(0f, 0f, 0f);

            if (StrikeDownCollider.enabled)
                StrikeDownCollider.transform.rotation = Quaternion.Euler(0f, 0f, 0f);
        }
    }


        public void OnAttackEnd() //Sicherheitshalber nochmal eine Methode was aufgerufen wird um isCurrentlyAttacking auf false zu setzen. Je nach bedarf einsetzen.
    {
        isCurrentlyAttacking = false;
    }



    // Kollisions-Logik: Wird aufgerufen, wenn der Collider mit einem Gegner kollidiert
    private void OnTriggerEnter2D(Collider2D other)
    {
        if (!isCurrentlyAttacking || camera_SystemScriptRef == null) return;

        if (LayerMask.LayerToName(other.gameObject.layer) != "RatEnemy") return;

        if (!other.TryGetComponent(out EnemyHealth enemyHealth)) return;

        camera_SystemScriptRef.TriggerShake(); // Kamera schütteln

        int damage = 0;

        // Falls Spieler im ersten Angriff ist
        if (playerAttackStatesScriptRef.currentAttackState == PlayerAttackStates.PlayerAttackState.inAttack1)
        {
            damage = PlayerMovement.isGrounded ? playerAttackDataScriptible.attackDamage : playerAttackDataScriptible.inAirAttackDamage;
        }
        // Falls Spieler im zweiten Angriff ist und auf dem Boden steht
        else if (playerAttackStatesScriptRef.currentAttackState == PlayerAttackStates.PlayerAttackState.inAttack2 && PlayerMovement.isGrounded)
        {
            damage = comboAttackDataScribtible.ComboAttackDamage;
        }

        // Falls kein Schaden gesetzt wurde, verlasse die Methode (z. B. falls inAttack2 in der Luft ist)
        if (damage == 0) return;

        enemyHealth.TakeDamage(damage);
    }


}

Additional Info:

My attack system works with several other scripts, but I have included the most relevant ones here. It consists of three main scripts:

  • The main combat script
  • The PlayerAnimationsScript, which manages animations
  • The PlayerAnimationsCombatManager script, which controls attack animations

Here are the relevant scripts:

1. PlayerAnimationsScript



 using UnityEngine;

public class PlayerAnimationsScript : MonoBehaviour
{
    [Header("Component References")]
    [SerializeField] private PlayerAttackData playerAttackDataScriptible; 
    [SerializeField] private Animator playerAnimator;
    [SerializeField] private SpriteRenderer playerSpriteRenderer;
    [SerializeField] private PlayerMovement playerMovementScriptRef;
    [SerializeField] private ComboAttackData comboAttackDataScriptible;
    [SerializeField] private PlayerCombatSystem playerCombatSystemScriptRef;


    [Header("Flip Sprite")]
    [HideInInspector] public bool lookingToRight; 

    [Header("PauseFrames")]
    private Coroutine CoroutineForAnimationFreeze; 

    [Header("Animation Parameters")]
    private const string WALK_PARAM = "isWalking";
    private const string JUMP_TRIGGER = "isJumping";
    private const string ATTACK_TRIGGER = "isAttack1";
    private const string COMBO_ATTACKTRIGGER = "isDownAttack"; 

    [SerializeField] private bool allowAnimatePlayer = true;

    private void Awake()
    {
        if (!playerAnimator) playerAnimator = GetComponent<Animator>();
        if (!playerSpriteRenderer) playerSpriteRenderer = GetComponent<SpriteRenderer>();
        if (!playerMovementScriptRef) playerMovementScriptRef = GetComponent<PlayerMovement>();
        if (!comboAttackDataScriptible)comboAttackDataScriptible = GetComponent<ComboAttackData>();
        if (!playerCombatSystemScriptRef) playerCombatSystemScriptRef = GetComponent<PlayerCombatSystem>();
    }

    private void Update()
    {
        if (!allowAnimatePlayer)
        {
            ResetToIdle();
            return;
        }
    }

    public void HandleMovementAnimation()
    {
        bool isMoving = Mathf.Abs(PlayerMovement.movementDirectionValue) > 0.1f;
        playerAnimator.SetBool(WALK_PARAM, isMoving);
    }

    public void ChangePlayerSprite()
    {
        if (Input.GetKey(KeyCode.A) && PlayerMovement.isWalkingAllowed && PlayerMovement.movementDirectionValue < 0)
        {
            lookingToRight = playerSpriteRenderer.flipX = true;
        }
        else if (Input.GetKey(KeyCode.D) && PlayerMovement.isWalkingAllowed && PlayerMovement.movementDirectionValue > 0)
        {
            lookingToRight = playerSpriteRenderer.flipX = false;
        }
    }

    public void HandleJumpAnimation()
    {
        if (Input.GetButtonDown("Jump") && PlayerMovement.isGrounded)
        {
            playerAnimator.SetTrigger(JUMP_TRIGGER);
        }
    }

    public void ResetToIdle()
    {
        playerAnimator.SetBool(WALK_PARAM, false);
        playerAnimator.ResetTrigger(JUMP_TRIGGER);
        playerAnimator.ResetTrigger(ATTACK_TRIGGER);
        playerAnimator.ResetTrigger(COMBO_ATTACKTRIGGER);
    }

    private void StopAnimationForFrames()
    {
        playerAnimator.speed = playerAttackDataScriptible.AnimationSpeedOnFrezze; 
    }

    private void StartAnimationAfterFrames()
    {
        playerAnimator.speed = playerAttackDataScriptible.AnimationSpeed; 
    }

    #region Combat Animations

    public void HandleAttackAnimation()
    {
        if (playerCombatSystemScriptRef.isCurrentlyAttacking)
        {
            // Für den ersten Angriff
            playerAnimator.SetTrigger(ATTACK_TRIGGER);
            playerAnimator.SetBool(WALK_PARAM, false);
        }
        else
        {
            playerAnimator.ResetTrigger(ATTACK_TRIGGER);
        }

        // Sicherheitsprüfung für Second Attack
        
    }

    #endregion


}
  1. PlayerAnimationsCombatManager
using UnityEngine;

public class PlayerAnimationsCombatManager : MonoBehaviour
{
    [Header("References")]
    [SerializeField] private PlayerAnimationsScript playerAnimationsScriptRef;
    [SerializeField] private Animator playerAnimator; 

    void Awake()
    {
        if(!playerAnimationsScriptRef) playerAnimationsScriptRef = GetComponent<PlayerAnimationsScript>();
        if(!playerAnimator) playerAnimator = GetComponent<Animator>();
    }

    void Update()
    {
        playerAnimationsScriptRef.HandleAttackAnimation();
    }

    public void HandleCombatAttacks()
    {
        playerAnimator.Play("isDownAttack"); 
    }

    
}

Maybe I’m missing something? Does anyone have an idea what could be causing this issue?

Thanks for any help! :blush:

2

You say this:

Well… is the code running?

Or is it running incorrectly?

Your actions to correct those two things will be completely different.

The actual problem could be something else.

You will know you have “debugged everything” when it works.

Sounds like you wrote a bug… and that means… time to start debugging!

By debugging you can find out exactly what your program is doing so you can fix it.

Use the above techniques to get the information you need in order to reason about what the problem is.

You can also use Debug.Log(...); statements to find out if any of your code is even running. Don’t assume it is.

Once you understand what the problem is, you may begin to reason about a solution to the problem.

Remember with Unity the code is only a tiny fraction of the problem space. Everything asset- and scene- wise must also be set up correctly to match the associated code and its assumptions.

1 Like
3

Thank you, Kurt-Dekker!

You were absolutely right—I must have made a dumb mistake. I went ahead and added another debug message that prints the current damage value whenever the player hits an enemy. That’s when I noticed that the comboAttackDamage was actually set to 0 in the code.

Thanks again for the reminder to always double-check my debugging process! I really appreciate your help. :blush:

4

Do yourself a favor and avoid such if/else if … these are hard to read. You should pull out either the common flag (isWalkingAllowed) or the Input, or both. And then maybe you’ll realize … wait a second … I already know the direction, and I don’t need to check input again (I assume PlayerMovement does this when it sets “movementDirectionValue”).

if (PlayerMovement.isWalkingAllowed)
   lookingToRight = playerSpriteRenderer.flipX = PlayerMovement.movementDirectionValue < 0;
1 Like
5

Also, I will add that this sort of silently figure out what I mean here please oh-please-don’t-give-me-a-null-ref stype of coding is cute, but it is a CONSTANT source of confusion:

In general, avoid coding like this. You are only increasing the number of things that can go wrong, all while thinking you’re saving yourself some time.

Keep in mind that using GetComponent() and its kin (in Children, in Parent, plural, etc) to try and tease out Components at runtime is definitely deep into super-duper-uber-crazy-Ninja advanced stuff.

Don’t use all those Get/Find things unless you absolutely MUST use them. And when you do decide you’re ready to do it, here’s the bare minimum of stuff you absolutely MUST keep track of if you insist on using these crazy Ninja methods:

  • what you’re looking for:
    → one particular thing?
    → many things?
  • where it might be located (what GameObject?)
  • where the Get/Find command will look:
    → on one GameObject? Which one? Do you have a reference to it?
    → on every GameObject?
    → on a subset of GameObjects?
  • what criteria must be met for something to be found (enabled, named, etc.)
  • if your code expects one instance and later you have many (intentional or accidental), does it handle it?

If you are missing knowledge about even ONE of the things above, your call is likely to FAIL.

If you have issues, start debugging. We know the above methods will ALWAYS work so find out what you’re doing wrong before bothering to make a forum post.

This sort of coding is to be avoided at all costs unless you know exactly what you are doing.

Botched attempts at using Get- and Find- are responsible for more crashes than useful code, IMNSHO.

If you run into an issue with any of these calls, start with the documentation to understand why.

There is a clear set of extremely-well-defined conditions required for each of these calls to work, as well as definitions of what will and will not be returned.

In the case of collections of Components, the order will NEVER be guaranteed, even if you happen to notice it is always in a particular order on your machine.

It is ALWAYS better to go The Unity Way™ and make dedicated public fields and drag in the references you want.

1 Like
6

Thanks a lot for the detailed explanation! I hadn’t thought too much about it before, but your points make total sense. I’ll definitely make sure to set my references properly in the future instead of spamming GetComponent() and Find().

Really appreciate you taking the time to explain it so clearly – it’s definitely going to help me improve my coding practices!

Best regards,

7

Thank you so much for your feedback and for pointing out ways to improve my code! Based on your suggestion, I’ve made a few changes to simplify the player movement logic and reduce redundancy. Here’s the updated version:

Original Code:

csharp

KopierenBearbeiten

if (Input.GetKey(KeyCode.A) && PlayerMovement.isWalkingAllowed && PlayerMovement.movementDirectionValue < 0)
{
    lookingToRight = playerSpriteRenderer.flipX = true;
}
else if (Input.GetKey(KeyCode.D) && PlayerMovement.isWalkingAllowed && PlayerMovement.movementDirectionValue > 0)
{
    lookingToRight = playerSpriteRenderer.flipX = false;
}

Updated Code:

csharp

KopierenBearbeiten

if (PlayerMovement.isWalkingAllowed)
{
    lookingToRight = playerSpriteRenderer.flipX = PlayerMovement.movementDirectionValue < 0;
}

Explanation:

  • I removed the repeated checks for Input.GetKey(KeyCode.A) and Input.GetKey(KeyCode.D) since the movementDirectionValue already contains the necessary information for determining the direction.
  • The check for PlayerMovement.isWalkingAllowed remains to ensure that the player can move, but now we simply check if movementDirectionValue is negative (for left) or positive (for right), which makes the code much cleaner and easier to read.

I appreciate your insights, and I’ll continue to refine the code further based on such valuable feedback.

Thanks again for your help!

Best regards,