Fix no vars if execute is last line & fix code cleanup

Reviewed-on: #20

README.md

@@ -13,9 +13,10 @@
 - Uses Java 11 instead of Java 8
 
 ### Installation
-1. Use Skript 2.5+
+1. Use 1.8+ Minecraft server version.
-2. Use Java 11+
+2. Use Skript 2.5+ (1.8 Skript fork is needed if you're using 1.8)
-3. Put skript-db in plugins folder and restart the server
+3. Use Java 11+ (If you use 1.8, a spigot fork is needed to support Java 11+)
+4. Put skript-db in plugins folder and restart the server
 ### Expression `Data Source` => `datasource`
 Stores the connection information for a data source. This should be saved to a variable in a
  `script load` event or manually through an effect command.
@@ -83,6 +84,18 @@ execute "select %unsafe {columns variable}% from %{table variable}%" in {sql}
 execute unsafe {fully dynamic query} in {sql}
 ```
 
+#### FAQ: How to return sql data in a function?
+You can't because functions don't allow delays, but you can use skript-reflect sections for this:
+```
+on load:
+	create new section stored in {-section::getPlayersFromDatabase}:
+		execute "SELECT uuid FROM table" in {-sql} and store the result in {_result::*}
+		return {_result::uuid::*}
+command /showplayers [<text>]:
+	trigger:
+		run section {-section::getPlayersFromDatabase} async and store result in {_uuids::*} and wait
+		send "%{_uuids::*}%"
+```
 ---
 ### Configuration
 plugins/skript-db/config.yml

pom.xml

@@ -6,7 +6,7 @@
 
     <groupId>com.btk5h</groupId>
     <artifactId>skript-db</artifactId>
-    <version>1.3.5</version>
+    <version>1.3.8</version>
     <packaging>jar</packaging>
 
     <repositories>
@@ -19,8 +19,8 @@
             <url>https://jitpack.io</url>
         </repository>
         <repository>
-            <id>commons-pool2</id>
+            <id>mvnrepository</id>
-            <url>https://mvnrepository.com/artifact/org.apache.commons/commons-pool2</url>
+            <url>https://mvnrepository.com</url>
         </repository>
         <repository>
             <id>PaperMC</id>
@@ -93,14 +93,14 @@
         <dependency>
             <groupId>com.github.SkriptLang</groupId>
             <artifactId>Skript</artifactId>
-            <version>2.6-beta3</version>
+            <version>2.6.1</version>
             <scope>provided</scope>
         </dependency>
         <!-- https://mvnrepository.com/artifact/org.mariadb.jdbc/mariadb-java-client -->
         <dependency>
             <groupId>org.mariadb.jdbc</groupId>
             <artifactId>mariadb-java-client</artifactId>
-            <version>3.0.3</version>
+            <version>3.0.9</version>
             <scope>compile</scope>
         </dependency>
 

src/main/java/com/btk5h/skriptdb/skript/EffExecuteStatement.java

@@ -10,7 +10,6 @@ import com.btk5h.skriptdb.SkriptDB;
 import com.btk5h.skriptdb.SkriptUtil;
 import com.zaxxer.hikari.HikariDataSource;
 import org.bukkit.Bukkit;
-import org.bukkit.entity.Player;
 import org.bukkit.event.Event;
 
 import javax.sql.DataSource;
@@ -61,7 +60,8 @@ public class EffExecuteStatement extends Effect {
     private VariableString var;
     private boolean isLocal;
     private boolean isList;
-    private boolean isSync;
+    private boolean quickly;
+    private boolean isSync = false;
 
     private void continueScriptExecution(Event e, Object populatedVariables) {
         lastError = null;
@@ -83,58 +83,26 @@ public class EffExecuteStatement extends Effect {
         String baseVariable = var != null ? var.toString(e).toLowerCase(Locale.ENGLISH) : null;
         //if data source isn't set
         if (ds == null) return;
-
-        boolean sync = !Bukkit.isPrimaryThread();
-
-        //if current thread is not main thread, then make this query to not have delays
-
         Object locals = Variables.removeLocals(e);
 
         //execute SQL statement
-        CompletableFuture<Object> sql =
+        if (Bukkit.isPrimaryThread()) {
-                CompletableFuture.supplyAsync(() -> executeStatement(ds, baseVariable, query), threadPool);
+            CompletableFuture<Object> sql = CompletableFuture.supplyAsync(() -> executeStatement(ds, baseVariable, query), threadPool);
-
-        //when SQL statement is completed
-        boolean finalSync = sync;
             sql.whenComplete((res, err) -> {
                 if (err != null) {
                     err.printStackTrace();
                 }
-
                 //handle last error syntax data
                 lastError = null;
                 if (res instanceof String) {
                     lastError = (String) res;
                 }
-
-            if (getNext() != null) {
                 //if local variables are present
                 //bring back local variables
-
                 //populate SQL data into variables
-                if (!(res instanceof String)) {
+                if (!quickly) {
-
-                    //also set variables in the sql query complete event
-
-                    //TEMPORARILY DISABLED, AS THIS WOULD WORSEN PERFORMANCE OF THE QUERIES AND NOT BE USED BY MOST PEOPLE.
-                    //I may add config option to enable this later?
-
-                    //SQLQueryCompleteEvent event = new SQLQueryCompleteEvent(this.query.getSingle(e));
-                    //((Map<String, Object>) res).forEach((name, value) -> setVariable(event, name, value));
-                    //SkriptDB.getPlugin(SkriptDB.class).getServer().getPluginManager().callEvent(event);
-                }
-                if (isSync || finalSync) {
-                    if (locals != null) {
-                        Variables.setLocalVariables(e, locals);
-                    }
-                    if (!(res instanceof String)) {
-                        ((Map<String, Object>) res).forEach((name, value) -> setVariable(e, name, value));
-                    }
-                    TriggerItem.walk(getNext(), e);
-                    Variables.removeLocals(e);
-                } else {
                     Bukkit.getScheduler().runTask(SkriptDB.getInstance(), () -> {
-                        if (locals != null) {
+                        if (locals != null && getNext() != null) {
                             Variables.setLocalVariables(e, locals);
                         }
                         if (!(res instanceof String)) {
@@ -142,20 +110,47 @@ public class EffExecuteStatement extends Effect {
                         }
                         TriggerItem.walk(getNext(), e);
                         //the line below is required to prevent memory leaks
-                        //no functionality difference notice with it being removed from my test, but the memory gets filled with leaks
-                        //so it must be kept
                         Variables.removeLocals(e);
                     });
+                } else {
+                    if (locals != null && getNext() != null) {
+                        Variables.setLocalVariables(e, locals);
                     }
+                    if (!(res instanceof String)) {
+                        ((Map<String, Object>) res).forEach((name, value) -> setVariable(e, name, value));
+                    }
+                    TriggerItem.walk(getNext(), e);
+                    //the line below is required to prevent memory leaks
+                    Variables.removeLocals(e);
                 }
             });
+            // sync executed SQL query, same as above, just sync
+        } else {
+            isSync = true;
+            Object resources = executeStatement(ds, baseVariable, query);
+            //handle last error syntax data
+            lastError = null;
+            if (resources instanceof String) {
+                lastError = (String) resources;
+            }
+            //if local variables are present
+            //bring back local variables
+            //populate SQL data into variables
+            if (locals != null && getNext() != null) {
+                Variables.setLocalVariables(e, locals);
+            }
+            if (!(resources instanceof String)) {
+                ((Map<String, Object>) resources).forEach((name, value) -> setVariable(e, name, value));
+            }
+            TriggerItem.walk(getNext(), e);
+            Variables.removeLocals(e);
+        }
     }
 
     @Override
     protected TriggerItem walk(Event e) {
         debug(e, true);
-        //I think no longer needed as of 1.3.0, uncomment if something breaks
+        if (!quickly || !isSync) {
-        if (!isSync) {
             Delay.addDelayedEvent(e);
         }
         execute(e);
@@ -341,7 +336,7 @@ public class EffExecuteStatement extends Effect {
         }
         dataSource = (Expression<HikariDataSource>) exprs[1];
         Expression<?> expr = exprs[2];
-        isSync = matchedPattern == 1;
+        quickly = matchedPattern == 1;
         if (expr instanceof Variable) {
             Variable<?> varExpr = (Variable<?>) expr;
             var = varExpr.getName();

src/main/java/com/btk5h/skriptdb/skript/Types.java

@@ -30,10 +30,6 @@ public class Types {
                         return o.getJdbcUrl();
                     }
 
-                    @Override
-                    public String getVariableNamePattern() {
-                        return "jdbc:.+";
-                    }
                 })
                 .serializer(new Serializer<HikariDataSource>() {
                     @Override