did whatever to fix older client holos#71
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughHologram teleport logic changes in LegacyHologram and ModernHologram, removing passenger re-parenting and instead independently positioning each hologram line. NPC hologram creation always uses the legacy hologram type and renders multi-line names via MiniMessage ChangesHologram teleport and NPC hologram lines
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPC.java`:
- Around line 331-344: The splitDisplayNameLines helper in NPC should stop using
MiniMessage.serialize(component) and splitting on the literal "<br>" tag, since
that is not a stable multiline delimiter. Update splitDisplayNameLines to split
on real newline characters (or accept already split lines) and then deserialize
each line with MiniMessage.miniMessage(), preserving multiline display names
without mangling literal "<br>" text. Also adjust the related doc comment to
describe newline-based splitting so the behavior is clear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c2cfd88b-83de-4afa-a835-b004f1e72a2d
📒 Files selected for processing (4)
api/src/main/java/me/tofaa/entitylib/wrapper/hologram/LegacyHologram.javaapi/src/main/java/me/tofaa/entitylib/wrapper/hologram/ModernHologram.javaspaceNPC/src/main/java/me/tofaa/entitylib/npc/NPC.javaspaceNPC/src/main/java/me/tofaa/entitylib/npc/NPCMovement.java
| private List<Component> splitDisplayNameLines(Component component) { | ||
| MiniMessage mm = MiniMessage.miniMessage(); | ||
| String serialized = mm.serialize(component); | ||
| // MiniMessage round-trips <br> as the literal tag string "<br>" | ||
| String[] parts = serialized.split("(?i)<br>", -1); | ||
| if (parts.length == 1) { | ||
| return Collections.singletonList(component); | ||
| } | ||
| List<Component> lines = new ArrayList<>(parts.length); | ||
| for (String part : parts) { | ||
| lines.add(mm.deserialize(part)); | ||
| } | ||
| return lines; | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
MiniMessage <br> newline tag deserialize serialize round trip literal
💡 Result:
In MiniMessage, the newline tag, which can be represented as or
, is used to insert a newline into formatted text [1][2]. Regarding serialization and deserialization round-tripping: 1. Deserialization: When you deserialize a MiniMessage string containing or
, the MiniMessage parser interprets these tags as instructions to include a newline character or structure within the resulting component hierarchy [3][1]. 2. Serialization: When serializing components back into a MiniMessage string, the behavior of how newlines are represented depends on your configuration. While MiniMessage supports these tags, serializing a component tree that contains literal newline characters (\n) or specialized newline components back into a MiniMessage string does not always automatically "round-trip" to the exact original tag (
or ). Often, literal newline characters in a component may be preserved as raw characters in the serialized string, or handled according to the specific serialization logic active in your environment [4][5]. 3. Literal Handling: MiniMessage does not treat literal newline characters (\n) in the input string as special tags; it generally parses them as part of the text content [6][7]. The
and tags are explicit additions provided by the library to allow for cleaner management of multiline text within the MiniMessage format [1][6]. If you are experiencing issues with round-tripping, ensure that your MiniMessage instance is configured correctly and consider whether the component tree itself contains the expected structure (e.g., text components containing \n vs. components relying on newline-specific logic) before serialization [8][7]. There is no automatic, guaranteed 1:1 conversion back to the original
tag for all component types, as the serializer interprets components based on their structure [4][8].
Citations:
- 1: https://jd.advntr.dev/text-minimessage/latest/net/kyori/adventure/text/minimessage/tag/standard/StandardTags.html
- 2: https://jd.advntr.dev/text-minimessage/4.23.0/net/kyori/adventure/text/minimessage/tag/standard/StandardTags.html
- 3: https://jd.advntr.dev/text-minimessage/latest/net/kyori/adventure/text/minimessage/MiniMessage.html
- 4: https://docs.papermc.io/adventure/minimessage/api/
- 5: https://jd.advntr.dev/text-minimessage/4.26.1/net/kyori/adventure/text/minimessage/MiniMessage.html
- 6: minimessage feature request: "<br>" or "\n" for multiline lores and messages PaperMC/adventure#619
- 7: Using \n (or <br> in mini) in multiline hover text breaks color on legacy PaperMC/adventure#1132
- 8: https://jd.advntr.dev/text-minimessage/4.23.0/net/kyori/adventure/text/minimessage/MiniMessage.Builder.html
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate the file and the helper's call sites.
rg -n --hidden --glob '!**/target/**' --glob '!**/build/**' \
'splitDisplayNameLines|createHologram|updateHologram' \
spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPC.java \
spaceNPC/src/main/java || true
# Show the surrounding code for the helper and its callers.
python3 - <<'PY'
from pathlib import Path
p = Path("spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPC.java")
if p.exists():
lines = p.read_text().splitlines()
for start in [300, 330, 360]:
end = min(len(lines), start + 70)
print(f"\n--- {p}:{start+1}-{end} ---")
for i in range(start, end):
print(f"{i+1:4d}: {lines[i]}")
PYRepository: Tofaa2/EntityLib
Length of output: 11084
Split on actual newlines instead of re-serializing MiniMessage
MiniMessage.serialize(component) does not preserve <br> as a stable delimiter, so multiline names can collapse into one hologram line and literal <br> text can be mangled. Update this helper (and its doc comment) to split on real \n or accept pre-split lines.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPC.java` around lines 331 -
344, The splitDisplayNameLines helper in NPC should stop using
MiniMessage.serialize(component) and splitting on the literal "<br>" tag, since
that is not a stable multiline delimiter. Update splitDisplayNameLines to split
on real newline characters (or accept already split lines) and then deserialize
each line with MiniMessage.miniMessage(), preserving multiline display names
without mangling literal "<br>" text. Also adjust the related doc comment to
describe newline-based splitting so the behavior is clear.
good luck have fun
Summary by CodeRabbit