text-serializer-legacy: Don't insert redundant resets

Author: zml2008

text-serializer-legacy/src/main/java/net/kyori/adventure/text/serializer/legacy/LegacyComponentSerializerImpl.java

@@ -26,6 +26,7 @@
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.EnumSet;
+import java.util.Iterator;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
@@ -297,7 +298,7 @@ private enum Reset implements TextFormat {
   private final class Cereal {
     private final StringBuilder sb = new StringBuilder();
     private final StyleState style = new StyleState();
-    private @Nullable TextFormat format;
+    private @Nullable TextFormat lastWritten;
 
     void append(final @NonNull Component component) {
       this.append(component, new StyleState());
@@ -317,22 +318,28 @@ private void append(final @NonNull Component component, final @NonNull StyleStat
       final List<Component> children = component.children();
       if(!children.isEmpty()) {
         final StyleState childrenStyle = new StyleState(style);
-        for(final Component child : children) {
-          this.append(child, childrenStyle);
-          childrenStyle.set(style);
+        for(final Iterator<Component> it = children.iterator(); it.hasNext();) {
+          this.append(it.next(), childrenStyle);
+          if(it.hasNext()) {
+            childrenStyle.set(style);
+          } else {
+            // compare style between self and parent node
+            // to see if we need to write a reset here
+            // if: color, or child has colour and parent does not. this prevents style from bleeding through
+            if((childrenStyle.color != null && style.color == null)
+              || (childrenStyle.color == style.color && !childrenStyle.decorations.equals(style.decorations))) {
+              this.append(Reset.INSTANCE);
+            }
+          }
         }
       }
-
-      if(!style.noColorOrDecorations()) {
-        this.append(Reset.INSTANCE);
-      }
     }
 
     void append(final @NonNull TextFormat format) {
-      if(this.format != format) {
+      if(this.lastWritten != format) {
         this.sb.append(LegacyComponentSerializerImpl.this.character).append(LegacyComponentSerializerImpl.this.toLegacyCode(format));
       }
-      this.format = format;
+      this.lastWritten = format;
     }
 
     @Override
@@ -343,6 +350,7 @@ public String toString() {
     private final class StyleState {
       private @Nullable TextColor color;
       private final Set<TextDecoration> decorations;
+      private boolean needsReset;
 
       StyleState() {
         this.decorations = EnumSet.noneOf(TextDecoration.class);
@@ -353,10 +361,6 @@ private final class StyleState {
         this.decorations = EnumSet.copyOf(that.decorations);
       }
 
-      boolean noColorOrDecorations() {
-        return this.color == null || this.decorations.isEmpty();
-      }
-
       void set(final @NonNull StyleState that) {
         this.color = that.color;
         this.decorations.clear();
@@ -376,16 +380,26 @@ void apply(final @NonNull Component component) {
               this.decorations.add(decoration);
               break;
             case FALSE:
-              this.decorations.remove(decoration);
+              if(this.decorations.remove(decoration)) {
+                this.needsReset = true;
+              }
               break;
           }
         }
       }
 
       void applyFormat() {
+        final boolean colorChanged = this.color != Cereal.this.style.color;
+        if(this.needsReset) {
+          if(!colorChanged) {
+            Cereal.this.append(Reset.INSTANCE);
+          }
+          this.needsReset = false;
+        }
+
         // If color changes, we need to do a full reset.
         // Additionally, if the last thing to be appended was a reset then we need to re-apply everything.
-        if(this.color != Cereal.this.style.color || Cereal.this.format == Reset.INSTANCE) {
+        if(colorChanged || Cereal.this.lastWritten == Reset.INSTANCE) {
           this.applyFullFormat();
           return;
         }

text-serializer-legacy/src/test/java/net/kyori/adventure/text/serializer/legacy/LegacyComponentSerializerTest.java

@@ -83,6 +83,7 @@ void testResetOverride() {
     assertEquals(component, LegacyComponentSerializer.legacy('&').deserialize("&a&lfoo&r&8bar"));
   }
 
+  @SuppressWarnings("checkstyle:AvoidEscapedUnicodeCharacters")
   @Test
   void testCompound() {
     final TextComponent component = Component.text()
@@ -96,7 +97,7 @@ void testCompound() {
         .build())
       .build();
 
-    assertEquals("hi there &athis bit is green &rthis isn't &aand woa, this is again", LegacyComponentSerializer.legacy('&').serialize(component));
+    assertEquals("hi there &athis bit is green &rthis isn't &aand woa, this is again&r", LegacyComponentSerializer.legacy('&').serialize(component));
   }
 
   @Test
@@ -153,7 +154,7 @@ void testToLegacy() {
           )
           .build())
       .build();
-    assertEquals("§e§lHello §a§lworld§r§e§l!§r", LegacyComponentSerializer.legacySection().serialize(c3));
+    assertEquals("§e§lHello §a§lworld§e§l!§r", LegacyComponentSerializer.legacySection().serialize(c3));
   }
 
   @Test
@@ -293,4 +294,27 @@ void testParseResetChar() {
     assertNull(lf.decoration());
     assertTrue(lf.reset());
   }
+
+  // https://github.com/KyoriPowered/adventure/issues/287
+  @Test
+  void testNoRedundantReset() {
+    final String text = "&a&lP&eaper&r";
+    final Component expectedDeserialized = Component.text()
+      .append(Component.text("P", NamedTextColor.GREEN, TextDecoration.BOLD))
+      .append(Component.text("aper", NamedTextColor.YELLOW))
+      .build();
+    final Component deserialized = LegacyComponentSerializer.legacyAmpersand().deserialize(text);
+
+    assertEquals(expectedDeserialized, deserialized);
+
+    final String roundtripped = LegacyComponentSerializer.legacyAmpersand().serialize(deserialized);
+    assertEquals(text, roundtripped);
+  }
+
+  @Test
+  void testPreserveTrailingReset() {
+    final String text = "&a&lPaper&r";
+    final String roundtripped = LegacyComponentSerializer.legacyAmpersand().serialize(LegacyComponentSerializer.legacyAmpersand().deserialize(text));
+    assertEquals(text, roundtripped);
+  }
 }