[Sign Tracker] Cache String lines ourselves to fix Paper performance issue

Signed-off-by: Irmo van den Berge <irmo.vandenberge@ziggo.nl>

src/main/generated/com/bergerkiller/generated/net/minecraft/world/level/block/entity/TileEntitySignHandle.java

@@ -21,13 +21,17 @@ public abstract class TileEntitySignHandle extends TileEntityHandle {
     /* ============================================================================== */
 
     public abstract Object[] getRawFrontLines();
+    public abstract String[] getMessageFrontLines();
     public abstract Object[] getRawBackLines();
+    public abstract String[] getMessageBackLines();
     public abstract void setFormattedFrontLine(int index, ChatText text);
     public abstract void setFormattedBackLine(int index, ChatText text);
     public static final Object[] ALL_EMPTY_RAW_LINES;
+    public static final String[] ALL_EMPTY_STRING_LINES;
     static {
         Object raw_empty = ChatText.empty().getRawHandle();
         ALL_EMPTY_RAW_LINES = new Object[] {raw_empty, raw_empty, raw_empty, raw_empty};
+        ALL_EMPTY_STRING_LINES = new String[] { "", "", "", "" };
     }
 
     @Override
@@ -44,7 +48,9 @@ public abstract class TileEntitySignHandle extends TileEntityHandle {
      */
     public static final class TileEntitySignClass extends Template.Class<TileEntitySignHandle> {
         public final Template.Method<Object[]> getRawFrontLines = new Template.Method<Object[]>();
+        public final Template.Method<String[]> getMessageFrontLines = new Template.Method<String[]>();
         public final Template.Method<Object[]> getRawBackLines = new Template.Method<Object[]>();
+        public final Template.Method<String[]> getMessageBackLines = new Template.Method<String[]>();
         public final Template.Method.Converted<Void> setFormattedFrontLine = new Template.Method.Converted<Void>();
         public final Template.Method.Converted<Void> setFormattedBackLine = new Template.Method.Converted<Void>();
 

src/main/java/com/bergerkiller/bukkit/common/block/SignChangeTracker.java

@@ -41,6 +41,8 @@ public class SignChangeTracker implements Cloneable {
     private TileEntitySignHandle tileEntity;
     private Object[] lastRawFrontLines;
     private Object[] lastRawBackLines;
+    private String[] lastMessageFrontLines;
+    private String[] lastMessageBackLines;
 
     // Need to swap what implementation we're using for certain server types
     private static final Function<Block, SignChangeTracker> constructor;
@@ -82,6 +84,8 @@ public class SignChangeTracker implements Cloneable {
             this.lastRawFrontLines = tile.getRawFrontLines().clone();
             this.lastRawBackLines = CommonCapabilities.HAS_SIGN_BACK_TEXT
                     ? tile.getRawBackLines().clone() : null;
+            this.lastMessageFrontLines = null;
+            this.lastMessageBackLines = null;
 
             // Note: it is possible for a TileEntity to be retrieved while it's added to a Chunk,
             // but not yet to a World. Especially on 1.12.2 and before. For that reason, we got to
@@ -95,6 +99,8 @@ public class SignChangeTracker implements Cloneable {
         this.tileEntity = null;
         this.lastRawFrontLines = null;
         this.lastRawBackLines = null;
+        this.lastMessageFrontLines = null;
+        this.lastMessageBackLines = null;
         this.blockData = null;
     }
 
@@ -141,8 +147,7 @@ public class SignChangeTracker implements Cloneable {
      * @return Line at this index
      */
     public String getLine(SignSide side, int index) {
-        checkRemoved();
-        return side.getLine(stateHandle, index);
+        return side.getLine(this, index);
     }
 
     /**
@@ -153,9 +158,7 @@ public class SignChangeTracker implements Cloneable {
      * @param text New text to put at this index
      */
     public void setLine(SignSide side, int index, String text) {
-        checkRemoved();
-        side.setLine(stateHandle, index, text);
-        state.update(true);
+        side.setLine(this, index, text);
     }
 
     /**
@@ -164,8 +167,7 @@ public class SignChangeTracker implements Cloneable {
      * @return Sign lines
      */
     public String[] getLines(SignSide side) {
-        checkRemoved();
-        return side.getLines(stateHandle);
+        return side.getLines(this);
     }
 
     /**
@@ -175,8 +177,7 @@ public class SignChangeTracker implements Cloneable {
      * @return Line at this index at the front of the sign
      */
     public String getFrontLine(int index) {
-        checkRemoved();
-        return stateHandle.getFrontLine(index);
+        return getFrontLines()[index];
     }
 
     /**
@@ -188,6 +189,9 @@ public class SignChangeTracker implements Cloneable {
      */
     public void setFrontLine(int index, String text) {
         checkRemoved();
+        if (lastMessageFrontLines != null) {
+            lastMessageFrontLines[index] = text;
+        }
         stateHandle.setFrontLine(index, text);
         state.update(true);
     }
@@ -199,7 +203,11 @@ public class SignChangeTracker implements Cloneable {
      */
     public String[] getFrontLines() {
         checkRemoved();
-        return stateHandle.getFrontLines();
+        String[] lines = lastMessageFrontLines;
+        if (lines == null) {
+            lastMessageFrontLines = lines = this.tileEntity.getMessageFrontLines();
+        }
+        return lines;
     }
 
     /**
@@ -225,6 +233,7 @@ public class SignChangeTracker implements Cloneable {
         this.tileEntity.setFormattedFrontLine(index, text);
 
         // Must recreate sign state, as that one caches the sign lines which are now outdated
+        this.lastMessageFrontLines = null;
         this.state = this.tileEntity.toBukkit();
         this.stateHandle = SignHandle.createHandle(this.state);
     }
@@ -247,8 +256,7 @@ public class SignChangeTracker implements Cloneable {
      * @return Line at this index at the back of the sign
      */
     public String getBackLine(int index) {
-        checkRemoved();
-        return stateHandle.getBackLine(index);
+        return getBackLines()[index];
     }
 
     /**
@@ -262,6 +270,9 @@ public class SignChangeTracker implements Cloneable {
     public void setBackLine(int index, String text) {
         checkRemoved();
         if (CommonCapabilities.HAS_SIGN_BACK_TEXT) {
+            if (lastMessageBackLines != null) {
+                lastMessageBackLines[index] = text;
+            }
             stateHandle.setBackLine(index, text);
             state.update(true);
         }
@@ -275,7 +286,11 @@ public class SignChangeTracker implements Cloneable {
      */
     public String[] getBackLines() {
         checkRemoved();
-        return stateHandle.getBackLines();
+        String[] lines = lastMessageBackLines;
+        if (lines == null) {
+            lastMessageBackLines = lines = this.tileEntity.getMessageBackLines();
+        }
+        return lines;
     }
 
     /**
@@ -304,6 +319,7 @@ public class SignChangeTracker implements Cloneable {
             this.tileEntity.setFormattedBackLine(index, text);
 
             // Must recreate sign state, as that one caches the sign lines which are now outdated
+            this.lastMessageBackLines = null;
             this.state = this.tileEntity.toBukkit();
             this.stateHandle = SignHandle.createHandle(this.state);
         }
@@ -387,7 +403,10 @@ public class SignChangeTracker implements Cloneable {
 
     /**
      * Gets a live-updated Sign instance.
-     * Only {@link #update()} updates the return value of this method.
+     * Only {@link #update()} updates the return value of this method.<br>
+     * <br>
+     * This object should <b>not</b> be used for getting and setting String text,
+     * because this is very laggy on Paper and partially broken on Spigot.
      *
      * @return Sign
      */
@@ -468,14 +487,12 @@ public class SignChangeTracker implements Cloneable {
         if (oldRawFrontLines.length != newRawFrontLines.length ||
                 (CommonCapabilities.HAS_SIGN_BACK_TEXT && oldRawBackLines.length != newRawBackLines.length)
         ) {
+            // Never happens, really
             this.lastRawFrontLines = newRawFrontLines.clone();
             if (CommonCapabilities.HAS_SIGN_BACK_TEXT) {
                 this.lastRawBackLines = newRawBackLines.clone();
             }
-            return true; // Never happens, really
-        }
-
-        if (!copyLinesCheckChanges(oldRawFrontLines, newRawFrontLines)) {
+        } else if (!copyLinesCheckChanges(oldRawFrontLines, newRawFrontLines)) {
             if (!CommonCapabilities.HAS_SIGN_BACK_TEXT || !copyLinesCheckChanges(oldRawBackLines, newRawBackLines)) {
                 // No changes.
                 return false;
@@ -485,6 +502,10 @@ public class SignChangeTracker implements Cloneable {
             System.arraycopy(oldRawBackLines, 0, newRawBackLines, 0, newRawBackLines.length);
         }
 
+        // Reset cached 'stringified' lines
+        this.lastMessageFrontLines = null;
+        this.lastMessageBackLines = null;
+
         // Detected a change. Re-create the Sign state with the updated lines,
         // and return true to indicate the change.
         this.state = this.tileEntity.toBukkit();
@@ -572,6 +593,8 @@ public class SignChangeTracker implements Cloneable {
         clone.tileEntity = this.tileEntity;
         clone.lastRawFrontLines = this.lastRawFrontLines;
         clone.lastRawBackLines = this.lastRawBackLines;
+        clone.lastMessageFrontLines = this.lastMessageFrontLines;
+        clone.lastMessageBackLines = this.lastMessageBackLines;
         return clone;
     }
 

src/main/java/com/bergerkiller/bukkit/common/block/SignSide.java

@@ -19,31 +19,47 @@ public enum SignSide {
     /**
      * Front side of the sign. Supported on all versions of Minecraft.
      */
-    FRONT(SignHandle::getFrontLines, SignHandle::getFrontLine, SignHandle::setFrontLine, true),
+    FRONT(SignHandle::getFrontLines, SignHandle::getFrontLine, SignHandle::setFrontLine,
+            SignChangeTracker::getFrontLines, SignChangeTracker::getFrontLine, SignChangeTracker::setFrontLine,
+            true),
     /**
      * Back side of the sign. Only supported on Minecraft 1.20+.
      */
-    BACK(SignHandle::getBackLines, SignHandle::getBackLine, SignHandle::setBackLine, CommonCapabilities.HAS_SIGN_BACK_TEXT);
+    BACK(SignHandle::getBackLines, SignHandle::getBackLine, SignHandle::setBackLine,
+            SignChangeTracker::getBackLines, SignChangeTracker::getBackLine, SignChangeTracker::setBackLine,
+            CommonCapabilities.HAS_SIGN_BACK_TEXT);
 
     static {
         FRONT.opposite = BACK;
         BACK.opposite = FRONT;
     }
 
-    private final Function<SignHandle, String[]> allLinesGetter;
-    private final LineGetterFunc lineGetter;
-    private final LineSetterFunc lineSetter;
+    // Bukkit (Deprecated. Slow. Bad.)
+    private final Function<SignHandle, String[]> bukkitAllLinesGetter;
+    private final BukkitLineGetterFunc bukkitLineGetter;
+    private final BukkitLineSetterFunc bukkitLineSetter;
+    // BKCommonLib SignChangeTracker (Good. Fast. Awesome.)
+    private final Function<SignChangeTracker, String[]> trackerAllLinesGetter;
+    private final SignTrackerLineGetterFunc trackerLineGetter;
+    private final SignTrackerLineSetterFunc trackerLineSetter;
     private final boolean supported;
     private SignSide opposite;
 
-    SignSide(Function<SignHandle, String[]> allLinesGetter,
-             LineGetterFunc lineGetter,
-             LineSetterFunc lineSetter,
-             boolean supported
+    SignSide(
+            final Function<SignHandle, String[]> bukkitAllLinesGetter,
+            final BukkitLineGetterFunc bukkitLineGetter,
+            final BukkitLineSetterFunc bukkitLineSetter,
+            final Function<SignChangeTracker, String[]> trackerAllLinesGetter,
+            final SignTrackerLineGetterFunc trackerLineGetter,
+            final SignTrackerLineSetterFunc trackerLineSetter,
+            final boolean supported
     ) {
-        this.allLinesGetter = allLinesGetter;
-        this.lineGetter = lineGetter;
-        this.lineSetter = lineSetter;
+        this.bukkitAllLinesGetter = bukkitAllLinesGetter;
+        this.bukkitLineGetter = bukkitLineGetter;
+        this.bukkitLineSetter = bukkitLineSetter;
+        this.trackerAllLinesGetter = trackerAllLinesGetter;
+        this.trackerLineGetter = trackerLineGetter;
+        this.trackerLineSetter = trackerLineSetter;
         this.supported = supported;
     }
 
@@ -136,11 +152,11 @@ public enum SignSide {
      * Returns an array of empty lines for the back side on Minecraft versions
      * below 1.20.
      *
-     * @param sign Sign to get lines of
+     * @param signChangeTracker SignChangeTracker to get lines of
      * @return lines
      */
-    public String[] getLines(Sign sign) {
-        return allLinesGetter.apply(SignHandle.createHandle(sign));
+    public String[] getLines(SignChangeTracker signChangeTracker) {
+        return trackerAllLinesGetter.apply(signChangeTracker);
     }
 
     /**
@@ -148,11 +164,11 @@ public enum SignSide {
      * Returns an empty line for the back side on Minecraft versions
      * below 1.20.
      *
-     * @param sign Sign to get a line of
+     * @param signChangeTracker SignChangeTracker to get a line of
      * @return line
      */
-    public String getLine(Sign sign, int index) {
-        return lineGetter.get(SignHandle.createHandle(sign), index);
+    public String getLine(SignChangeTracker signChangeTracker, int index) {
+        return trackerLineGetter.get(signChangeTracker, index);
     }
 
     /**
@@ -160,11 +176,11 @@ public enum SignSide {
      * Does nothing for the back side on Minecraft versions
      * below 1.20.
      *
-     * @param sign Sign to set a line of
+     * @param signChangeTracker SignChangeTracker to set a line of
      * @param text Line to put
      */
-    public void setLine(Sign sign, int index, String text) {
-        lineSetter.set(SignHandle.createHandle(sign), index, text);
+    public void setLine(SignChangeTracker signChangeTracker, int index, String text) {
+        trackerLineSetter.set(signChangeTracker, index, text);
     }
 
     /**
@@ -174,9 +190,11 @@ public enum SignSide {
      *
      * @param sign Sign to get lines of
      * @return lines
+     * @deprecated Bukkit Sign legacy lines are very slow. Use {@link SignChangeTracker} instead.
      */
-    public String[] getLines(SignHandle sign) {
-        return allLinesGetter.apply(sign);
+    @Deprecated
+    public String[] getLines(Sign sign) {
+        return bukkitAllLinesGetter.apply(SignHandle.createHandle(sign));
     }
 
     /**
@@ -186,9 +204,11 @@ public enum SignSide {
      *
      * @param sign Sign to get a line of
      * @return line
+     * @deprecated Bukkit Sign legacy lines are very slow. Use {@link SignChangeTracker} instead.
      */
-    public String getLine(SignHandle sign, int index) {
-        return lineGetter.get(sign, index);
+    @Deprecated
+    public String getLine(Sign sign, int index) {
+        return bukkitLineGetter.get(SignHandle.createHandle(sign), index);
     }
 
     /**
@@ -198,9 +218,53 @@ public enum SignSide {
      *
      * @param sign Sign to set a line of
      * @param text Line to put
+     * @deprecated Bukkit Sign legacy lines are very slow. Use {@link SignChangeTracker} instead.
      */
+    @Deprecated
+    public void setLine(Sign sign, int index, String text) {
+        bukkitLineSetter.set(SignHandle.createHandle(sign), index, text);
+    }
+
+    /**
+     * Gets all the lines put on this side of the sign.
+     * Returns an array of empty lines for the back side on Minecraft versions
+     * below 1.20.
+     *
+     * @param sign Sign to get lines of
+     * @return lines
+     * @deprecated Bukkit Sign legacy lines are very slow. Use {@link SignChangeTracker} instead.
+     */
+    @Deprecated
+    public String[] getLines(SignHandle sign) {
+        return bukkitAllLinesGetter.apply(sign);
+    }
+
+    /**
+     * Gets a line put on this side of the sign.
+     * Returns an empty line for the back side on Minecraft versions
+     * below 1.20.
+     *
+     * @param sign Sign to get a line of
+     * @return line
+     * @deprecated Bukkit Sign legacy lines are very slow. Use {@link SignChangeTracker} instead.
+     */
+    @Deprecated
+    public String getLine(SignHandle sign, int index) {
+        return bukkitLineGetter.get(sign, index);
+    }
+
+    /**
+     * Sets a line on this side of the sign.
+     * Does nothing for the back side on Minecraft versions
+     * below 1.20.
+     *
+     * @param sign Sign to set a line of
+     * @param text Line to put
+     * @deprecated Bukkit Sign legacy lines are very slow. Use {@link SignChangeTracker} instead.
+     */
+    @Deprecated
     public void setLine(SignHandle sign, int index, String text) {
-        lineSetter.set(sign, index, text);
+        bukkitLineSetter.set(sign, index, text);
     }
 
     /**
@@ -225,12 +289,22 @@ public enum SignSide {
     }
 
     @FunctionalInterface
-    private interface LineGetterFunc {
+    private interface SignTrackerLineGetterFunc {
+        String get(SignChangeTracker tracker, int index);
+    }
+
+    @FunctionalInterface
+    private interface SignTrackerLineSetterFunc {
+        void set(SignChangeTracker tracker, int index, String text);
+    }
+
+    @FunctionalInterface
+    private interface BukkitLineGetterFunc {
         String get(SignHandle sign, int index);
     }
 
     @FunctionalInterface
-    private interface LineSetterFunc {
+    private interface BukkitLineSetterFunc {
         void set(SignHandle sign, int index, String text);
     }
 }

src/main/templates/com/bergerkiller/templates/global/global.txt

@@ -178,3 +178,13 @@ import net.minecraft.network.chat.contents.TranslatableContents;
     }
     return out.toString();
 }
+
+#require net.minecraft.network.chat.IChatBaseComponent public static String[] formatLinesAsLegacyStrings(IChatBaseComponent[] lines) {
+    int numLines = lines.length;
+    String[] legacyLines = new String[numLines];
+    for (int i = 0; i < numLines; i++) {
+        IChatBaseComponent component = lines[i];
+        legacyLines[i] = (component == null) ? "" : component#formatAsLegacyString();
+    }
+    return legacyLines;
+}

src/main/templates/com/bergerkiller/templates/net/minecraft/world/level_block_tileentity.txt

@@ -251,11 +251,24 @@ class TileEntitySign extends TileEntity {
 #endif
     }
 
+    public String[] getMessageFrontLines() {
+#if version >= 1.20
+        IChatBaseComponent[] lines = instance.getFrontText().getMessages(false);
+#elseif version >= 1.17
+        IChatBaseComponent[] lines = instance.messages;
+#else
+        IChatBaseComponent[] lines = instance.lines;
+#endif
+        return #formatLinesAsLegacyStrings(lines);
+    }
+
 <code>
     public static final Object[] ALL_EMPTY_RAW_LINES;
+    public static final String[] ALL_EMPTY_STRING_LINES;
     static {
         Object raw_empty = ChatText.empty().getRawHandle();
         ALL_EMPTY_RAW_LINES = new Object[] {raw_empty, raw_empty, raw_empty, raw_empty};
+        ALL_EMPTY_STRING_LINES = new String[] { "", "", "", "" };
     }
 </code>
 
@@ -267,6 +280,15 @@ class TileEntitySign extends TileEntity {
 #endif
     }
 
+    public String[] getMessageBackLines() {
+#if version >= 1.20
+        IChatBaseComponent[] lines = instance.getBackText().getMessages(false);
+        return #formatLinesAsLegacyStrings(lines);
+#else
+        return TileEntitySignHandle.ALL_EMPTY_STRING_LINES;
+#endif
+    }
+
     public void setFormattedFrontLine(int index, (ChatText) IChatBaseComponent text) {
 #if version >= 1.20
         instance.setText(instance.getFrontText().setMessage(index, text), true);