From 51c461febc57a574f09c60d9cc460351b327993a Mon Sep 17 00:00:00 2001 From: gnembon Date: Sun, 20 Dec 2020 20:42:09 -0400 Subject: [PATCH 1/3] Note about callin `game_tick()` inside the game tick. --- docs/scarpet/api/Auxiliary.md | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/docs/scarpet/api/Auxiliary.md b/docs/scarpet/api/Auxiliary.md index 49a24255..3db7a3df 100644 --- a/docs/scarpet/api/Auxiliary.md +++ b/docs/scarpet/api/Auxiliary.md @@ -406,9 +406,24 @@ averaging. ### `game_tick(mstime?)` -Causes game to run for one tick. By default runs it and returns control to the program, but can optionally -accept expected tick length, in milliseconds. You can't use it to permanently change the game speed, but setting -longer commands with custom tick speeds can be interrupted via `/script stop` command +Causes game to run for one tick. By default, it runs it and returns control to the program, but can optionally +accept expected tick length, in milliseconds, waits that extra remaining time and then returns the control to the program. +You can't use it to permanently change the game speed, but setting +longer commands with custom tick speeds can be interrupted via `/script stop` command - if you can get access to the +command terminal. + +Running `game_tick()` as part of the code that runs within the game tick itself is generally a bad idea, +unless you know what this entails. Triggering the `game_tick()` will cause the current (shoulder) tick to pause, then run the internal tick, +then run the rest of the shoulder tick, which may lead to artifacts in between regular code execution and your game simulation code. +If you need to break +up your execution into chunks, you could queue the rest of the work into the next task using `schedule`, or perform your actions +defining `__on_tick()` event handler, but in case you need to take a full control over the game loop and run some simulations using +`game_tick()` as the way to advance the game progress, that might be the simplest way to do it, +and triggering the script in a 'proper' way (there is not 'proper' way, but via commmand line, or server chat is the most 'proper'), +would be the safest way to do it. For instance, running `game_tick()` from a command block triggered with a button, or in an entity + event triggered in an entity tick, may technically +cause the game to run and encounter that call again, causing stack to overflow. Thankfully it doesn't happen in vanilla running +carpet, but may happen with other modified (modded) versions of the game. 
 loop(1000,game_tick())  // runs the game as fast as it can for 1000 ticks

From faa0c65af4da5f465d0a96b609378d6c7c8eab3f Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" 
Date: Mon, 21 Dec 2020 00:42:52 +0000
Subject: [PATCH 2/3] Merge docs for 'Note about callin  inside the game tick.'

---
 docs/scarpet/Full.md | 21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/docs/scarpet/Full.md b/docs/scarpet/Full.md
index 035c4560..b5aec1b9 100644
--- a/docs/scarpet/Full.md
+++ b/docs/scarpet/Full.md
@@ -4860,9 +4860,24 @@ averaging.
 
 ### `game_tick(mstime?)`
 
-Causes game to run for one tick. By default runs it and returns control to the program, but can optionally 
-accept expected tick length, in milliseconds. You can't use it to permanently change the game speed, but setting 
-longer commands with custom tick speeds can be interrupted via `/script stop` command
+Causes game to run for one tick. By default, it runs it and returns control to the program, but can optionally 
+accept expected tick length, in milliseconds, waits that extra remaining time and then returns the control to the program.
+You can't use it to permanently change the game speed, but setting 
+longer commands with custom tick speeds can be interrupted via `/script stop` command - if you can get access to the 
+command terminal.
+
+Running `game_tick()` as part of the code that runs within the game tick itself is generally a bad idea, 
+unless you know what this entails. Triggering the `game_tick()` will cause the current (shoulder) tick to pause, then run the internal tick, 
+then run the rest of the shoulder tick, which may lead to artifacts in between regular code execution and your game simulation code.
+If you need to break
+up your execution into chunks, you could queue the rest of the work into the next task using `schedule`, or perform your actions
+defining `__on_tick()` event handler, but in case you need to take a full control over the game loop and run some simulations using 
+`game_tick()` as the way to advance the game progress, that might be the simplest way to do it, 
+and triggering the script in a 'proper' way (there is not 'proper' way, but via commmand line, or server chat is the most 'proper'),
+would be the safest way to do it. For instance, running `game_tick()` from a command block triggered with a button, or in an entity
+ event triggered in an entity tick, may technically
+cause the game to run and encounter that call again, causing stack to overflow. Thankfully it doesn't happen in vanilla running 
+carpet, but may happen with other modified (modded) versions of the game.
 
 
 loop(1000,game_tick())  // runs the game as fast as it can for 1000 ticks

