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

Feat(API): Mutable Location in PlayerTeleportToPlotEvent #4196

Merged
merged 3 commits into from
Nov 21, 2023

Conversation

PierreSchwang
Copy link
Member

Overview

In case of some more abstract plot layouts (e.g. floating balloons in the sky), the event now provides the ability to set the location for any teleport programmatically.

Example:

@Subscribe
public void scootRightOver(PlayerTeleportToPlotEvent event) {
    if (event.getCause() == TeleportCause.COMMAND_HOME) {
        event.setLocation(event.getLocation().add(-15, 0, 0));
    }
}

I did not deprecate the constructor nor the event dispatcher method itself. Events should not be called by end-users either way. If no location is passed, weird behavior can occur when trying to mutate the original location (which is null in case of those deprecated methods)

Submitter Checklist

@PierreSchwang PierreSchwang requested a review from a team as a code owner October 2, 2023 16:10
@github-actions github-actions bot added the Feature This PR proposes a new feature label Oct 2, 2023
@dordsor21
Copy link
Member

dordsor21 commented Oct 3, 2023

I wonder if two events for teleport would be better. If the event is cancelled it would be good to not search for a location and possibly load chunks needlessly. Perhaps add a new "PlayerTeleportToPlotLocationChosenEvent" :D

@PierreSchwang
Copy link
Member Author

I wonder if two events for teleport would be better. If the event is cancelled it would be good to not search for a location and possibly load chunks needlessly. Perhaps add a new "PlayerTeleportToPlotLocationChosenEvent" :D

I may have an idea to mitigate that issue. Tbh, I'm not that big of a fan for an additional event (especially with that very java-ish name :D)

@PierreSchwang
Copy link
Member Author

PierreSchwang commented Oct 3, 2023

I wonder if two events for teleport would be better. If the event is cancelled it would be good to not search for a location and possibly load chunks needlessly. Perhaps add a new "PlayerTeleportToPlotLocationChosenEvent" :D

Should be better now I'd say - usage would be now:

@Subscribe
public void yeet(PlayerTeleportToPlotEvent event) {
    event.setLocationTransformer(origin -> origin.add(-15, 0, 0));
}

Therefore, only if the event is not cancelled, the transformer is called and no unnecessary chunk loads would be performed.

Obviously, a new Location instance may be returned instead of calling methods on the origin Location

Copy link
Member

@SirYwell SirYwell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. The only thing we could potentially change is to make it non-null and use UnaryOperator.identity() as default value, but I don't have a strong opinion on that.

@dordsor21 dordsor21 merged commit eb63e43 into main Nov 21, 2023
7 checks passed
@dordsor21 dordsor21 deleted the feat/mutablePlotTeleportLocation branch November 21, 2023 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature This PR proposes a new feature hacktoberfest-accepted
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants