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

Introduce 'Try & Catch' Section for Handling Exceptions #55

Open
NotSoDelayed opened this issue May 23, 2022 · 1 comment
Open

Introduce 'Try & Catch' Section for Handling Exceptions #55

NotSoDelayed opened this issue May 23, 2022 · 1 comment
Labels
enhancement New feature or request help wanted Extra attention is needed priority: low

Comments

@NotSoDelayed
Copy link

NotSoDelayed commented May 23, 2022

Is your feature request related to a problem? Please describe.
When I want to perform a try & catch for a list of methods, I would have to append try infront for each, which may look messy when wanted to handle exceptions from multiple methods in one go.
e.g.

try SomeObject.someMethod(0)
try SomeObject.someMethod(1)
try SomeObject.someMethod(2)

Describe the solution you'd like
Introduce a new section node for try & catch, and additional local expression to access thrown exception from this section.
e.g.

try:
    SomeObject.someMethod(0)
    SomeObject.someMethod(1)
    SomeObject.someMethod(2)
catch:
    exception.printStackTrace()
    send "Oh no! %exception.getMessage()%" to console

Describe alternatives you've considered
Appending try to each methods and handle them individually, which doesn't sound fun to do in certain cases.

Additional context
None.

@TPGamesNL
Copy link
Member

I think it's a good suggestion, I've thought about adding this before. The problem is that Skript's execution model (i.e. TriggerItem#walk and its implementation) doesn't allow for this naturally, without causing issues regarding delays.

Reason: take the line broadcast "Test: %MyClass.myMethod()%". What you would expect from Java (and what I would want to implement) is that as soon as myMethod throws an exception, nothing is broadcast (the effect isn't executed, or at least not completely executed). This can be achieved by making the expression's get method throw an exception. This exception should then be caught by the try section, but here's why the issue lies.

Skript code is executed sequentially, each statement executed returns which statement should be executed next. A section is therefore executed by returning the first statement in the section. However, with this approach, the section no longer has control over the execution of the statement and can therefore not catch any thrown exceptions in it.

Another way to execute a section is to manually start a new TriggerItem sequence from the first element, in which case you can control the execution and catch exceptions. The problem with this method is the implementation of a delay in Skript: it stops the current trigger flow (it exits the statement iteration) and starts a new trigger flow from the next statement at the point in time where this is needed. Because of this, if we implement the try section to start a new trigger flow, a delay in the try section will make the try section exit its execution and think the section is over while it isn't, though this issue can be worked around. However, we are not able to give the same execution control to the code after the delay, because it is the delay starting the trigger flow, not the try section.

While I could disallow delays in the try section, it wouldn't make much sense to users and it wouldn't work reliably (because plenty of addons don't properly mark triggers as delayed during loading).

If anyone has an idea for implementing this, please let me know (PS I probably don't want to implement this by modifying the bytecode of Skript to change the implementation of exception catching in trigger execution for this).

I'm leaving this open for implementation suggestions.

@TPGamesNL TPGamesNL added enhancement New feature or request priority: low labels May 23, 2022
@TPGamesNL TPGamesNL added the help wanted Extra attention is needed label Aug 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed priority: low
Projects
None yet
Development

No branches or pull requests

2 participants