Skip to content

did whatever to fix older client holos#71

Merged
Tofaa2 merged 2 commits into
Tofaa2:masterfrom
InstantlyMoist:fix/hologram-older-client
Jul 4, 2026
Merged

did whatever to fix older client holos#71
Tofaa2 merged 2 commits into
Tofaa2:masterfrom
InstantlyMoist:fix/hologram-older-client

Conversation

@InstantlyMoist

@InstantlyMoist InstantlyMoist commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

good luck have fun

Summary by CodeRabbit

  • New Features
    • NPC hologram name tags can now render across multiple lines using configured line breaks.
  • Bug Fixes
    • Holograms now update more reliably during NPC movement by teleporting to the NPC each sync tick with correct yaw/pitch and sitting-dependent vertical offset.
    • Improved hologram line positioning and alignment after teleporting, including consistent per-line spacing.
    • Removed a post-teleport parent/passenger reassignment behavior for legacy holograms that could affect follow-up teleport state.

@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e495c8d9-24ef-4b05-bf53-7cf43dae6357

📥 Commits

Reviewing files that changed from the base of the PR and between 1373d6e and 3a6b4fd.

📒 Files selected for processing (1)
  • api/src/main/java/me/tofaa/entitylib/wrapper/hologram/LegacyHologram.java

📝 Walkthrough

Walkthrough

Hologram 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 <br> splitting. NPCMovement now teleports holograms directly instead of setting a parent.

Changes

Hologram teleport and NPC hologram lines

Layer / File(s) Summary
Hologram teleport/parenting core changes
api/.../hologram/LegacyHologram.java, api/.../hologram/ModernHologram.java
LegacyHologram no longer re-parents passengers after teleport; ModernHologram introduces a LINE_SPACING constant and teleports each line individually to a computed offset instead of teleporting only the first line and attaching others as passengers.
NPC hologram creation and update with split lines
spaceNPC/.../npc/NPC.java, spaceNPC/.../npc/NPCMovement.java
Adds a MiniMessage-based helper to split display names into multiple lines on <br>, always creates Hologram.Legacy (removing the protocol-version branch), applies adjusted seated/non-seated Y-offsets, updates hologram lines fully on update, and changes NPCMovement to teleport the hologram to the NPC's location with yaw/pitch instead of setting its parent.

Estimated code review effort: 3 (Moderate) | ~25 minutes

Possibly related PRs

  • Tofaa2/EntityLib#52: Modifies the same processViewerSync hologram update logic in NPCMovement.java.
  • Tofaa2/EntityLib#53: Also removes the post-teleport re-parenting behavior in LegacyHologram.teleport(Location).
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Title check ❓ Inconclusive The title is vague and non-descriptive, so it does not clearly summarize the main change in the pull request. Replace it with a concise, specific title such as updating hologram behavior for older clients.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between e797eee and 1373d6e.

📒 Files selected for processing (4)
  • api/src/main/java/me/tofaa/entitylib/wrapper/hologram/LegacyHologram.java
  • api/src/main/java/me/tofaa/entitylib/wrapper/hologram/ModernHologram.java
  • spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPC.java
  • spaceNPC/src/main/java/me/tofaa/entitylib/npc/NPCMovement.java

Comment on lines +331 to +344
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;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 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:


🏁 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]}")
PY

Repository: 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.

@Tofaa2 Tofaa2 merged commit ee500f4 into Tofaa2:master Jul 4, 2026
1 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants