moderocky / byteskript Goto Github PK

How can I make custom events?

Issue Description

When unloading a script using Skript#unloadScript(Script), one would expect all traces of said script including its code to be removed, such that a new script with the same name can be loaded without problems. Currently, this is not the case: the loaders field of the Skript instance is not updated when a script is unloaded.

Reproducing the Issue

1. Load a script
2. Unload the script
3. Observe the loaders list in the Skript instance

Effects

• Old scripts are never truly deleted until the Skript instance is garbage collected or the program is restarted
• Classes from scripts that have the same name as an older script from the same session will not be available — when requested, the older classes will be returned. This also means that as long as the same Skript instance is kept, scripts cannot be "reloaded"; doing so would simply cause both scripts' class loaders to exist simultaneously, and since the older script's class loader has a lower index in the loaders list, its classes are always preferred.

Not so simple!

This problem would be easy to fix if each class loader in the loaders array represented one script file. If this was the case, one could simply iterate loaders, look for the class loader that matches the class loader of the main class of the script, and then remove said class loader from the script, done trivially with the built-in removeIf(Predicate<T>) method. Sadly, though, loaders class loaders do not necessarily have a one-to-one correspondence with script files — many methods in Skript explicitly load PostCompileClasses by calling createLoader() for each class, even when those classes come from the same script.

Solution

To solve this issue properly (and thus avoid unnecessary calls to createLoader(), create a more strict correspondence between class loaders and scripts, and allow for hot-swapping (reloading) of scripts), two things must be done:

• In places where several calls to Skript#loadClass(String, byte[]) are issued, one call to createLoader() and then multiple calls to SkriptMirror#loadClass(String, byte[]) should be issued instead.
• In Skript#unloadScript(Script), the class loader that represents the script being unloaded should be properly removed from the list of active class loaders.

Describe the bug
Compiled jars don't run and just give an error.

To Reproduce
Steps to reproduce the behavior:

1. Create a bsk file and write some code. I've used the code below

on load:
    trigger:
        print "hello"

2. Use the jar command to create a jar.
3. Run the new jar
4. See error

Expected behavior
It should work the same as when you use the run command by printing "hello"

Screenshots

Desktop (please complete the following information):

• OS: Windows 11
• Version: latest

Describe the bug
When a script containing events is unloaded, an UnsupportedOperationException is thrown by sun.misc.Unsafe, as ByteSkript attempts to acquire the field offset on a record class, specifically InvokingScriptRunner.

To Reproduce
Steps to reproduce the behaviour:

1. Unload a script that contains an event using Skript#unloadScript(Script)
2. See error

Expected behavior
The script should be unloaded without errors.

Errors

Caused by: java.lang.UnsupportedOperationException: can't get field offset on a record class: private final java.lang.Class org.byteskript.skript.runtime.internal.InvokingScriptRunner.owner
  at jdk.unsupported/sun.misc.Unsafe.objectFieldOffset(Unsafe.java:648)
  at BSFP-0.1.0-dev-all.jar//org.byteskript.skript.runtime.UnsafeAccessor.graveyard(UnsafeAccessor.java:48)
  at BSFP-0.1.0-dev-all.jar//org.byteskript.skript.runtime.Skript.unloadScript(Skript.java:731)
  at BSFP-0.1.0-dev-all.jar//blue.lhf.bsfp.BSFP.loadScript(BSFP.java:115)
  at BSFP-0.1.0-dev-all.jar//blue.lhf.bsfp.BSFP.loadScriptTree(BSFP.java:151)
  at BSFP-0.1.0-dev-all.jar//blue.lhf.bsfp.BSFP.lambda$loadScriptTreeAsync$7(BSFP.java:187)
  at BSFP-0.1.0-dev-all.jar//blue.lhf.bsfp.util.MayThrow$Supplier.get(MayThrow.java:10)

Additional context
I'm running ByteSkript v1.0.24. Additionally, this issue can be solved by, for example, checking if the object being graveyarded is an instance of Record in org.byteskript.skript.runtime.UnsafeAccessor.graveyard(Object)

All syntax should have individual tests.
These will be stored in resources/tests and run automatically.
These may replace the existing bulk tests.

These are designed to (significantly) overlap, which will increase test coverage and catch potential edge cases.

• Comparisons
• Configs
• Control
• Dictionary
• Entries
  • Syntax
• Events
• Flow
  • Conditionals
  • Errors
  • Execution
  • Lambdas
  • Loops
• Functions
• Generic
• Lists
• Literals
• Maps
• Maths
• Scripts
• Tests
• Timings
• Types
  • Properties
• Variables

