From 823e084128aba240dfa3030f4ae705d30fe8c9e0 Mon Sep 17 00:00:00 2001 From: Emilia Dreamer Date: Tue, 14 Dec 2021 12:52:21 +0200 Subject: [PATCH 1/4] api: Use initial style when replacement-rendering a hover event This matters if the component is entirely replaced due to an exact match --- .../adventure/text/TextReplacementRenderer.java | 3 ++- .../text/TextReplacementRendererTest.java | 15 +++++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/net/kyori/adventure/text/TextReplacementRenderer.java b/api/src/main/java/net/kyori/adventure/text/TextReplacementRenderer.java index 01e963206..51a9ad410 100644 --- a/api/src/main/java/net/kyori/adventure/text/TextReplacementRenderer.java +++ b/api/src/main/java/net/kyori/adventure/text/TextReplacementRenderer.java @@ -52,6 +52,7 @@ private TextReplacementRenderer() { final List oldChildren = component.children(); final int oldChildrenSize = oldChildren.size(); + final Style oldStyle = component.style(); List children = null; Component modified = component; // replace the component itself @@ -150,7 +151,7 @@ private TextReplacementRenderer() { // Only visit children if we're running if (state.running) { // hover event - final HoverEvent event = modified.style().hoverEvent(); + final HoverEvent event = oldStyle.hoverEvent(); if (event != null) { final HoverEvent rendered = event.withRenderedValue(this, state); if (event != rendered) { diff --git a/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java b/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java index b22676747..398701774 100644 --- a/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java +++ b/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java @@ -279,4 +279,19 @@ void testDoesNotDuplicateComponents() { TextAssertions.assertEquals(expected, replaced); } + + // https://github.com/KyoriPowered/adventure/issues/638 + @Test + void testExactMatchHover() { + Component expected = Component.text("two").hoverEvent(Component.text("one!")).append(Component.text("one?")); + Component replacedExact = Component.text("one").replaceText(c -> c.match("one").replacement(expected)); + + Component replacedNonExact = Component.text("one ").replaceText(c -> c.match("one").replacement(expected)); + Component expectedNonExact = Component.text("") + .append(expected) + .append(Component.space()); + + TextAssertions.assertEquals(expected, replacedExact); + TextAssertions.assertEquals(expectedNonExact, replacedNonExact); + } } From 6aa63c8eec14488ed60fa84061e4c0671e08f153 Mon Sep 17 00:00:00 2001 From: Emilia Dreamer Date: Tue, 14 Dec 2021 13:01:49 +0200 Subject: [PATCH 2/4] forgot to checkstyle --- .../kyori/adventure/text/TextReplacementRendererTest.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java b/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java index 398701774..26fefb218 100644 --- a/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java +++ b/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java @@ -283,11 +283,11 @@ void testDoesNotDuplicateComponents() { // https://github.com/KyoriPowered/adventure/issues/638 @Test void testExactMatchHover() { - Component expected = Component.text("two").hoverEvent(Component.text("one!")).append(Component.text("one?")); - Component replacedExact = Component.text("one").replaceText(c -> c.match("one").replacement(expected)); + final Component expected = Component.text("two").hoverEvent(Component.text("one!")).append(Component.text("one?")); + final Component replacedExact = Component.text("one").replaceText(c -> c.match("one").replacement(expected)); - Component replacedNonExact = Component.text("one ").replaceText(c -> c.match("one").replacement(expected)); - Component expectedNonExact = Component.text("") + final Component replacedNonExact = Component.text("one ").replaceText(c -> c.match("one").replacement(expected)); + final Component expectedNonExact = Component.text("") .append(expected) .append(Component.space()); From a4bc5a456193767ad6e6dd96cd6717767d452806 Mon Sep 17 00:00:00 2001 From: Emilia Dreamer Date: Tue, 14 Dec 2021 22:50:51 +0200 Subject: [PATCH 3/4] Add API test to ensure replacement hover wins a collision --- .../text/TextReplacementRendererTest.java | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java b/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java index 26fefb218..5075fcc2e 100644 --- a/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java +++ b/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java @@ -294,4 +294,18 @@ void testExactMatchHover() { TextAssertions.assertEquals(expected, replacedExact); TextAssertions.assertEquals(expectedNonExact, replacedNonExact); } + + @Test + void testHoverCollision() { + final Component original = Component.text("one") + .hoverEvent(Component.text("less important")); + + final Component replaced = original.replaceText(c -> c.match("one") + .replacement(Component.text("two").hoverEvent(Component.text("important")))); + + final Component expected = Component.text("two") + .hoverEvent(Component.text("important")); + + TextAssertions.assertEquals(expected, replaced); + } } From e18474e59ea75d09b438aef0ace4c022c13893fb Mon Sep 17 00:00:00 2001 From: Emilia Dreamer Date: Tue, 14 Dec 2021 23:19:47 +0200 Subject: [PATCH 4/4] api: Ensure replacement doesn't trigger on original hover if it's been replaced --- .../adventure/text/TextReplacementRenderer.java | 6 +++++- .../text/TextReplacementRendererTest.java | 14 ++++++++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/api/src/main/java/net/kyori/adventure/text/TextReplacementRenderer.java b/api/src/main/java/net/kyori/adventure/text/TextReplacementRenderer.java index 51a9ad410..854fdc218 100644 --- a/api/src/main/java/net/kyori/adventure/text/TextReplacementRenderer.java +++ b/api/src/main/java/net/kyori/adventure/text/TextReplacementRenderer.java @@ -52,7 +52,7 @@ private TextReplacementRenderer() { final List oldChildren = component.children(); final int oldChildrenSize = oldChildren.size(); - final Style oldStyle = component.style(); + Style oldStyle = component.style(); List children = null; Component modified = component; // replace the component itself @@ -79,6 +79,10 @@ private TextReplacementRenderer() { modified = replacement == null ? Component.empty() : replacement.asComponent(); + if (modified.style().hoverEvent() != null) { + oldStyle = oldStyle.hoverEvent(null); // Remove original hover if it has been replaced completely + } + // merge style of the match into this component to prevent unexpected loss of style modified = modified.style(modified.style().merge(component.style(), Style.Merge.Strategy.IF_ABSENT_ON_TARGET)); diff --git a/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java b/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java index 5075fcc2e..f8da4c6f1 100644 --- a/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java +++ b/api/src/test/java/net/kyori/adventure/text/TextReplacementRendererTest.java @@ -308,4 +308,18 @@ void testHoverCollision() { TextAssertions.assertEquals(expected, replaced); } + + @Test + void testReplacementHoverWithOriginalHoverAlsoMatching() { + final Component original = Component.text("Hello") + .hoverEvent(Component.text("Hello Kyori")); + + final Component replaced = original.replaceText(c -> c.match("Hello") + .replacement(Component.text("Goodbye world").hoverEvent(Component.text("Hello world")))); + + final Component expected = Component.text("Goodbye world") + .hoverEvent(Component.text("Hello world")); + + TextAssertions.assertEquals(expected, replaced); + } }