Core/Spells: Remove the unnecessary method ProcEventInfo::GetProcTarget to avoid confusion.

[r4d1sh]

As I understand from studying the code, ProcEventInfo::GetProcTarget() was originally created for just three uses in aura handlers like SPELL_AURA_PROC_TRIGGER_DAMAGE. However, people started using it in scripts, which led to significant confusion.

Since this method can return either the actor or the action target depending on the context, it creates a lot of ambiguity, making the code difficult to understand and maintain.

Let’s remove this method permanently. Always use ProcEventInfo::GetActor() and ProcEventInfo::GetActionTarget() in your scripts.

Changes proposed:

• Remove ProcEventInfo::GetProcTarget
• Replace its usage everywhere with the more descriptive ProcEventInfo::GetActor() and ProcEventInfo::GetActionTarget()

Tests performed:

Builded with clang, not tested in-game.

[Ovahlord]

ActionTarget and Actor both sound unrelated to procs, which is why people went for GetProcTarget.

So personally, I would rather kill GetActionTarget and rename GetActor into GetProcOwner or sth the like.

So we would end up with GetProcTarget (whoever is target of the proc (including self procs)
And GetProcOwner to identify who is the unit that has the proc aura.

Going with actor and actiontarget could potentially lead to lots of coalescing when it doesnt matter if the actor is also the target

[Shauren]

ActionTarget and Actor both sound unrelated to procs, which is why people went for GetProcTarget.

So personally, I would rather kill GetActionTarget and rename GetActor into GetProcOwner or sth the like.

So we would end up with GetProcTarget (whoever is target of the proc (including self procs) And GetProcOwner to identify who is the unit that has the proc aura.

Going with actor and actiontarget could potentially lead to lots of coalescing when it doesnt matter if the actor is also the target

No.

Actor is just a generic name for the object doing the action, caster/attacker (if not a spell)/jumper (jump flag)
ActionTarget is obvious

GetProcTarget (being killed) is confusing because its intent was to "use the return value of this function for targets in your procs" - which is just a wrong assumption, you can have a DONE proc that does extra damage on target (proctarget points to actiontarget) or a DONE proc that gives you a buff (proctarget still points to actiontarget)

[r4d1sh]

ActionTarget and Actor both sound unrelated to procs, which is why people went for GetProcTarget.

So personally, I would rather kill GetActionTarget and rename GetActor into GetProcOwner or sth the like.

So we would end up with GetProcTarget (whoever is target of the proc (including self procs) And GetProcOwner to identify who is the unit that has the proc aura.

Going with actor and actiontarget could potentially lead to lots of coalescing when it doesnt matter if the actor is also the target

Let's say there is an aura that procs when its owner takes damage. In your version, the unit that dealt the damage would be obtained through "GetProcTarget", even though it is not the target. This causes further confusion.

I think we should continue using the concept of "action"