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

hotfix for magiport #110

Closed
wants to merge 1 commit into from
Closed

hotfix for magiport #110

wants to merge 1 commit into from

Conversation

thmsndk
Copy link
Contributor

@thmsndk thmsndk commented Feb 21, 2024

give magiport a large range, we should handle .global skills and ignore the range check if they are global

give magiport a large range, we should handle .global skills and ignore the range check if they are global
@FreezePhoenix
Copy link
Contributor

FreezePhoenix commented Feb 21, 2024

Isn't magiport intentionally cross-map? I don't see what adding a range value contributes.

@thmsndk
Copy link
Contributor Author

thmsndk commented Feb 21, 2024

@FreezePhoenix It is, but the isTargetTooFar check currently defaults to player range, without respecting .global
This was an easy hotfix to do to make it work again, a proper PR to handle .global should be made :)

As a result players using magiport currently receives this error
image

@FreezePhoenix
Copy link
Contributor

@earthiverse You broke magiport.
@kaansoral A fix for this should be made before servers are restarted.

@FreezePhoenix
Copy link
Contributor

This PR is rendered inconsequential by 1e43904

@thmsndk
Copy link
Contributor Author

thmsndk commented Feb 21, 2024

Yeah, I was too afraid to mess anymore with the logic in socket.on("skill" when I don't have the ability to test it and get the full understanding of it :)

Closing this PR :)

@thmsndk thmsndk closed this Feb 21, 2024
@thmsndk thmsndk deleted the patch-4 branch May 8, 2024 21:16
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.

2 participants