From d99ab4fc240fc8e8a3d9f42df599be233d68add7 Mon Sep 17 00:00:00 2001
From: gnembon 
Date: Sun, 20 Dec 2020 22:58:41 -0400
Subject: [PATCH 3/3] limit depth of game_tick, just in case.

---
 .../carpet/script/CarpetScriptServer.java     |  1 +
 .../java/carpet/script/api/Auxiliary.java     | 40 ++++++++++++-------
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/src/main/java/carpet/script/CarpetScriptServer.java b/src/main/java/carpet/script/CarpetScriptServer.java
index 7f0a3f2c..f6f47d2d 100644
--- a/src/main/java/carpet/script/CarpetScriptServer.java
+++ b/src/main/java/carpet/script/CarpetScriptServer.java
@@ -51,6 +51,7 @@ public class CarpetScriptServer
     public  Set unloadableModules;
     public long tickStart;
     public boolean stopAll;
+    public int tickDepth;
     private  Set holyMoly;
     public  CarpetEventServer events;
 
diff --git a/src/main/java/carpet/script/api/Auxiliary.java b/src/main/java/carpet/script/api/Auxiliary.java
index eeefd681..cb8dbee4 100644
--- a/src/main/java/carpet/script/api/Auxiliary.java
+++ b/src/main/java/carpet/script/api/Auxiliary.java
@@ -634,30 +634,40 @@ public class Auxiliary {
 
 
         expression.addLazyFunction("game_tick", -1, (c, t, lv) -> {
-            ServerCommandSource s = ((CarpetContext)c).s;
+            CarpetContext cc = (CarpetContext)c;
+            ServerCommandSource s = cc.s;
             if (!s.getMinecraftServer().isOnThread()) throw new InternalExpressionException("Unable to run ticks from threads");
-            ((MinecraftServerInterface)s.getMinecraftServer()).forceTick( () -> System.nanoTime()- CarpetServer.scriptServer.tickStart<50000000L);
-            if (lv.size()>0)
+            if (CarpetServer.scriptServer.tickDepth > 16) throw new InternalExpressionException("'game_tick' function caused other 'game_tick' functions to run. You should not allow that.");
+            try
             {
-                long ms_total = NumericValue.asNumber(lv.get(0).evalValue(c)).getLong();
-                long end_expected = CarpetServer.scriptServer.tickStart+ms_total*1000000L;
-                long wait = end_expected-System.nanoTime();
-                if (wait > 0L)
+                CarpetServer.scriptServer.tickDepth ++;
+                ((MinecraftServerInterface) s.getMinecraftServer()).forceTick(() -> System.nanoTime() - CarpetServer.scriptServer.tickStart < 50000000L);
+                if (lv.size() > 0)
                 {
-                    try
-                    {
-                        Thread.sleep(wait/1000000L);
-                    }
-                    catch (InterruptedException ignored)
+                    long ms_total = NumericValue.asNumber(lv.get(0).evalValue(c)).getLong();
+                    long end_expected = CarpetServer.scriptServer.tickStart + ms_total * 1000000L;
+                    long wait = end_expected - System.nanoTime();
+                    if (wait > 0L)
                     {
+                        try
+                        {
+                            Thread.sleep(wait / 1000000L);
+                        }
+                        catch (InterruptedException ignored)
+                        {
+                        }
                     }
                 }
+                CarpetServer.scriptServer.tickStart = System.nanoTime(); // for the next tick
+                Thread.yield();
+            }
+            finally
+            {
+                CarpetServer.scriptServer.tickDepth --;
             }
-            CarpetServer.scriptServer.tickStart = System.nanoTime(); // for the next tick
-            Thread.yield();
             if(CarpetServer.scriptServer.stopAll)
                 throw new ExitStatement(Value.NULL);
-            return (cc, tt) -> Value.TRUE;
+            return (_c, _t) -> Value.TRUE;
         });
 
         expression.addLazyFunction("seed", -1, (c, t, lv) -> {