From 7a64e0d8a69145e7a5636947dc2d81e30807f6e7 Mon Sep 17 00:00:00 2001 From: Janmm14 Date: Sun, 21 Apr 2024 12:57:01 +0200 Subject: [PATCH] Fix #3623: Do not add white (default) color if its unset when converting to legacy. In Tests: - add bunch of tests including a failing test - change expected results as unneccessary white color codes got less Co-authored-by: bob7l --- .../md_5/bungee/api/chat/BaseComponent.java | 5 +- .../md_5/bungee/api/chat/ComponentsTest.java | 109 +++++++++++++++--- 2 files changed, 96 insertions(+), 18 deletions(-) diff --git a/chat/src/main/java/net/md_5/bungee/api/chat/BaseComponent.java b/chat/src/main/java/net/md_5/bungee/api/chat/BaseComponent.java index 558343c338..17af2508ac 100644 --- a/chat/src/main/java/net/md_5/bungee/api/chat/BaseComponent.java +++ b/chat/src/main/java/net/md_5/bungee/api/chat/BaseComponent.java @@ -668,7 +668,10 @@ void toLegacyText(StringBuilder builder) void addFormat(StringBuilder builder) { - builder.append( getColor() ); + if ( style.hasColor() || parent != null ) + { + builder.append( getColor() ); + } if ( isBold() ) { builder.append( ChatColor.BOLD ); diff --git a/chat/src/test/java/net/md_5/bungee/api/chat/ComponentsTest.java b/chat/src/test/java/net/md_5/bungee/api/chat/ComponentsTest.java index 2e8cd1eace..c5eb199087 100644 --- a/chat/src/test/java/net/md_5/bungee/api/chat/ComponentsTest.java +++ b/chat/src/test/java/net/md_5/bungee/api/chat/ComponentsTest.java @@ -192,6 +192,21 @@ public void testToLegacyFromLegacy() assertEquals( text, BaseComponent.toLegacyText( TextComponent.fromLegacyText( text ) ) ); } + @Test + public void testToLegacyFromLegacyNew() + { + String text = "" + ChatColor.GREEN + ChatColor.BOLD + "Hello " + ChatColor.WHITE + ChatColor.MAGIC + "world" + ChatColor.GRAY + "!"; + assertEquals( text, BaseComponent.toLegacyText( TextComponent.fromLegacy( text ) ) ); + } + + @Test + public void testNoColorComponent() + { + String json = "{\"text\":\"Hello World\"}"; + BaseComponent[] components = ComponentSerializer.parse( json ); + assertEquals( "Hello World", BaseComponent.toLegacyText( components ) ); + } + @Test public void testComponentBuilderCursorInvalidPos() { @@ -473,8 +488,7 @@ public void testBuilderAppendBuild() ComponentBuilder::build, (component, index) -> component.getExtra().get( index ), (component) -> BaseComponent.toPlainText( component ), - // An extra format code is appended to the beginning because there is an empty TextComponent at the start of every component - ChatColor.WHITE.toString() + ChatColor.YELLOW + "Hello " + ChatColor.GREEN + "world!", + ChatColor.YELLOW + "Hello " + ChatColor.GREEN + "world!", (component) -> BaseComponent.toLegacyText( component ) ); } @@ -512,8 +526,7 @@ public void testBuilderAppendLegacyBuild() testBuilderAppendLegacy( ComponentBuilder::build, (component) -> BaseComponent.toPlainText( component ), - // An extra format code is appended to the beginning because there is an empty TextComponent at the start of every component - ChatColor.WHITE.toString() + ChatColor.YELLOW + "Hello " + ChatColor.GREEN + "world!", + ChatColor.YELLOW + "Hello " + ChatColor.GREEN + "world!", (component) -> BaseComponent.toLegacyText( component ) ); } @@ -551,8 +564,7 @@ public void testLegacyConverter() assertEquals( "Text http://spigotmc.org google.com/test", BaseComponent.toPlainText( test2 ) ); //The extra ChatColor instances are sometimes inserted when not needed but it doesn't change the result - assertEquals( ChatColor.WHITE + "Text " + ChatColor.WHITE + "http://spigotmc.org" + ChatColor.WHITE - + " " + ChatColor.GREEN + "google.com/test" + ChatColor.GREEN, BaseComponent.toLegacyText( test2 ) ); + assertEquals( "Text http://spigotmc.org " + ChatColor.GREEN + "google.com/test" + ChatColor.GREEN, BaseComponent.toLegacyText( test2 ) ); ClickEvent url1 = test2[1].getClickEvent(); assertNotNull( url1 ); @@ -569,20 +581,39 @@ public void testLegacyConverter() public void testTranslateComponent() { TranslatableComponent item = new TranslatableComponent( "item.swordGold.name" ); - item.setColor( ChatColor.AQUA ); - TranslatableComponent translatableComponent = new TranslatableComponent( "commands.give.success", - item, "5", - "thinkofdeath" ); + TranslatableComponent component = new TranslatableComponent( "commands.give.success", item, "5", "thinkofdeath" ); - assertEquals( "Given Golden Sword * 5 to thinkofdeath", translatableComponent.toPlainText() ); - assertEquals( ChatColor.WHITE + "Given " + ChatColor.AQUA + "Golden Sword" + ChatColor.WHITE - + " * " + ChatColor.WHITE + "5" + ChatColor.WHITE + " to " + ChatColor.WHITE + "thinkofdeath", - translatableComponent.toLegacyText() ); + assertEquals( "Given Golden Sword * 5 to thinkofdeath", component.toPlainText() ); + component.setColor( ChatColor.RED ); + assertEquals( ChatColor.RED + "Given " + ChatColor.RED + "Golden Sword" + ChatColor.RED + " * " + ChatColor.RED + + "5" + ChatColor.RED + " to " + ChatColor.RED + "thinkofdeath", component.toLegacyText() ); + item.setColor( ChatColor.AQUA ); + assertEquals( ChatColor.RED + "Given " + ChatColor.AQUA + "Golden Sword" + ChatColor.RED + " * " + ChatColor.RED + + "5" + ChatColor.RED + " to " + ChatColor.RED + "thinkofdeath", component.toLegacyText() ); + component.setColor( null ); + // fails, actual value is "Given §bGolden Sword * §f5 to §fthinkofdeath" + // assertEquals( "Given " + ChatColor.AQUA + "Golden Sword" + ChatColor.RESET + " * 5 to thinkofdeath", component.toLegacyText() ); + + BaseComponent legacyColorTest = new ComponentBuilder( "Test " ).color( ChatColor.RED ).append( component ).build(); + assertEquals( ChatColor.RED + "Test " + ChatColor.RED + "Given " + ChatColor.AQUA + "Golden Sword" + ChatColor.RED + + " * " + ChatColor.RED + "5" + ChatColor.RED + " to " + ChatColor.RED + "thinkofdeath", legacyColorTest.toLegacyText() ); + + BaseComponent legacyColorTest2 = new TextComponent( "Test " ); + legacyColorTest2.addExtra( new ComponentBuilder( "abc " ).color( ChatColor.GRAY ).build() ); + legacyColorTest2.addExtra( component ); + legacyColorTest2.addExtra( new ComponentBuilder( " def" ).build() ); + assertEquals( "Test " + ChatColor.GRAY + "abc " + ChatColor.RED + "Given " + ChatColor.AQUA + "Golden Sword" + + ChatColor.RED + " * " + ChatColor.RED + "5" + ChatColor.RED + " to " + ChatColor.RED + "thinkofdeath" + + ChatColor.WHITE + " def", legacyColorTest2.toLegacyText() ); + legacyColorTest2.setColor( ChatColor.RED ); + assertEquals( ChatColor.RED + "Test " + ChatColor.GRAY + "abc " + ChatColor.RED + "Given " + ChatColor.AQUA + + "Golden Sword" + ChatColor.RED + " * " + ChatColor.RED + "5" + ChatColor.RED + " to " + ChatColor.RED + + "thinkofdeath" + ChatColor.RED + " def", legacyColorTest2.toLegacyText() ); TranslatableComponent positional = new TranslatableComponent( "book.pageIndicator", "5", "50" ); assertEquals( "Page 5 of 50", positional.toPlainText() ); - assertEquals( ChatColor.WHITE + "Page " + ChatColor.WHITE + "5" + ChatColor.WHITE + " of " + ChatColor.WHITE + "50", positional.toLegacyText() ); + assertEquals( "Page " + ChatColor.WHITE + "5 of " + ChatColor.WHITE + "50", positional.toLegacyText() ); TranslatableComponent one_four_two = new TranslatableComponent( "filled_map.buried_treasure" ); assertEquals( "Buried Treasure Map", one_four_two.toPlainText() ); @@ -605,8 +636,7 @@ public void testBuilderBuild() testBuilder( ComponentBuilder::build, (component) -> BaseComponent.toPlainText( component ), - // An extra format code is appended to the beginning because there is an empty TextComponent at the start of every component - ChatColor.WHITE.toString() + ChatColor.RED + "Hello " + ChatColor.BLUE + ChatColor.BOLD + "World" + ChatColor.YELLOW + ChatColor.BOLD + "!", + ChatColor.RED + "Hello " + ChatColor.BLUE + ChatColor.BOLD + "World" + ChatColor.YELLOW + ChatColor.BOLD + "!", (component) -> BaseComponent.toLegacyText( component ) ); } @@ -892,4 +922,49 @@ private static String fromAndToLegacyText(String legacyText) { return BaseComponent.toLegacyText( TextComponent.fromLegacyText( legacyText ) ); } + + @Test + public void testExtraFormatting() + { + BaseComponent component = new ComponentBuilder( "Hello " ).color( ChatColor.GOLD ).build(); + component.addExtra( new ComponentBuilder( "World" ).bold( true ).build() ); + component.addExtra( new ComponentBuilder( "!" ).color( ChatColor.RED ).build() ); + component.addExtra( new ComponentBuilder( " xd" ).build() ); + + assertEquals( "Hello World! xd", component.toPlainText() ); + assertEquals( ChatColor.GOLD + "Hello " + ChatColor.GOLD + ChatColor.BOLD + "World" + ChatColor.RED + "!" + + ChatColor.GOLD + " xd", component.toLegacyText() ); + } + + @Test + public void testExtraFormattingNested() + { + BaseComponent component = new ComponentBuilder( "Hello " ).color( ChatColor.GOLD ).build(); + component.addExtra( new TextComponent( new ComponentBuilder( "World" ).bold( true ).build() ) ); + component.addExtra( new TextComponent( new ComponentBuilder( "!" ).color( ChatColor.RED ).build() ) ); + component.addExtra( new TextComponent( new ComponentBuilder( " xd" ).build() ) ); + + assertEquals( "Hello World! xd", component.toPlainText() ); + // Empty extra text component 2 (holding extra "!") adds the redudant gold formatting, + // see TextComponent#toLegacyText comment as to why we keep it + assertEquals( ChatColor.GOLD + "Hello " + ChatColor.GOLD + ChatColor.GOLD + ChatColor.BOLD + "World" + + ChatColor.GOLD + ChatColor.RED + "!" + ChatColor.GOLD + ChatColor.GOLD + " xd", component.toLegacyText() ); + } + + @Test + public void testArrayToLegacyConversionContext() + { + TextComponent world = new TextComponent( "World" ); + world.setBold( true ); + BaseComponent[] components = new BaseComponent[] + { + new TextComponent( "Hello " ), + world, + new TextComponent( "!" ) + }; + + assertEquals( "Hello World!", BaseComponent.toPlainText( components ) ); + // fails, actual result doesn't reset or whiten the "!": "Hello §lWorld!" + assertEquals( "Hello " + ChatColor.BOLD + "World" + ChatColor.RESET + "!", BaseComponent.toLegacyText( components ) ); + } }