test/MONSTER_DESTROY_ERROR_ANALYSIS.md

Monster Destroy Error Analysis

Error Message

[SKILL_GFX_DEBUG] ComposerMan.Play: Exception in composer.Play() - The object of type 'CECMonster' has been destroyed but you are still trying to access it.

Root Cause

This is a race condition between skill casting and monster destruction. Here's the flow:

The Problem Flow

1. Skill is Cast → CECAttacksMan.AddSkillAttack() creates a CECAttackEvent
2. Attack Event Queued → Event is added to m_targets linked list in CECAttacksMan
3. Monster Destroyed → Monster dies/disappears, removed from m_NPCTab, GameObject destroyed
4. Attack Event Fires → CECAttackEvent.DoFire() is called (after delay)
5. GFX Event Created → A3DSkillGfxComposer.Play() creates CECSkillGfxEvent with target ID
6. GFX Event Ticks → CECSkillGfxEvent.Tick() tries to get target position
7. ERROR → get_pos_by_id() calls GetNPCFromAll() which returns null or destroyed object
8. Access Attempt → Code tries to access pNPC.GetPosVector3() or pNPC.transform on destroyed GameObject

Monster Destruction Flow (C#)

Entry Points for Monster Destruction:

1. CECNPCMan.OnMsgNPCDied() (line 291)

   • Receives MSG_NM_NPCDIED message
   • Calls pNPC.Killed(bDelay)
   • May call NPCDisappear() later

2. CECNPCMan.OnMsgNPCDisappear() (line 149)

   • Receives MSG_NM_NPCDISAPPEAR message
   • Calls NPCDisappear(nid)

3. CECNPCMan.OnMsgNPCOutOfView() (line 90)

   • Receives MSG_NM_NPCOUTOFVIEW message
   • Calls NPCLeave(nid)

4. CECNPCMan.OnMsgInvalidObject() (line 80)

   • Receives MSG_NM_INVALIDOBJECT message
   • Calls NPCLeave(nid)

Destruction Process:

// CECNPCMan.NPCDisappear() - line 156
void NPCDisappear(int nid)
{
    CECNPC pNPC = GetNPC(nid);
    if (pNPC)
    {
        pNPC.Disappear();
        NPCLeave(nid, true, false);  // Remove from active table
        m_aDisappearNPCs.Add(pNPC);  // Add to disappear table
    }
}

// CECNPCMan.NPCLeave() - line 177
void NPCLeave(int nid, bool bUpdateMMArray = true, bool bRelease = true)
{
    CECNPC pNPC = GetNPC(nid);
    if (!pNPC) return;
    
    // Remove from active NPC table
    m_NPCTab.Remove(nid);  // ← Monster removed from dictionary here
    
    if (bRelease)
        ReleaseNPC(pNPC);  // ← Calls DestroySelf()
}

// CECNPCMan.ReleaseNPC() - line 214
void ReleaseNPC(CECNPC pNPC)
{
    if (pNPC)
    {
        pNPC.Release();
        pNPC.DestroySelf();  // ← Calls Destroy(gameObject)
    }
}

// CECNPC.DestroySelf() - line 578
public void DestroySelf()
{
    Destroy(gameObject);  // ← Unity destroys the GameObject
}

Skill Casting Flow (C#)

When Skill is Cast:

// CECAttacksMan.AddSkillAttack() - line 270
public CECAttackEvent AddSkillAttack(int idHost, int idCastTarget, int idTarget, ...)
{
    var newEvent = new CECAttackEvent(...);
    m_targets.AddLast(newEvent);  // ← Event queued with target ID
    return newEvent;
}

// CECAttacksMan.Update() - line 142
private void Update()
{
    var node = m_targets.First;
    while (node != null)
    {
        if (!node.Value.m_bFinished)
            node.Value.Tick(dwDeltaTime);  // ← Event ticks every frame
        node = next;
    }
}

// CECAttackEvent.DoFire() - line 845
bool DoFire()
{
    // ... skill logic ...
    composerMan.Play(m_idSkill, m_idHost, m_idCastTarget, m_targets);
    // ← Creates GFX event with target IDs
}

// A3DSkillGfxComposer.Play() - line 630
public void Play(int nHostID, int nCastTargetID, List<TARGET_DATA> targets, ...)
{
    // Creates CECSkillGfxEvent for each target
    AddOneTarget(nCastTargetID, nHostID, szFly, szHit, tar, ...);
}

// A3DSkillGfxMan.AddOneSkillGfxEvent() - line 117
public bool AddOneSkillGfxEvent(...)
{
    A3DSkillGfxEvent pEvent = SkillGfxMan.InstanceSub.GetEmptyEvent(mode);
    pEvent.SetTargetID(nTargetID);  // ← Stores target ID
    PushEvent(pEvent);  // ← Event added to active list
}

// CECSkillGfxEvent.Tick() - line 126
public override void Tick(uint dwDeltaTime)
{
    // Updates host and target positions every frame
    m_bTargetExist = get_pos_by_id(..., (int)m_nTargetID, out m_vTargetPos, ...);
    // ← Tries to get position of target (monster)
    
    base.Tick(dwDeltaTime);  // ← May call GetTargetCenter()
}

// CECSkillGfxEvent.GetTargetCenter() - line 80
public override Vector3 GetTargetCenter()
{
    get_pos_by_id(..., (int)m_nTargetID, out vTargetCenter, ...);
    // ← Tries to access destroyed monster here!
    return vTargetCenter;
}

// CECSkillGfxEvent.get_pos_by_id() - line 449
private static bool get_pos_by_id(..., int nID, out Vector3 vPos, ...)
{
    if (GPDataTypeHelper.ISNPCID(nID))
    {
        CECNPC pNPC = pNPCMan?.GetNPCFromAll(nID);  // ← Returns null if destroyed
        
        if (pNPC != null)
        {
            vPos = pNPC.GetPosVector3();  // ← ERROR: GameObject destroyed!
            // OR
            vPos = pNPC.transform.position;  // ← ERROR: GameObject destroyed!
        }
    }
}

Why This Happens in C#

Unity's Destroy Behavior:

In Unity, when you call Destroy(gameObject):

1. The GameObject is marked for destruction
2. At the end of the frame, Unity destroys it
3. However, C# references to the component (CECNPC) are NOT set to null immediately
4. The reference still exists, but accessing any property throws: "The object has been destroyed"

The Race Condition:

Frame N:   Skill cast → CECAttackEvent created → Queued (delay: 200ms)
Frame N+1: Monster dies → Removed from m_NPCTab → GameObject destroyed
Frame N+2: CECAttackEvent.DoFire() called → Creates CECSkillGfxEvent
Frame N+3: CECSkillGfxEvent.Tick() → Tries to access destroyed monster → ERROR

Why C++ Didn't Have This Issue:

In C++, when an object is deleted:

• The pointer becomes invalid immediately
• Accessing it causes a crash (but predictable)
• The code likely had better null checks or the timing was different

In C# Unity:

• Destroyed objects are "fake null" - == null returns true, but the reference isn't actually null
• Unity's == operator is overloaded to handle destroyed objects
• But direct property access still throws the error

The Bug in GetNPCFromAll

There's also a secondary bug in CECNPCMan.GetNPCFromAll():

// Line 545-546 - BUG!
CECNPC pNPC = GetNPC(nid);
BMLogger.LogError($"... GetNPC returned {(pNPC.name)}");  // ← Crashes if pNPC is null!
if (pNPC != null)  // ← Check happens AFTER accessing pNPC.name

This will crash if pNPC is null.

Solutions

Solution 1: Check for Destroyed Objects (Recommended)

In CECSkillGfxEvent.get_pos_by_id(), check if the GameObject is destroyed:

private static bool get_pos_by_id(..., int nID, out Vector3 vPos, ...)
{
    if (GPDataTypeHelper.ISNPCID(nID))
    {
        CECNPC pNPC = pNPCMan?.GetNPCFromAll(nID);
        
        // Check if NPC exists AND GameObject is not destroyed
        if (pNPC != null && pNPC.gameObject != null)
        {
            vPos = pNPC.GetPosVector3();
            return true;
        }
    }
    return false;
}

Solution 2: Early Termination of GFX Events

When target is destroyed, mark the GFX event as finished:

// In CECSkillGfxEvent.Tick()
if (!m_bTargetExist && m_nTargetID != 0)
{
    // Target was destroyed, finish the event
    m_enumState = GfxSkillEventState.enumFinished;
    return;
}

Solution 3: Fix GetNPCFromAll Bug

public CECNPC GetNPCFromAll(int nid)
{
    CECNPC pNPC = GetNPC(nid);
    
    // Check null BEFORE accessing properties
    if (pNPC != null)
    {
        BMLogger.LogError($"... GetNPC returned {pNPC.name}");
        return pNPC;
    }
    
    BMLogger.LogError($"... NPC {nid} NOT FOUND");
    return null;
}

Solution 4: Validate Targets Before Creating GFX Events

In A3DSkillGfxComposer.Play(), validate targets exist before creating events:

public void Play(int nHostID, int nCastTargetID, List<TARGET_DATA> targets, ...)
{
    // Validate targets exist before creating GFX events
    if (targets != null)
    {
        for (int i = targets.Count - 1; i >= 0; i--)
        {
            var tar = targets[i];
            if (!ValidateTargetExists(tar.idTarget))
            {
                targets.RemoveAt(i);  // Remove invalid target
            }
        }
    }
    // ... rest of Play()
}

Recommended Fix

Apply all four solutions:

1. Fix the GetNPCFromAll null check bug
2. Add destroyed object checks in get_pos_by_id
3. Early terminate GFX events when target is destroyed
4. Validate targets before creating GFX events

This provides defense in depth and handles the race condition at multiple levels.