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

feat(Core/Combat): Threat & Combat rewrite (source: TC) #12076

Closed
wants to merge 35 commits into from
Closed

feat(Core/Combat): Threat & Combat rewrite (source: TC) #12076

wants to merge 35 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jun 17, 2022

Co-authored-by: Treeston treeston.nmoc@gmail.com
Co-authored-by: Shauren shauren.trinity@gmail.com
Co-authored-by: jackpoz giacomopoz@gmail.com

Resume of global changes:

** Description in progress**

Changes Proposed:

Description come from to the initial Treeston PR:

  • PvE combat is now always mutual. UNIT_FLAG_IN_COMBAT is backed by actual references to the units we're in combat with.
  • PvP combat is now also tracked, and almost always mutual; spells like Vanish and Feign Death can break this rule. That means we can easily determine a list of players we're fighting.
  • By extension, IsInCombatWith now has sensible behavior when invoked on nonplayers.
  • Threat and combat systems are no longer the same.
  • They still have an enforced relationship (threat implies combat - clearing combat clears threat) ...but we can have combat without threat. A creature (with threat list) isn't considered to be engaged until it has an entry on its threat list. Which means we can now faithfully replicate retail engage behavior. Combat on projectile launch - engagement start on projectile impact. Yay for progress!
  • Vehicle threat is now properly pooled on the main vehicle body
  • Various edge case bug fixes for threat redirects (Misdirection "cancelling" Vigilance and similar).
  • Target re-selection is now significantly faster.

Issues Addressed:

TC SOURCE:

Main commits:

Others commits:

Tests To Do or Done:

  • Check the new SelectTarget and SelectTargetList methods
  • Check Dual (borken)
  • Check Pet
  • Check Ancient Event
  • Check DoZonInCombat
  • Check Scripts
  • Check evade system
  • Check spells class modif
  • Check debug command
  • Check behavior when unit/player charmed
  • Check behavior with vehicules
  • Taunt System (work)

Known Issues and TODO List:

  • Logs "CreatureAI::EngagementOver called even though creature is not currently engaged. Creature debug info:
    %s" doesn't return the creature id.
  • Need to find why sometime "EngagementOver called even though creature is not currently engaged."
  • Dots doesn't keep in combat the player
  • Pets doesn't keep in combat the player in Pvp (work well in pve)
  • Import detailed assert in CombatMgr
  • Mind control stop shortly if use in pvp
  • Duel PvP doesn't work. Return target is in combat.
  • When mind control (priest spell) target is able to auto attack
  • Razorgore adds in end of P2 evade but not despawn. So they stay in P3
  • Razorgore P2, if player controlling orb is hit and cancel, the spell is interrupt but he keep the control of the boss

How to Test AzerothCore PRs

When a PR is ready to be tested, it will be marked as [WAITING TO BE TESTED].

You can help by testing PRs and writing your feedback here on the PR's page on GitHub. Follow the instructions here:

http://www.azerothcore.org/wiki/How-to-test-a-PR

REMEMBER: when testing a PR that changes something generic (i.e. a part of code that handles more than one specific thing), the tester should not only check that the PR does its job (e.g. fixing spell XXX) but especially check that the PR does not cause any regression (i.e. introducing new bugs).

For example: if a PR fixes spell X by changing a part of code that handles spells X, Y, and Z, we should not only test X, but we should test Y and Z as well.

Maelthyr and others added 2 commits June 17, 2022 18:49
* list of commits used in PR description

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
@ghost ghost added BREAKING CHANGE Change that will require data re-extraction or major DB table alterations SENSITIVE WIP SENSITIVE Work in Progress, Do not Update, Do not Merge until author approves. CORE Related to the core Script file-cpp Used to trigger the matrix build Waiting to be Tested and removed Waiting to be Tested labels Jun 17, 2022
Maelthyr and others added 21 commits June 19, 2022 19:43
*Only send periodic threat list updates while unit is engaged
*Fix a potential super edge case iterator invalidation
*Split (instead of copied) between all creatures threatened by the action.
*Passive creatures now properly become engaged when adding an offline threat entry.

