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

Fix range check compatibility mode when range finder frame is already visible #293

Merged
merged 1 commit into from
Oct 24, 2023

Conversation

opatut
Copy link
Contributor

@opatut opatut commented Oct 24, 2023

My guild is having issues in Wotlk Classic ICC Blood Prince Council with the range finder not showing players in range.

I believe this is due to the BPCouncil Mod choosing 12y range, and that not being a valid range in a restricted area such as a raid.

I think it is caused by the check I modified in this PR. If someone has the frame visible at the start of the fight (with maybe 8 yards selected), then at the time the mod choses 12 yards for the range, the frame is already visible and the compatibility conversion is not being done. The conversion of the range should only depend on restrictionsActive, not on not textFrame:IsShown(), i. e. be moved before the if statement in L898.

I have validated the behaviour and the fix in a 2-player party in Violet Hold, I assume it will work the same in the raid.

@MysticalOS MysticalOS merged commit 1d4df10 into DeadlyBossMods:master Oct 24, 2023
2 checks passed
@MysticalOS
Copy link
Member

Merged but also changed mod to 13, because per https://www.wowhead.com/spell=72037/shock-vortex it is 13 radius anyways and 13 is a valid input.

@opatut
Copy link
Contributor Author

opatut commented Oct 25, 2023

Thanks for merging so quickly (and for maintaining the tool in the first place).

I believe 12 was "correct", as the actual ability is this: https://www.wowhead.com/spell=72038/empowered-shock-vortex The one you linked is the single target spell when Valanar is not active, which has its own warning and special warning, but the /range is needed for the "empowered" version.

@MysticalOS
Copy link
Member

That must be why i went with 12 originally. either way, 1 yard too big is better than 1 too small and giving false sense of safety

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants