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

Core/Creatures: moved combat pulse into heartbeat handling and implemented CREATURE_STATIC_FLAG_2_FORCE_PARTY_MEMBERS_INTO_COMBAT #29946

Merged
merged 10 commits into from Apr 28, 2024

Conversation

Ovahlord
Copy link
Contributor

@Ovahlord Ovahlord commented Apr 27, 2024

Changes proposed:

  • the combat pulse mechanic is being moved into heartbeat handling and now fires once on engaging a creature
  • This mechanic will now only kick in for creatures that use CREATURE_STATIC_FLAG_2_FORCE_PARTY_MEMBERS_INTO_COMBAT
  • this furthermore no longer binds the mechanic to dungeons but it can now also act in the open world since we now draw our targets from combat references instead of iterating over the entire map.

Tests performed:

  • tested ingame with a 2nd character in a party

Todo

  • The static flag must be set in DB for all (raid) bosses now

…ented CREATURE_STATIC_FLAG_2_FORCE_PARTY_MEMBERS_INTO_COMBAT
@Nyr97
Copy link
Contributor

Nyr97 commented Apr 27, 2024

Are you sure this must be set for all bosses? Asking because I've been recently playing wotlk classic and on low level dungeons, for example, Deadmines, when engaging a boss, I wouldn't be dragged into combat if my team mates engaged it while I was drinking. And as far as I recall, engaging would break drinking and my drinking wasn't being broken either. Could this be a result of a bug from official servers in any case?

@Ovahlord
Copy link
Contributor Author

Ovahlord commented Apr 27, 2024

It is. That static flag is set on pretty much every single boss. including Kologarns arms. And no, engaging does not break drinking.

@Ghaster
Copy link
Contributor

Ghaster commented Apr 27, 2024

Flag was also added (at earliest) very late in Vanilla or very early TBC so there's a chance that earlier Vanilla bosses simply didn't have the flag until later.

@Ovahlord
Copy link
Contributor Author

It applies primarily for raid bosses only. Dungeon bosses are not part of this mechanic for most of the part.

@Nyr97
Copy link
Contributor

Nyr97 commented Apr 27, 2024

I see then, thanks for the clarification!

@Ovahlord Ovahlord marked this pull request as ready for review April 27, 2024 20:43
@Nyr97
Copy link
Contributor

Nyr97 commented Apr 28, 2024

Since you've removed DoZoneInCombat(), this means that, in raid encounters where there are creatures that engage the boss when players do, e.g., Archimonde in Hellfire Citadel or Sylvanas Windrunner in Sanctum of Domination, we're forced to find those creatures and call AttackStart on the bosses, is that it?

@@ -3644,6 +3624,9 @@ void Creature::AtEngage(Unit* target)
ai->JustEngagedWith(target);
if (CreatureGroup* formation = GetFormation())
formation->MemberEngagingTarget(this, target);

// Creatures with CREATURE_STATIC_FLAG_2_FORCE_PARTY_MEMBERS_INTO_COMBAT periodically force party members into combat
ForcePartyMembersIntoCombat();

This comment was marked as resolved.

@Ovahlord
Copy link
Contributor Author

Since you've removed DoZoneInCombat(), this means that, in raid encounters where there are creatures that engage the boss when players do, e.g., Archimonde in Hellfire Citadel or Sylvanas Windrunner in Sanctum of Domination, we're forced to find those creatures and call AttackStart on the bosses, is that it?

DoZoneInCombat only put all players (and their summons) on the map in combat with the target creature. So it was just making sure that when you engage the boss (e.g. Tank start attacking) that all players were put in combat which is only correct for raid bosses and this is now done via static flag and only for party members, not for all players on the map.

@Ovahlord Ovahlord merged commit 11f32a2 into TrinityCore:master Apr 28, 2024
2 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants