Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix attack scheduler #1305

Closed
wants to merge 1 commit into from
Closed

Conversation

ranisalt
Copy link
Member

I have stumbled across this problem when I was trying to use an attack speed that was not a multiple of 1000. Apparently the server only checks if there is a new action every 1000 ms, but this limits attack speed to multiples of this time frame, as the server won't check if there is a new attack to calculate in between time frames.

This fix changes it by adding a new SchedulerTask when the attack is in the middle of a time frame. I haven't had time and resources to check if this is a heavy change and if this works properly 100% of the time, but I hope @marksamman can have a look and say somethind 😀

@brunominervino
Copy link
Contributor

Hello @ranisalt,

This change don't do effect in distance weapon.

@ranisalt
Copy link
Member Author

ranisalt commented May 1, 2015

@brunominervino yes, distance attacks are treated differently (you can see the if weapon->interruptSwing that checks if weapon is melee or not) and I haven't figured out how to work this yet.

I will try my best.

@brunominervino
Copy link
Contributor

@ranisalt hmm, I understand, first thank you.
I will try to help you :) if I have any progress, I post here 👍

@@ -3383,15 +3383,22 @@ void Player::doAttacking(uint32_t)
Item* tool = getWeapon();
const Weapon* weapon = g_weapons->getWeapon(tool);
if (weapon) {
uint32_t delay;

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

        SchedulerTask* task = createSchedulerTask(delay, std::bind(&Game::checkCreatureAttack,
                              &g_game, getID()));
        setNextActionTask(task);

Where this change affect distance weapons too :)

@ranisalt
Copy link
Member Author

Thanks for pointing out, @brunominervino. I have fixed the code and it now works with distance weapons too :D just remember the MINIMUM attack speed is 100 ms, since it is hardcoded and setting it lower may break your server because too many actions will happen in a short amount of time.

I have also squashed the commits.

@brunominervino
Copy link
Contributor

@ranisalt now is working 👍 :)
Thank you bro!

@ranisalt
Copy link
Member Author

How's that going?

@kanohn
Copy link

kanohn commented Jun 26, 2015

if character try to attack while running speed attack does not work

@ranisalt
Copy link
Member Author

@kanohn this is intended behavior. One can not swing a weapon or throw a spear while running away, not even in Tibia.

Replace L3392 delay = getNextActionTime(); with delay = getAttackSpeed(); and it may work as you want.

@strndi
Copy link

strndi commented Aug 24, 2015

ranisalt, I have added this commit to my server and after that I have noticed that sometimes when you are attacking a creature, it will be impossible to push it. I'm almost certain that this change is what causes it, are you aware of any fix for this? Also, it seems to be when I am fist fightning that it's possible for me to push the creature again

I also did this change "Replace L3392 delay = getNextActionTime(); with delay = getAttackSpeed(); and it may work as you want."

@strndi
Copy link

strndi commented Aug 29, 2015

also attack speed behaves weirdly, both with this
"Replace L3392 delay = getNextActionTime(); with delay = getAttackSpeed();"
and your original code. I've tested both and there doesn't seem to be any difference.

when you move, it puts a delay on the attack but it only happens sometimes, my attackspeed is 300ms. i'll give you a demonstration: http://gfycat.com/RashClearcutGannet as you can see when I move around I attack much slower

@ranisalt
Copy link
Member Author

That's because your attack speed is so abusive it breaks. The canDoAction() checks if you aren't moving, as realistically you can not attack while moving, but you don't want that. You could try replacing everything inside if (weapon) with:

            result = weapon->useWeapon(this, tool, attackedCreature);
            SchedulerTask* task = createSchedulerTask(getAttackSpeed(), std::bind(&Game::checkCreatureAttack, &g_game, getID()));
            setNextActionTask(task);

That way the attack only cares for the last attack time, not if you are moving or whatever.

@ranisalt ranisalt mentioned this pull request Aug 31, 2015
@marcelaodev
Copy link

your code ignored the fist fighting

@ranisalt ranisalt closed this Sep 3, 2016
@zydent
Copy link

zydent commented Jan 17, 2017

I have the same problem as grisig https://gfycat.com/RashClearcutGannet @ranisalt

		Item* tool = getWeapon();
		const Weapon* weapon = g_weapons->getWeapon(tool);
		if (weapon) {
            result = weapon->useWeapon(this, tool, attackedCreature);
            SchedulerTask* task = createSchedulerTask(getAttackSpeed(), std::bind(&Game::checkCreatureAttack, &g_game, getID()));
            setNextActionTask(task);
		} else {
			result = Weapon::useFist(this, attackedCreature);
			SchedulerTask* task = createSchedulerTask(getAttackSpeed(), std::bind(&Game::checkCreatureAttack, &g_game, getID()));
            setNextActionTask(task);
		}

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

It does not work!

@brunominervino
Copy link
Contributor

@zydent have a open issue about this case #1319

@Mkalo
Copy link
Contributor

Mkalo commented Jan 20, 2017

@zydent This is not a bug in his change, distance weapons have interrupt swing (even in real tibia), it stops your attack when you move. If you want to remove this you can change this line:
https://github.com/otland/forgottenserver/blob/master/src/weapons.h#L163
to false.

The only problem with this pull request is it * completely ignored fist weapon.

@Mkalo Mkalo mentioned this pull request Jan 20, 2017
@ranisalt
Copy link
Member Author

The PR is broken because I messed with the branch and I will reopen it.

@zydent
Copy link

zydent commented Jan 20, 2017

@Mkalo I made the change "return false;" The speed stop when I move. https://gfycat.com/ShorttermOptimisticFlamingo

@Mkalo
Copy link
Contributor

Mkalo commented Jan 20, 2017

@zydent Then remove those lines:
https://github.com/otland/forgottenserver/blob/master/src/player.cpp#L1212-L1213

This may affect something else.

@zydent
Copy link

zydent commented Jan 21, 2017

@Mkalo Any action makes the fast attack stop, Of me emotion I hadn't noticed this big mistake. https://gfycat.com/SpicyQuarrelsomeIcelandichorse

@Mkalo
Copy link
Contributor

Mkalo commented Jan 21, 2017

@zydent Alright just delete setNextActionTask(task); in the doAttacking, that way nothing can interfere with the scheduled check. (That's not how real tibia works but thats just for your custom case.)

Also here:
https://github.com/otland/forgottenserver/blob/master/src/player.cpp#L3142-L3151
Remove everything and place only:
result = weapon->useWeapon(this, tool, attackedCreature);

You can undo the last change about those 2 lines before with this change.

If anything goes wrong please create a thread in the forum and tag me as this is just for your custom case not really a bug.

@Roniox12
Copy link

Hi @Mkalo can you help me with issue that ranged weapons stop attack speed during movement and using runes on this distro https://github.com/TwistedScorpio/OTHire ? Thank you !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants