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

[CPP] Update useMobAbility to check distance #5815

Conversation

0x05010705
Copy link
Contributor

I affirm:

  • I understand that if I do not agree to the following points by completing the checkboxes my PR will be ignored.
  • I understand I should leave resolving conversations to the LandSandBoat team so that reviewers won't miss what was said.
  • I have read and understood the Contributing Guide and the Code of Conduct.
  • I have tested my code and the things my code has changed since the last commit in the PR and will test after any later commits.

What does this pull request do?

This resolves an edge-case where if a mob is issued the command mob:useMobAbility(####), no distance check is performed on the ability being used. This can cause the mob to ready a skill while the player is already out of range and results in the mob pseudo-stunning itself. In cases where a mob may be issued multiple calls to useMobAbility(####) in quick succession, the mob becomes locked in place preventing it from moving or performing any other actions.

The function itself, in its current iteration, provides no guarantee that the mob will successfully use the ability specified as an argument unless distance checks are performed within that same mob script. This update does not change this consideration, but rather instead of the mob stunning itself for 3s, it will fail the distance check and just continue without performing the skill.

Steps to test these changes

** Before Rebuilding **
1 - Move a character to any zone with mobs
2 - Find a mob and engage combat with it.
3 - Use !exec target:useMobAbility(470) (or any mob skill)
4 - Update your movement speed !speed 200
5 - Run away from the mob and issue !exec target:useMobAbility(470) while you are out of range (15y+)
6 - Note that the mob will still "Ready" the skill.
7 - Repeatedly issue the useMobAbility command and note that the mob becomes stunlocked.

** After Rebuilding **

  • Repeat the steps as listed above
  • After reaching Step 5, note that the mob no longer attempts to "Ready" the skill if you are out of range
  • After reaching Step 7, note that the mob no longer becomes stun locked -- It instead will continue pathing to the target.

Special Thanks to @TracentEden for helping me develop/update this.

@zach2good zach2good added the core Gives visibility for reviewers for impacted areas involving cpp label May 27, 2024
@zach2good
Copy link
Contributor

This can cause the mob to ready a skill while the player is already out of range and results in the mob pseudo-stunning itself.

Whats the behaviour on retail? They won't fire the skill if the player is out of range and just keep chasing, and not lose their TP? What relation does this have to kiting, etc.

@0x05010705
Copy link
Contributor Author

So perhaps a more in-depth description

On retail, mobs do not ready a TP Skill on a player unless the player is within range. A mob can ready a skill while the player is in range, but then fail because the player has moved out of range by the end of the wind-up time.

If a mob has no valid targets for a TP move, but does have access to a self-target skill or buff skill, then it will/should opt for that skill.

This is currently how things work in LSB as well.

However, there are two different cases of useMobAbility based on whether an argument is presented or not.
1 - useMobAbility() --- No argument
The logic performed when this is called on a mob acts like so:

  • Grab the ability list for this mob from the database.
  • Shuffle the order of the skills that were found.
  • Pick the first skill in the shuffled list and check if targets are in range.
    • If there is a target in range, then ready/use this skill
    • If there is no target in range of this skill, move to the next skill and repeat
      • If the skill is a self-buff (IE: Pollen on Bees), it stops here
  • If the end of the skill list is reached, and no targets are in range, then do not use a skill and continue pathing to player.
  • No further attempts are made to use a skill - The checks have failed on all skills.

Side Note: This is slightly different than if a mob has TP. In this case on the next server tick, the mob performs the checks again over and over until it has a skill with a valid target in range (pathing towards the player/target if the ability firing fails).

2 - useMobAbility(####) --- Argument Supplied is a skill ID from mob_skills.sql
The logic performed when this is called on a mob acts like so:

  • Immediately ready the skill, ignoring whether or not I have a target in range.

In both cases, neither guarantees that a mob ability will be used successfully. In the first case, it will just keep pathing to the player. In the second case, currently, it will end up 'stunning' the mob because it will ready the skill and then fail even when a target was not in range when it went to ready the skill.

This PR attempts to remedy the 2nd case only in that it will match the behavior of the 1st case where no targets are available for any skill (aka: It will just continue pathing to the player and not use the specified skill).

The implementation of the drawIn and getAbilityDistance functions from PR #5833 are meant to help with remedying this lack of assurance when calling useMobAbility with an argument. When useMobAbility is called within a script, it would now be possible for the scripter to check for the distance of the ability they are wanting to use and ensure that there is a target within range before calling it, drawing in the target (if required), or just queue it within the lua script instead of messing with the mobs action queue within core.

@claywar claywar merged commit d3978dc into LandSandBoat:base Jun 1, 2024
13 checks passed
@0x05010705 0x05010705 deleted the update-useMobAbility-to-use-range-check branch June 4, 2024 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Gives visibility for reviewers for impacted areas involving cpp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants