Fix CJK line wrapping (#792) and harden FluidIntersectionMarker WAILA#831
Fix CJK line wrapping (#792) and harden FluidIntersectionMarker WAILA#831KevinWoodWL wants to merge 2 commits into
Conversation
LordIdra
left a comment
LordIdra
left a comment
There was a problem hiding this comment.
Could use @JustAHuman-xD eyes on packet handler stuff
Does this close #792? From what I gather it doesn't actually implement line wrapping for strings without spaces, just stops them from erroring?
| val display = try { | ||
| fluidIntersectionDisplay | ||
| } catch (_: Throwable) { | ||
| return null | ||
| } |
There was a problem hiding this comment.
Use getHeldRebarEntity(FluidIntersectionDisplay::class.java, "intersection") instead of try-catching
There was a problem hiding this comment.
The changes in this file aren't really necessary, as the FluidButton constructor expects at least 1 fluid to be provided and this is explicitly stated in documentation. I would just add an initialiser which checks that at least one fluid has been provided and throws an exception if not, to provide a clearer error message. But there isn't really a need for a player-facing error message here
| content = if (endIndex+1 == content.length) "" else content.substring(endIndex+1) | ||
| content = if (endIndex >= content.length) "" else content.substring(endIndex + 1) |
There was a problem hiding this comment.
What is this change for? The new code is not semantically equivalent
| break | ||
| } | ||
|
|
||
| // Make sure we snap to the end of a word (i.e. don't cut words in half) |
There was a problem hiding this comment.
Forgot to move this comment down too
| val maxCharsOnLine = RebarConfig.TRANSLATION_WRAP_LIMIT - currentLineLength | ||
| if (maxCharsOnLine <= 0) { | ||
| lines.add(currentLine) | ||
| currentLine = DEFAULT_COMPONENT | ||
| currentLineLength = 0 | ||
| continue | ||
| } |
There was a problem hiding this comment.
I would add a comment to explain why this is necessary
|
Having written the original packet handler, I can tell you that we mutate the internal stacks for a reason. This is extremely performance sensitive code, and even doing something like cloning an item stack has severe performance impacts, which is why me mutate the internal stack. Those we mutate are already cloned by the packet system, so I find it very hard to believe that a race condition can even happen here given that this is in the middle of sending the packet over the network. And yes all my assetions I have tested. |
|
ah thanks |
…r WAILA LineWrapping: properly wrap text that has no spaces (e.g. Chinese/Japanese) or a single word longer than the wrap limit. Previously the word-snap loop walked the break index back to 0, appended an empty substring, and dropped one character per iteration, producing many empty lines and silently losing text. Now it hard-breaks at the wrap limit when no space is found, so it both wraps correctly and always makes progress. Closes pylonmc#792. FluidIntersectionMarker: resolve the connected pipe defensively via getHeldRebarEntity so a lost intersection/pipe display no longer NPEs the WAILA update tick - getWaila hides the WAILA and getPickItem returns null instead.
KevinWoodWL
force-pushed
the
fix/robustness-crash-fixes
branch
from
0a119a2 to
bcbcc99
Compare
KevinWoodWL
changed the title
|
Fair enough, thanks for explaining! Dropped the packet handler change. The crash I hit had invui's PacketListener in the stack too, so it's probably an interaction with that rather than your code, I'll look into it separately with a proper repro. Also reverted the FluidButton change since the init already covers it. PR is now just the CJK line-wrap fix (#792) and the FluidIntersectionMarker WAILA null-safety (switched to getHeldRebarEntity as @LordIdra suggested). |
# Conflicts: # rebar/src/main/kotlin/io/github/pylonmc/rebar/content/fluid/FluidIntersectionMarker.kt
|
To re-iterate: Does this close #792? From what I gather it doesn't actually implement line wrapping for strings without spaces, just stops them from erroring? |
4 participants
What
Four small, self-contained robustness fixes that prevent server-side crashes and log spam:
1.
PlayerPacketHandler— don't mutate sharedItemStacks on the Netty threadOutgoing item translation previously modified the NMS
ItemStacks in place. This handler runs on Netty's I/O thread, while those same stacks can be touched by the main thread, so mutating their component maps races. On 1.21.6+ this surfaces asArrayIndexOutOfBoundsExceptioninside fastutil'sReference2ObjectArrayMapand thenCan't find id for 'null'when the corrupted packet is encoded.Now we translate copies and rebuild the affected packets (
ContainerSetContent,ContainerSetSlot,SetCursorItem,SetEquipment). Packet rewriting is also wrapped in try/catch so a failure falls back to sending the original packet instead of dropping the connection.2.
LineWrapping— infinite loop / index out of bounds on spaceless textThe word-wrap loop assumed text contained spaces and that no single token exceeded the wrap limit. With CJK text (no spaces) or an over-long token it could loop forever or throw
StringIndexOutOfBoundsException. Added guards for empty content, non-positive remaining width, and clamps the cut index to the content length.3.
FluidButton—IndexOutOfBoundson empty fluid listcurrentFluidindexed into the fluids list unconditionally. Made it nullable viagetOrNull, fall back to a shared error item, and bail out of click handling when no fluid is resolvable.4.
FluidIntersectionMarker— NPE in WAILA when pipe display is missingIf the held intersection display or its connected pipe display entity has been lost (e.g. entity cleanup, world data issues), the
pipegetter NPE'd inside the WAILA update tick. AddedpipeOrNull();getWailanow hides the WAILA andgetPickItemreturns null instead of throwing.Why
All four were hit on a live server and either crashed a subsystem or spammed the console every tick.
Notes
Each fix is independent and touches a single file — happy to split into separate PRs if preferred.