Is code in episode 1 of New Game 2 actually an infinite loop?

Question

In episode 1 of New Game 2, around 9:40, there is a shot of the code that Nene has written:

Here is it in text form with the comments translated:

// the calculation of damage when attacked
void DestructibleActor::ReceiveDamage(float sourceDamage) 
{
    // apply debuffs
    auto resolvedDamage = sourceDamage;
    for (const auto& debuf:m_debufs)
    {
        resolvedDamage = debuf.ApplyToDamage(resolvedDamage);
        m_currentHealth -= resolvedDamage
        if (m_currentHealth <= 0.f)
        {
            m_currentHealth = 0.f;
            DestroyMe();
        }
    }
}

After the shot, Umiko, pointing at the for loop, said that the reason why the code crashed is that there is an infinite loop.

I don't really know C++ so I'm not sure whether what she is saying is true.

From what I can see, the for loop is just iterating through the debufs that the Actor currently has. Unless the Actor has an infinite amount of debufs, I don't think it can possibly become an infinite loop.

But I'm not sure because the only reason that there is a shot of the code is that they wanted to put an easter egg here, right? We would have just gotten a shot of the back of the laptop and heard Umiko saying "Oh you've got an infinite loop there". The fact that they actually showed some code makes me think that somehow the code is an easter egg of some sort.

Will the code actually create an infinite loop?

Answer 1

The code is not an infinite loop but it is a bug.

There's two (possibly three) issues:

• If no debufs are present no damage will be applied at all
• Excessive damage will be applied if there is more than 1 debuf
• If DestroyMe() immediately deletes the object and there are still m_debufs to be processed the loop will be executing over a deleted object and trashing memory. Most game engines have a destroy queue to work around this and more so that may not be an issue.

The application of damage should be outside the loop.

Here's the corrected function:

// the calculation of damage when attacked
void DestructibleActor::ReceiveDamage(float sourceDamage) 
{
    // apply debuffs
    auto resolvedDamage = sourceDamage;
    for (const auto& debuf:m_debufs)
    {
        resolvedDamage = debuf.ApplyToDamage(resolvedDamage);
    }
    m_currentHealth -= resolvedDamage
    if (m_currentHealth <= 0.f)
    {
        m_currentHealth = 0.f;
        DestroyMe();
    }
}

Answer 2

The code does not seem to create an infinite loop.

The only way the loop would be infinite would be if

debuf.ApplyToDamage(resolvedDamage);

or

DestroyMe();

were to add new items to the m_debufs container.

This seems unlikely. And if it were the case, the program could crash because of changing the container while being iterated.

The program would most likely crash because of the call to DestroyMe(); which presumably destroys the current object that is currently running the loop.

We can think of it as the cartoon where the 'bad guy' saws a branch to have the 'good guy' fall with it, but realizes too late that he's on the wrong side of the cut. Or the Midgaard Snake eating it's own tail.

---

I should also add that the most common symptom of an infinite loop is that it freezes the program or makes it not responsive. It will crash the program if it allocates memory repeatedly, or does something that ends up dividing by zero, or the likes.

---

Based on the comment by Aki Tanaka ,

Probably helpful: additional screenshot of Umiko saying that "It was calling the same operation over and over again", which might not be shown in the code.

"It was calling the same operation over and over again" This is more likely.

Assuming that DestroyMe(); is not designed to be called more than once, it is more likely to cause a crash.

A way to fix this issue would be to change the if for something like this:

    if (m_currentHealth <= 0.f)
    {
        m_currentHealth = 0.f;
        DestroyMe();
        break;
    }

This would exit the loop when the DestructibleActor is destroyed, making sure that 1) the DestroyMe method is called only once and 2) don't apply buffs uselessly once the object is already considered dead.

Answer 3

There are several issues with the code:

1. If there are no debufs, no damage would be taken.
2. DestroyMe() function name sounds dangerous. Depending on how its implemented, it might or might not be an issue. If it's just a call to the destructor of the current object wrapped in a function, then there is an issue, as the object would be destroyed in the middle of it executing code. If it's a call to a function which queues the deletion event of the current object, then there is no issue, as the object would be destroyed after it completes its execution and the event loop kicks in.
3. The actual issue that seems to be mentioned in the anime, the "It was calling the same operation over and over again" -- it will call DestroyMe() as long as m_currentHealth <= 0.f and there are more debuffs left to iterate, which might result in DestroyMe() being called multiple times, over and over again. The loop should stop after the first DestroyMe() call, because deleting an object more than once results in memory corruption, which will likely result in a crash in the long run.

I'm not really sure why every debuf takes away the health, instead of the health being taken away only once, with the effects of all debuffs being applied on the initial damage taken, but I will assume that's the correct game logic.

The correct code would be

// the calculation of damage when attacked
void DestructibleActor::ReceiveDamage(float sourceDamage) 
{
    // apply debuffs
    auto resolvedDamage = sourceDamage;
    for (const auto& debuf:m_debufs)
    {
        resolvedDamage = debuf.ApplyToDamage(resolvedDamage);
        m_currentHealth -= resolvedDamage
        if (m_currentHealth <= 0.f)
        {
            m_currentHealth = 0.f;
            DestroyMe();
            break;
        }
    }
}