Describe the bug
When a loop is nested within another section (the length of the sections array in the context is 2 or more), the loop fails to balance the file tree at section exit. This is due to a mismatch between the owner metadata assigned to the tree by the loop section and the owner metadata it expects when exiting

To Reproduce
Steps to reproduce the behavior:

1. Load this script:

on script load:
  trigger:
    if 1 is 1:
      set {ctr} to 0

      // Any section that builds the tree with context.getSection(1) and exits expecting context.getSection()
      // to be the owner of the tree causes the exception. Affects at least While, LoopTimes, and LoopIn.
      while {ctr} is less than 10:
        add 1 to {ctr}

2. Notice ScriptCompileError: Unable to balance ___ flow tree. (Line 9)

Expected behaviour
The script should compile and effectively be a no-op, since no visual output is given.

Additional Context
I'm using ByteSkript v1.0.21.

Is your feature request related to a problem? Please describe.
Currently all events, functions, etc require a trigger: section to execute the code.
Even if the trigger: section is the only member, it's still required.

Describe the solution you'd like
Make trigger: implicit if there are no other members.
Example:

on load:
    trigger:
        print "hello"

Would become:

on load:
    print "hello"

Describe alternatives you've considered
I've thought about making a preprocessor to automatically generate trigger: sections when needed.

Is your feature request related to a problem? Please describe.
The fact that the external function expression (ExprFunctionExternal) and function property expression (ExprFunctionProperty) are distinguished only by the use of non-identifier symbols in the function source is limiting — for example, function() from cool expression is parsed as a usage of the external function expression, despite the source, cool expression, being an expression, and not a valid script or library.

Describe the solution you'd like
I propose that the syntax for the external function expression be changed to include a syntax specifier, something like function(...) from source %Source%. This way, the function property expression would take precedence when the source is an expression or a valid Object, allowing for more concise function calls, like function() from cool expression in place of

set {x} to cool expression
function() from {x}

Describe alternatives you've considered
Another possible alternative would be to include a keyword, like external function, in front of the existing syntax for the external function expression. This is also valid, though the syntax may become quite lengthy. A third alternative would be to add a syntax specifier to the function property expression, instead. This is also valid, though arguably a worse solution because the function property expression is more common.

Describe the bug
When StringLiteral matches some input, it does not do bounds checks prior to calling String#substring(int, int) to get the potential content of the string literal. For obvious reasons, this fails for inputs of length 1 — the result is an attempted substring from index 1 inclusive to index 0 exclusive.

To Reproduce
Steps to reproduce the behavior:

1. Load this script:

on script load:
  trigger:
    // This is initially parsed as a less-than comparison,
    // causing the parser to attempt to match ", which breaks
    // string literal's content matching
    set {foo} to "Hello " + "<there!"

2. Notice the ScriptCompileError: An unknown error occurred while compiling, caused by a SIOOBE when matching " in StringLiteral.
3. Realise that you cannot set {foo} to "Hello <there!", and become overwhelmed with panic.
   Expected behavior
   The input should compile as the addition of the string literals "test" and "<hello".

Additional context
I'm running ByteSkript version 1.0.21.

Suggested changes
Add a guard condition to the matcher, which returns null if the length of the string is less than 2.

Is your feature request related to a problem? Please describe.
In its current state, the requiresMainThread setting for expressions is never respected. In fact, the term "requiresMainThread" only occurs twice in the entirety of the ByteSkript source code: once in SyntaxElement.java, where it is defined, and once in ExprSystemInput.java, which is the only built-in expression to use the feature.

Describe the solution you'd like
The requiresMainThread setting should cause not only the expression's compiled code but also its handlers to be wrapped using a mechanism that allows the code to run only on the main thread of execution.

Describe alternatives you've considered
Not applicable.

Additional context
Project maintainers are likely already aware of the lack of implementation for this feature. This issue is not a feature request per se, rather it is meant to express the demand for the feature along with sparking discussion regarding possible implementation details.

Is your feature request related to a problem? Please describe.
Currently, properties must be valid ByteSkript identifiers. For most properties, this is fine, but consider something like the last login date of a Minecraft player. How might this be accessed in ByteSkript, given a player object? There are two existing solutions to this problem:

1. A custom expression. It's clear why this is undesirable: it's tedious, it requires a lot of boilerplate, and it duplicates the standard property syntax.
2. Use a non-english identifier as the property name. For the last login date property, lastlogindate could be used. This is obviously also suboptimal. ByteSkript is supposed to help you program with the power of words, not word.

Describe the solution you'd like
Allow for spaces or other non-identifier characters in property names.

Describe alternatives you've considered
See above. Alternatives exist, but they suck.

Additional context
Care must be taken when implementing this — I'm not sure of the implications a change like this would have on ByteSkript's pattern matching.