attackSpeed enabled = HIGH CPU USAGE

[Drucken]

Engine: TFS 1.5 (Nekiro Downgrade)
Tibia Version: 8.60

Is there any solution for the attack speed issue in TFS 1.5 version 8.60 downgraded by Nekiro? I ask because having the "classicAttackSpeed" boolean activated causes very high CPU usage, which causes lag on the server. When "classicAttackSpeed" is disabled, CPU usage drops by 70%.

The problem seems to come from the "doAttacking" function in the player.cpp file... This is the TFS 1.5 doAttacking:

Spoiler: doAttacking TFS 1.5 Function

void Player::doAttacking(uint32_t)
{
    if (lastAttack == 0) {
        lastAttack = OTSYS_TIME() - getAttackSpeed() - 1;
    }

    if (hasCondition(CONDITION_PACIFIED)) {
        return;
    }

    if ((OTSYS_TIME() - lastAttack) >= getAttackSpeed()) {
        bool result = false;

        Item* tool = getWeapon();
        const Weapon* weapon = g_weapons->getWeapon(tool);
        uint32_t delay = getAttackSpeed();
        bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);

        if (weapon) {
            if (!weapon->interruptSwing()) {
                result = weapon->useWeapon(this, tool, attackedCreature);
            } else if (!classicSpeed && !canDoAction()) {
                delay = getNextActionTime();
            } else {
                result = weapon->useWeapon(this, tool, attackedCreature);
            }
        } else {
            result = Weapon::useFist(this, attackedCreature);
        }

        SchedulerTask* task = createSchedulerTask(std::max<uint32_t>(SCHEDULER_MINTICKS, delay), std::bind(&Game::checkCreatureAttack, &g_game, getID()));
        if (!classicSpeed) {
            setNextActionTask(task, false);
        } else {
            g_scheduler.addEvent(task);
        }

        if (result) {
            lastAttack = OTSYS_TIME();
        }
    }
}

And here is another type of "doAttacking" from OTX 2 (based on TFS 0.3.7, older version), I'm only putting it here because it's done differently:

Spoiler: doAttacking OTX 2 Function

void Player::doAttacking(uint32_t)
{
    uint32_t attackSpeed = getAttackSpeed();
    if(attackSpeed == 0 || (hasCondition(CONDITION_PACIFIED) && !hasCustomFlag(PlayerCustomFlag_IgnorePacification)))
    {
        lastAttack = OTSYS_TIME();
        return;
    }

    if(!lastAttack)
        lastAttack = OTSYS_TIME() - attackSpeed - 1;
    else if((OTSYS_TIME() - lastAttack) < attackSpeed)
        return;

    if(const Weapon* _weapon = g_weapons->getWeapon(weapon))
    {
        if(!g_config.getBool(ConfigManager::CLASSIC_ATTACK_SPEED) && _weapon->interruptSwing() && !canDoAction())
        {
            SchedulerTask* task = createSchedulerTask(getNextActionTime(),
                boost::bind(&Game::checkCreatureAttack, &g_game, getID()));
            setNextActionTask(task);
        }
        else
        {
            if((!_weapon->hasExhaustion() || !hasCondition(CONDITION_EXHAUST)) && _weapon->useWeapon(this, weapon, attackedCreature))
                lastAttack = OTSYS_TIME();

            updateWeapon();
        }
    }
    else if(Weapon::useFist(this, attackedCreature))
        lastAttack = OTSYS_TIME();
}

I have no idea if the attack speed is better processed in OTX 2, I only know that TFS 1.5 does not have a good way of processing attacks, which causes very high CPU usage.

Someone who can help with this please
​

BUMP

 

[Svira]

Several techniques can be used to optimize this code and reduce CPU usage:

Use the modulo (%) operator to avoid calling OTSYS_TIME() every cycle of the loop. The value returned by this function changes too quickly to be used as the basis for a continuous countdown.

Checking if we need to call useWeapon() only once instead of twice. This will save time and reduce CPU usage.

Avoid creating new objects when not needed. Instead of creating a new SchedulerTask object each time, you can store it in the Player class and reuse it when required.

Removal of unnecessary conditions that can lead to wasted time and increased CPU usage. For example, the "if (!classicSpeed)" condition is executed only when classicSpeed is true, which means that the negation value is always false. This can be simplified by removing the condition altogether.

Reducing the number of function calls by removing unnecessary intermediate variables. For example, the value of the variable "result" is only ever used once. This can be simplified by doing the check directly on the condition where it is needed.

Below is the optimized code:

void Player::doAttacking(uint32_t)
{
static SchedulerTask* task = createSchedulerTask(SCHEDULER_MINTICKS, std::bind(&Game::checkCreatureAttack, &g_game, getID()));

if (lastAttack == 0) {
lastAttack = OTSYS_TIME() - getAttackSpeed() - 1;
}

if (hasCondition(CONDITION_PACIFIED)) {
return;
}

uint32_t time = OTSYS_TIME();
uint32_t attackSpeed = getAttackSpeed();
if ((time - lastAttack) % attackSpeed != 0) {
return;
}

Item* tool = getWeapon();
const Weapon* weapon = g_weapons->getWeapon(tool);
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);

bool result = weapon ? !weapon->interruptSwing() && weapon->useWeapon(this, tool, attackedCreature) : Weapon::useFist(this, attackedCreature);
if (result) {
lastAttack = time;
}