Cherry-pick:
*TrinityCore/TrinityCore@a08ad9a
*TrinityCore/TrinityCore@b53cbf4
*TrinityCore/TrinityCore@013d456
*TrinityCore/TrinityCore@a373275

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
…n (vanished/invis'd units). For non-PASSIVE units, this will immediately cause an evade.

* Fix taunt logic relying on unspecified behavior by unordered boost heap iterators. Use ordered iterators instead, this is cheap for our use case anyway. This will make taunt behave consistently again.

Cherry-pick:
*TrinityCore/TrinityCore@2402406
*TrinityCore/TrinityCore@71b5ed6
*TrinityCore/TrinityCore@7fe59c9

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
*Now set engaged state for all units on offline reference registration (vanished/invis'd units). For non-PASSIVE units, this will immediately cause an evade.
* Fix taunt logic relying on unspecified behavior by unordered boost heap iterators. Use ordered iterators instead, this is cheap for our use case anyway. This will make taunt behave consistently again.

Cherry-pick:
*TrinityCore/TrinityCore@2402406
*TrinityCore/TrinityCore@71b5ed6
*TrinityCore/TrinityCore@7fe59c9

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
*fix also some build errors and codestyle

Cherry-pick:
*TrinityCore/TrinityCore@fd33b1c
*TrinityCore/TrinityCore@623bc64

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
Co-authored-by: jackpoz <giacomopoz@gmail.com>
* Online states are now re-evaluated before victim update instead of continuously.
* Victim update now happens every 1s as opposed to every server tick unless current target goes away.
* Suppressed threat is no longer re-established until the victim gains additional threat (by hitting the target, for instance).
* Assistance threat is now split between non-controlled units threatened by target, as opposed to all units threatened by target.
*Also new build fixes.

Cherr-pick:
*TrinityCore/TrinityCore@5cea572

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
*send SMSG_THREAT_UPDATE if the threat list actually changed
*Fixed an issue where creatures would not properly assist formation members in certain scenarios.
*Fear no longer suppresses threat. All confuse effects now suppress threat, even ones that wouldn't break on damage.
*Suppressed threat is now re-evaluated on taunt state update, and taunting units can no longer be suppressed.
*Properly sequence checking offline state _after_ adding the reference to the threat list.

Cherry-pick:
*TrinityCore/TrinityCore@408ce48
*TrinityCore/TrinityCore@4e5d1b7
*TrinityCore/TrinityCore@a001bc6
*TrinityCore/TrinityCore@55cb4f9
*TrinityCore/TrinityCore@fdbec8b
*TrinityCore/TrinityCore@9f1755d

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
*Kick engagement logic upstairs to Unit (from ThreatManager), since all Units with AI need it (not just those with threat list).
*Properly unset engaged flag for creates that cannot have a threat list on combat exit.
*Align ThreatList command to TC
*Fix issues by dynamic analysis
*Fix new gcc and clang build errors

Cherrt-pick:
*TrinityCore/TrinityCore@dbe3bbe
*TrinityCore/TrinityCore@f44539b
*TrinityCore/TrinityCore@9942047

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
*Split ThreatManager::NotifyDisengaged off from ThreatManager::ClearAllThreat. NotifyDisengaged signifies intent to clear the engagement flag, and should only be called from AI.

Cherry-pick:
*TrinityCore/TrinityCore@1158f26

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
*Finally move the "is creature engaged" flag to be a property of the creature AI, where it honestly always belonged.
*Properly update offline states even if all threat references are offline.
*Allow refresh pvp combat when assisting a unit that is in pvp combat

Cherry-pick:
*TrinityCore/TrinityCore@c13d837
*TrinityCore/TrinityCore@55ec3bd
*TrinityCore/TrinityCore@372c843

Co-authored-by: Treeston <treeston.nmoc@gmail.com>
Co-authored-by: Jildor
<https://github.com/Jildor>
@ghost ghost marked this pull request as ready for review June 26, 2022 18:01
@acidmanifesto
Copy link
Contributor

Status of PR?

@ghost
Copy link
Author

ghost commented Jul 10, 2022

Still in WIP. Need more tests and fix actual bugs.

@ghost ghost removed the Waiting to be Tested label Jul 20, 2022
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BREAKING CHANGE Change that will require data re-extraction or major DB table alterations CORE Related to the core file-cpp Used to trigger the matrix build Ready to be Reviewed Script SENSITIVE WIP SENSITIVE Work in Progress, Do not Update, Do not Merge until author approves.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite combat and threat system to be mutual from TC
1 participant