Skip to content

Core/Spells: Implemented priority-based target sorting via PriorityRules system #30458

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.

Sign up for GitHub

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

Jump to bottom
Merged
Shauren merged 2 commits into from
Sep 6, 2025
Merged

Core/Spells: Implemented priority-based target sorting via PriorityRules system #30458

Shauren merged 2 commits into from
Sep 6, 2025

Conversation

@Nyr97
Copy link
Contributor

@Nyr97 Nyr97 commented Nov 22, 2024 •
edited
Loading

Introduces a lightweight, flexible system to sort spell targets based on script-defined priority rules. This replaces hardcoded or purely random selection logic with a more precise, predictable approach using weighted scoring.

Changes proposed:

  • Added PriorityRules structure to define scoring conditions with weights.
  • Implemented SortTargetsWithPriorityRules():
  1. Scores all eligible targets based on provided rules.
  2. Sorts by descending score.
  3. Shuffles tied entries around the cutoff to avoid deterministic bias.
  • SortTargetsWithPriorityRules now requires an explicit rule set, improving clarity and avoiding hidden logic.

Tests performed:

  • Verified Power Word: Radiance correctly prioritizes groupmates with no Atonement before other targets.
  • Confirmed deterministic behavior no longer occurs on repeated casts with tied scores.

Known issues and TODO list:

None.

@Nyr97 Nyr97 changed the title Core/Spells: Improve SelectRandomInjuredTargets for better efficiency and extendability Core/Spells: Refactor SelectRandomInjuredTargets into SortTargetsWithPriorityRules Nov 23, 2024
@Nyr97
Copy link
Contributor Author

Nyr97 commented Jul 7, 2025

not sure how to ask for a review so I'm pinging @Shauren or @mdX7

@Shauren
Copy link
Member

Shauren commented Jul 7, 2025

I don't like this

  • Doesn't make sense as Unit members (its a spell func)
  • Always selects the same targets on every cast (if their classification doesnt change between casts - sorting is now deterministic, first by priority then by the order they were added to grid)
  • Uses std::function

@Nyr97 Nyr97 changed the title Core/Spells: Refactor SelectRandomInjuredTargets into SortTargetsWithPriorityRules Core/Spells: Implemented priority-based target sorting via PriorityRules system Jul 7, 2025
@Nyr97
Copy link
Contributor Author

Nyr97 commented Jul 7, 2025 •
edited
Loading

Moved it to Spell class inside Trinity namespace, made sorting less deterministic by randomizing equal-scored entries and restored old SelectRandomInjured stuff, also updated PR desc

@Faq
Copy link
Contributor

Faq commented Jul 8, 2025

Perhaps when rewriting such system logic, could add some tests?

@Nyr97
Copy link
Contributor Author

Nyr97 commented Jul 8, 2025

Description clearly states I tested it on Radiance.

@Shauren Shauren force-pushed the SelectRandomInjuredTargets_improvements branch from 3a8c090 to 73c57d4 Compare September 5, 2025 19:04
@Shauren Shauren force-pushed the SelectRandomInjuredTargets_improvements branch 4 times, most recently from 99f072f to 629f42f Compare September 5, 2025 23:58
@Shauren
Core/Spells: Simplify SortTargetsWithPriorityRules
c206d15 
* Remove manul weight assignment
* Removed std::vector alloc
@Shauren Shauren force-pushed the SelectRandomInjuredTargets_improvements branch from 629f42f to c206d15 Compare September 6, 2025 10:28
@Shauren Shauren merged commit 03d072d into TrinityCore:master Sep 6, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

Branch-master

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Nyr97 @Shauren @Faq