Fix regression in 1.8.3. Updated unit tests.

Author: tomcashman

CHANGES

@@ -1,3 +1,6 @@
+[1.8.4]
+- Fix regression in 1.8.3. Updated unit tests.
+
 [1.8.3]
 - Fix interactive scripts not triggering onScriptBegin
 

core/src/main/java/org/mini2Dx/miniscript/core/InteractiveScriptListener.java

@@ -97,6 +97,10 @@ public void onScriptException(int scriptId, Exception e) {
 		}
 	}
 
+	public ScriptInvocationListener getInvocationListener() {
+		return invocationListener.get();
+	}
+
 	@Override
 	public boolean callOnGameThread() {
 		return false;

core/src/main/java/org/mini2Dx/miniscript/core/ScriptExecutionTask.java

@@ -65,17 +65,11 @@ public ScriptExecutionTask(int taskId, GameScriptingEngine gameScriptingEngine,
 	public void run() {
 		try {
 			if(scriptInvocationListener != null) {
-				if(scriptInvocationListener.callOnGameThread() && !syncCall) {
-					final ScriptBeginNotification beginNotification = new ScriptBeginNotification(scriptInvocationListener, scriptId);
-					scriptingEngine.scriptNotifications.offer(beginNotification);
-
-					while(!beginNotification.isProcessed()) {
-						synchronized(beginNotification) {
-							beginNotification.wait();
-						}
-					}
+				if(scriptInvocationListener instanceof InteractiveScriptListener) {
+					final ScriptInvocationListener actualListener = ((InteractiveScriptListener) scriptInvocationListener).getInvocationListener();
+					invokeOnScriptBegin(actualListener);
 				} else {
-					scriptInvocationListener.onScriptBegin(scriptId);
+					invokeOnScriptBegin(scriptInvocationListener);
 				}
 			}
 
@@ -115,6 +109,24 @@ public void run() {
 		completed.set(true);
 	}
 
+	private void invokeOnScriptBegin(ScriptInvocationListener listener) throws InterruptedException {
+		if(listener == null) {
+			return;
+		}
+		if(listener.callOnGameThread() && !syncCall) {
+			final ScriptBeginNotification beginNotification = new ScriptBeginNotification(listener, scriptId);
+			scriptingEngine.scriptNotifications.offer(beginNotification);
+
+			while(!beginNotification.isProcessed()) {
+				synchronized(beginNotification) {
+					beginNotification.wait();
+				}
+			}
+		} else {
+			listener.onScriptBegin(scriptId);
+		}
+	}
+
 	public void skipScript() {
 		if (taskFuture.isDone()) {
 			return;

core/src/test/java/org/mini2Dx/miniscript/core/AbstractGameScriptingEngineTest.java

@@ -25,6 +25,7 @@
 
 import java.io.InputStream;
 import java.util.concurrent.atomic.AtomicBoolean;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import java.util.concurrent.atomic.AtomicReference;
 
@@ -225,12 +226,19 @@ public boolean callOnGameThread() {
 	public void testInvokeScriptBeginNotificationOnGameThread() throws Exception {
 		final int expectedScriptId = scriptingEngine.compileScript(getDefaultScript());
 		final AtomicBoolean notificationReceived = new AtomicBoolean(false);
+		final AtomicLong notificationThreadId = new AtomicLong(-1);
+		final AtomicBoolean beginResult = new AtomicBoolean(false);
 
 		scriptingEngine.invokeCompiledScript(expectedScriptId, scriptBindings, new ScriptInvocationListener() {
 
 			@Override
 			public void onScriptBegin(int scriptId) {
+				try {
+					Thread.sleep(500);
+				} catch (Exception e) {}
+				scriptBindings.put("beginNotificationResult", true);
 				notificationReceived.set(true);
+				notificationThreadId.set(Thread.currentThread().getId());
 			}
 
 			@Override
@@ -244,6 +252,9 @@ public void onScriptSuccess(int scriptId, ScriptExecutionResult executionResult)
 				} else {
 					scriptResult.set(ScriptResult.SUCCESS);
 					scriptExecuted.set(true);
+
+					//Ensure begin is processed before execution
+					beginResult.set(executionResult.containsKey("beginNotificationResult"));
 				}
 			}
 
@@ -266,6 +277,84 @@ public boolean callOnGameThread() {
 			}
 		});
 		while(!scriptExecuted.get()) {
+			if(!notificationReceived.get()) {
+				try {
+					Thread.sleep(100);
+				} catch (Exception e) {}
+			}
+
+			scriptingEngine.update(1f);
+		}
+		Assert.assertEquals(ScriptResult.SUCCESS, scriptResult.get());
+		Assert.assertEquals(true, gameFuture.isUpdated());
+		Assert.assertEquals(false, gameFuture.waitOccurred());
+		Assert.assertEquals(false, gameFuture.isFutureSkipped());
+		Assert.assertEquals(false, gameFuture.isScriptSkipped());
+		Assert.assertEquals(true, notificationReceived.get());
+		Assert.assertEquals(Thread.currentThread().getId(), notificationThreadId.get());
+	}
+
+	@Test
+	public void testInvokeInteractiveScriptBeginNotificationOnGameThread() throws Exception {
+		final int expectedScriptId = scriptingEngine.compileScript(getDefaultScript());
+		final AtomicBoolean notificationReceived = new AtomicBoolean(false);
+		final AtomicLong notificationThreadId = new AtomicLong(-1);
+		final AtomicBoolean beginResult = new AtomicBoolean(false);
+
+		scriptingEngine.invokeCompiledScript(expectedScriptId, scriptBindings, new ScriptInvocationListener() {
+
+			@Override
+			public void onScriptBegin(int scriptId) {
+				try {
+					Thread.sleep(500);
+				} catch (Exception e) {}
+				scriptBindings.put("beginNotificationResult", true);
+				notificationReceived.set(true);
+				notificationThreadId.set(Thread.currentThread().getId());
+			}
+
+			@Override
+			public void onScriptSuccess(int scriptId, ScriptExecutionResult executionResult) {
+				if(scriptId != expectedScriptId) {
+					scriptResult.set(ScriptResult.INCORRECT_SCRIPT_ID);
+					scriptExecuted.set(true);
+				} else if(!checkExpectedScriptResults(executionResult)) {
+					scriptResult.set(ScriptResult.INCORRECT_VARIABLES);
+					scriptExecuted.set(true);
+				} else {
+					scriptResult.set(ScriptResult.SUCCESS);
+					scriptExecuted.set(true);
+
+					//Ensure begin is processed before execution
+					beginResult.set(executionResult.containsKey("beginNotificationResult"));
+				}
+			}
+
+			@Override
+			public void onScriptSkipped(int scriptId) {
+				scriptResult.set(ScriptResult.SKIPPED);
+				scriptExecuted.set(true);
+			}
+
+			@Override
+			public void onScriptException(int scriptId, Exception e) {
+				e.printStackTrace();
+				scriptResult.set(ScriptResult.EXCEPTION);
+				scriptExecuted.set(true);
+			}
+
+			@Override
+			public boolean callOnGameThread() {
+				return true;
+			}
+		}, 0, true);
+		while(!scriptExecuted.get()) {
+			if(!notificationReceived.get()) {
+				try {
+					Thread.sleep(100);
+				} catch (Exception e) {}
+			}
+
 			scriptingEngine.update(1f);
 		}
 		Assert.assertEquals(ScriptResult.SUCCESS, scriptResult.get());
@@ -274,6 +363,7 @@ public boolean callOnGameThread() {
 		Assert.assertEquals(false, gameFuture.isFutureSkipped());
 		Assert.assertEquals(false, gameFuture.isScriptSkipped());
 		Assert.assertEquals(true, notificationReceived.get());
+		Assert.assertEquals(Thread.currentThread().getId(), notificationThreadId.get());
 	}
 
 	@Test

kotlin/src/test/resources/default.kts

@@ -1,3 +1,4 @@
+val beginNotificationResult: Boolean = false
 val stringValue: String = bindings["stringValue"] as String + "123"
 val intValue: Int = 101
 val booleanValue: Boolean = true