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

Fixes an exception from being thrown on async events #5699

Merged
merged 14 commits into from
Sep 9, 2023

Conversation

TheLimeGlass
Copy link
Contributor

@TheLimeGlass TheLimeGlass commented May 17, 2023

Description

An exception gets thrown when attempting to execute a required synchronous task like setting a block or executing a console command on an asynchronous event.

Because of this, i've added an override-able method in SkriptEvent for API developers to change how it performs, and should acknowledge that it then becomes the users fault for exceptions when running an asynchronous event with synchronous executes.

I literally have no clue how this was not caught sooner. I wonder if Paper has started throwing exceptions rather than silent. I could not reproduce the issue, but this solution will work.

JUnit tests currently fail, because there is a loss of context bug that is fixed in #5219 waiting on that merge before continuing.


Target Minecraft Versions: any
Requirements: none
Related Issues: #5689

@TheLimeGlass TheLimeGlass added the bug An issue that needs to be fixed. Alternatively, a PR fixing an issue. label May 17, 2023
@TheLimeGlass TheLimeGlass marked this pull request as draft May 17, 2023 03:18
@AyhamAl-Ali AyhamAl-Ali mentioned this pull request Jul 14, 2023
1 task
@sovdeeth sovdeeth added the 2.7 Targeting a 2.7.X version release label Sep 4, 2023
@APickledWalrus APickledWalrus removed the 2.7 Targeting a 2.7.X version release label Sep 5, 2023
@TheLimeGlass TheLimeGlass marked this pull request as ready for review September 5, 2023 21:33
@APickledWalrus APickledWalrus added the 2.7 Targeting a 2.7.X version release label Sep 6, 2023
Copy link
Member

@APickledWalrus APickledWalrus left a comment

Choose a reason for hiding this comment

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

There is also another triggerEvent#check call on line 123 (can't comment on it)

src/main/java/ch/njol/skript/SkriptEventHandler.java Outdated Show resolved Hide resolved
src/main/java/ch/njol/skript/SkriptEventHandler.java Outdated Show resolved Hide resolved
@APickledWalrus APickledWalrus merged commit 9c82fed into master Sep 9, 2023
@APickledWalrus APickledWalrus deleted the fix/async-events branch September 9, 2023 00:31
AyhamAl-Ali pushed a commit to AyhamAl-Ali/Skript that referenced this pull request Sep 9, 2023
Moderocky added a commit that referenced this pull request Sep 9, 2023
Fixes an exception from being thrown on async events (#5699)

Co-authored-by: LimeGlass <[email protected]>
Co-authored-by: Moderocky <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.7 Targeting a 2.7.X version release bug An issue that needs to be fixed. Alternatively, a PR fixing an issue.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants