Skip to content

Commit

Permalink
Fix SpigotMC#3623: Do not add white (default) color if its unset when…
Browse files Browse the repository at this point in the history 
… 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 <[email protected]>
  • Loading branch information
@Janmm14 @bob7l
Janmm14 and bob7l committed Apr 21, 2024
1 parent 06ef4b6 commit 7a64e0d
Show file tree
Hide file tree
Showing 2 changed files with 96 additions and 18 deletions.
5 changes: 4 additions & 1 deletion chat/src/main/java/net/md_5/bungee/api/chat/BaseComponent.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down
109 changes: 92 additions & 17 deletions chat/src/test/java/net/md_5/bungee/api/chat/ComponentsTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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 )
);
}
Expand Down Expand Up @@ -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 )
);
}
Expand Down Expand Up @@ -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 );
Expand All @@ -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() );
Expand All @@ -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 )
);
}
Expand Down Expand Up @@ -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 ) );
}
}

0 comments on commit 7a64e0d

Please sign in to comment.