if (!classicSpeed && !canDoAction()) {
uint32_t delay = getNextActionTime();
task->setInterval(std::max<uint32_t>(SCHEDULER_MINTICKS, delay));
setNextActionTask(task, false);
} else {
g_scheduler.addEvent(task);
}


}

 

[GamerGoiano]

Try it too:

void Player::doAttacking(uint32_t)
{
if (hasCondition(CONDITION_PACIFIED)) {
return;
}

static uint32_t attackSpeed = getAttackSpeed();
static SchedulerTask* task = createSchedulerTask(std::max<uint32_t>(SCHEDULER_MINTICKS, getNextActionTime()), std::bind(&Game::checkCreatureAttack, &g_game, getID()));

if (lastAttack == 0) {
    lastAttack = OTSYS_TIME() - attackSpeed - 1;
}

if ((OTSYS_TIME() - lastAttack) % attackSpeed != 0) {
    return;
}

Item* tool = getWeapon();
const Weapon* weapon = g_weapons->getWeapon(tool);
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);

bool result = weapon ? !weapon->interruptSwing() && weapon->useWeapon(this, tool, attackedCreature) : Weapon::useFist(this, attackedCreature);
if (result) {
    lastAttack = OTSYS_TIME();
}

if (!classicSpeed && !canDoAction()) {
    uint32_t delay = getNextActionTime();
    task->setInterval(std::max<uint32_t>(SCHEDULER_MINTICKS, delay));
    setNextActionTask(task, false);
} else {
    g_scheduler.addEvent(task);
}

}

 

[Drucken]

@Svira im going to test it.

@GamerGoiano your function uses the "task->setInterval", well "setInterval" this is not defined in tfs 1.5, could you share this function as well please?

 

[GamerGoiano]

@Svira im going to test it.

@GamerGoiano your function uses the "task->setInterval", well "setInterval" this is not defined in tfs 1.5, could you share this function as well please?

class SchedulerTask {
public:
    void setInterval(uint32_t interval) {
        this->interval = interval;
    }

    uint32_t getInterval() const {
        return interval;
    }

    // Other member functions and data members here...

private:
    uint32_t interval;
    // Other data members here...
};

 

[Drucken]

Apparently there is a problem in both functions @GamerGoiano @Svira , apart from the setInterval (I added it), the character literally does not attack.

 

[GamerGoiano]

void Player::doAttacking(uint32_t)
{
static SchedulerTask* task = nullptr; // Only create once
uint32_t currentTime = OTSYS_TIME();
uint32_t attackDelay = getAttackSpeed();
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);

if (lastAttack == 0 || (currentTime - lastAttack) % attackDelay == 0) {
    lastAttack = currentTime;

    if (hasCondition(CONDITION_PACIFIED)) {
        return;
    }

    Item* tool = getWeapon();
    const Weapon* weapon = g_weapons->getWeapon(tool);

    if (weapon) {
        bool interruptSwing = weapon->interruptSwing();

        if (!interruptSwing || (!classicSpeed && canDoAction())) {
            if (weapon->useWeapon(this, tool, attackedCreature)) {
                if (classicSpeed) {
                    if (task) {
                        g_scheduler.removeEvent(task);
                    }
                    task = createSchedulerTask(SCHEDULER_MINTICKS, std::bind(&Game::checkCreatureAttack, &g_game, getID()));
                    g_scheduler.addEvent(task);
                }
            }
        } else {
            attackDelay = getNextActionTime();
        }
    } else {
        if (Weapon::useFist(this, attackedCreature)) {
            if (classicSpeed) {
                if (task) {
                    g_scheduler.removeEvent(task);
                }
                task = createSchedulerTask(SCHEDULER_MINTICKS, std::bind(&Game::checkCreatureAttack, &g_game, getID()));
                g_scheduler.addEvent(task);
            }
        }
    }
}
}

 

[ForgottenNot]

Abu solution for this?

 

[kamilcioo]

Remove:
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);
No need for an extra if you never will actually use it.

Introduce variable for OTSYS_TIME since it's used twice
Use guard so it's more readable:

    uint32_t currentTime = OTSYS_TIME();
    if (lastAttack == 0) {
        lastAttack = currentTime - getAttackSpeed() - 1;
    }

    if (hasCondition(CONDITION_PACIFIED)) {
        return;
    }

    if ((currentTime - lastAttack) < getAttackSpeed()) {
        return;
    }

 

[mano368]

Remove:
bool classicSpeed = g_config.getBoolean(ConfigManager::CLASSIC_ATTACK_SPEED);
No need for an extra if you never will actually use it.

Introduce variable for OTSYS_TIME since it's used twice
Use guard so it's more readable:

    uint32_t currentTime = OTSYS_TIME();
    if (lastAttack == 0) {
        lastAttack = currentTime - getAttackSpeed() - 1;
    }

    if (hasCondition(CONDITION_PACIFIED)) {
        return;
    }

    if ((currentTime - lastAttack) < getAttackSpeed()) {
        return;
    }

how do you implement that in the 1.5 tfs downgrade?

 

[Nekiro]

great thread I learned a lot on how to optimize code, I will now remove every single if from my code and never declare variables, because its apparently very slow and causes huge cpu usage