RuntimeErrorCatcher + Runtime Elements #7823
Conversation
Absolutionism
requested review from
APickledWalrus
and removed request for
a team
src/main/java/ch/njol/skript/test/runner/ExprRuntimeErrors.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/log/runtime/RuntimeErrorManager.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/log/runtime/RuntimeErrorCatcher.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/log/runtime/RuntimeErrorCatcher.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/log/runtime/RuntimeErrorCatcher.java
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/log/runtime/RuntimeErrorCatcher.java
Outdated
Show resolved
Hide resolved
src/main/java/org/skriptlang/skript/log/runtime/RuntimeErrorCatcher.java
Show resolved
Hide resolved
Absolutionism
changed the title
src/main/java/org/skriptlang/skript/log/runtime/RuntimeErrorCatcher.java
Outdated
Show resolved
Hide resolved
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
||
| @Name("Catch Runtime Errors") | ||
| @Description("Catch any runtime errors produced by code within the section. This is an in progress feature") |
There was a problem hiding this comment.
shouldn't this be an experiment then?
There was a problem hiding this comment.
I had the option to choose between making it an experiment or just adding an "in progress" in the description. I chose the description.
There was a problem hiding this comment.
What might change here? I think syntax that is subject to change would best be put behind an experiment. It might not hurt here either since error catching is a "new" concept for Skript.
There was a problem hiding this comment.
Mainly catching specific types of errors, but that shouldn't be breaking if added in the future.
| import java.util.concurrent.atomic.AtomicBoolean; | ||
|
|
||
| @Name("Catch Runtime Errors") | ||
| @Description("Catch any runtime errors produced by code within the section. This is an in progress feature") |
There was a problem hiding this comment.
What might change here? I think syntax that is subject to change would best be put behind an experiment. It might not hurt here either since error catching is a "new" concept for Skript.
| consumers.addAll(Arrays.asList(newConsumers)); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Removes a {@link RuntimeErrorConsumer} from the tracked list. | ||
| * @param consumer The consumer to remove. |
| * Consumers will be maintained when the manager is refreshed. | ||
| * @param newConsumers The {@link RuntimeErrorConsumer}s to add. | ||
| */ | ||
| public void addConsumers(RuntimeErrorConsumer ... newConsumers) { |
There was a problem hiding this comment.
| public void addConsumers(RuntimeErrorConsumer ... newConsumers) { | |
| public void addConsumers(RuntimeErrorConsumer... newConsumers) { |
|
|
||
| static { | ||
| Skript.registerExpression(ExprCaughtErrors.class, String.class, ExpressionType.SIMPLE, | ||
| "last caught [run[ ]time] errors"); |
There was a problem hiding this comment.
| "last caught [run[ ]time] errors"); | |
| "[the] last caught [run[ ]time] errors"); |
|
|
||
| AtomicBoolean delayed = new AtomicBoolean(false); | ||
| Runnable afterLoading = () -> delayed.set(!getParser().getHasDelayBefore().isFalse()); | ||
| trigger = loadCode(sectionNode, "runtime", afterLoading, Event.class); |
There was a problem hiding this comment.
Given that the event context is not changing, you should not load this code into a trigger. You can simply load it as normal and check the updated delay value in ParserInstance. As for walk, to execute the section, you can do something like:
last.setNext(null);
TriggerItem.walk(first, event);first and last are available from TriggerSection and represent the first and last elements of the section, respectively.
| /** | ||
| * Gets all the cached {@link RuntimeError}s. | ||
| */ | ||
| public @Unmodifiable List<RuntimeError> getCachedErrors() { |
There was a problem hiding this comment.
I would annotate this more specifically as @UnmodifiableView (per the docs on how the collections method functions)
Description
This PR aims to add a
RuntimeErrorCatcherallowingRuntimeErrorsprinted fromRuntimeErrorManagerto be caught and logged.With this addition, allows for a
SecCatchErrorsandExprCaughtErrorsto create code that catches errors, preventing them from being logged into terminal, and accessing the errors that were caught.In the related issue (by me), I realized that
SecParsenever ran the code within the section, so I decided to make a new section and expression that waySecParsecould maintain it's behavior.Target Minecraft Versions: any
Requirements: none
Related Issues: #7766