Add custom text truncation support, and improve scoreboard legibility#7105
Add custom text truncation support, and improve scoreboard legibility#7105Lightningbulb2 wants to merge 12 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (4)
💤 Files with no reviewable changes (1)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughAdds truncation support to the UI Text component and applies it to the scoreboard: separates name/division rendering, adds time/gameSpeed/gameQuality header controls with tooltips, positions those controls in compact layouts, and updates localization and changelog entries. ChangesScoreboard Text Truncation and Display Enhancement
🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
lua/maui/text.lua (1)
78-80: ⚡ Quick win
SetTruncationTextdoesn't reapply truncation to the current display.If called after text has already been set (e.g. to swap
"..."for"-"), the display won't update until the nextSetTextor width change. Consider re-triggering_applyTruncationhere when_fullTextis already populated.♻️ Proposed fix
SetTruncationText = function (self, text) self._truncationText = tostring(text) + if self._fullText ~= nil and self._initialized then + self:_applyTruncation() + end end,🤖 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 `@lua/maui/text.lua` around lines 78 - 80, SetTruncationText currently only updates self._truncationText and doesn't refresh the rendered text; modify SetTruncationText to set self._truncationText = tostring(text) and then, if self._fullText is non-nil/non-empty, call self:_applyTruncation() so the new truncation marker is applied immediately to the current display (same behavior as after SetText or width changes).
🤖 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 `@lua/maui/text.lua`:
- Around line 113-117: The loop is byte-oriented and can break multi-byte UTF‑8
characters: replace uses of string.len and str:sub(1, -2) with the UTF‑8 safe
helpers used elsewhere (use STR_Utf8Len to compute i and STR_Utf8SubString to
remove the last character), and ensure the truncation loop that calls
self:GetStringAdvance(str .. ellipsis) operates on the UTF‑8 substring so
multi‑byte characters aren’t split.
- Around line 106-110: In _applyTruncation: guard against _fullText being nil by
initializing it from the current display text (e.g. if self._fullText == nil
then self._fullText = self:GetDisplayText() or ""), then use that value when
calling GetStringAdvance; if the text fits (GetStringAdvance(...) <= maxWidth)
call SetDisplayText(self._fullText) and clear any truncation flag (e.g.
self._truncated = false) to restore the full text; otherwise perform the
truncation as before, call SetDisplayText(truncatedString) and set
self._truncated = true. Ensure you continue to use _truncationText/_fullText and
methods _applyTruncation, GetStringAdvance, SetDisplayText, GetDisplayText in
these changes.
In `@lua/ui/game/score.lua`:
- Around line 657-660: The variable q is being assigned without local scope
inside the block that checks sessionInfo.Options.Quality, causing a global leak;
make q a local variable (consistent with t and s used earlier) before assigning
string.format("Q:%.2f%%", sessionInfo.Options.Quality) and then call
controls.gameQuality:SetText(q) with that local q to avoid polluting the global
namespace.
- Line 132: The color string passed to controls.gameSpeed:SetColor is missing
the alpha channel; change the hex from 'BADAFF' to include an opaque alpha
prefix (match other calls like 'ff00dbff') so use an 8-character AARRGGBB string
(e.g. 'ffBADAFF') when calling controls.gameSpeed:SetColor to ensure the
intended opaque color is applied.
---
Nitpick comments:
In `@lua/maui/text.lua`:
- Around line 78-80: SetTruncationText currently only updates
self._truncationText and doesn't refresh the rendered text; modify
SetTruncationText to set self._truncationText = tostring(text) and then, if
self._fullText is non-nil/non-empty, call self:_applyTruncation() so the new
truncation marker is applied immediately to the current display (same behavior
as after SetText or width changes).
🪄 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: dfbb29a2-64fd-486d-a6ff-1f7e5a1e0ec2
📒 Files selected for processing (6)
changelog/snippets/graphics.7105.mdloc/US/strings_db.lualua/maui/text.lualua/ui/game/layouts/score_mini.lualua/ui/game/score.lualua/ui/help/tooltips.lua
lL1l1
added
area: ui
| self._truncationText = "" | ||
| self._fullText = nil | ||
|
|
||
|
|
||
| --- Direct Engine SetText() that changes what text is displayed | ||
| ---@type function | ||
| ---@param str string | number | ||
| self.SetDisplayText = self.SetText | ||
|
|
||
|
|
||
| --- FAF extensible SetText() that uses SetDisplayText() but can retain it's original text for fancy text display setups like truncation | ||
| ---@param text string | number | ||
| self.SetText = function (self, text) | ||
| self.SetDisplayText(self,text) | ||
| self._fullText = text | ||
| if self._truncationText ~= nil and self._truncationText ~= "" then | ||
| if self._initialized then | ||
| self:_applyTruncation() | ||
| end | ||
| end | ||
| end | ||
|
|
||
| --- Direct Engine GetText() for getting the current displayed value | ||
| ---@type function | ||
| ---@return string | ||
| self.GetDisplayText = self.GetText | ||
|
|
||
| --- FAF extensible GetText() that retrieves raw original text that isn't modified for display | ||
| ---@return string | ||
| self.GetText = function (self) | ||
| return self._fullText | ||
| end |
There was a problem hiding this comment.
Make a separate class for Truncatable text. You are polluting Text class with functionality used by scoreboard only.
There was a problem hiding this comment.
There are really 2 changes here. A text rendering feature (better truncation), and a text architecture change (display text) which the truncation relies on.
1. Better Text Truncation
It's only used by the scoreboard because it didn't exist to be used for other things yet.
Text can already be truncated, but this simply allows the option for setting how it truncates it. You may want a dash, dots, or warning text (for texts that shouldn't be truncated but would be hard to notice if cut off at the right spot).
It's more of a small extension to SetClipToWidth() than a new special text type.
SetTruncationText() is just setting the trailing characters, if there was any confusion there. Maybe SetTrailingCharacters() would be better?
2. SetText() and SetDisplayText()
The separation of the full text and the "displayed text" sent to the engine, is much more useful than just supporting truncation text. It allows much more elaborate text animation/visual effects for modding and development because it's no longer on you to keep track of the old state. Plus anything that tries to GetText() will get it's real value instead of the partial text effect value that may have been setup. GetDisplayText() would be used for getting what the engine sees.
Hopefully that makes sense. Thanks for the review!
There was a problem hiding this comment.
Can I get a response by the Human being? Still i insist on creating a separate class.
There was a problem hiding this comment.
I typed all that myself, dome to keyboard :(
The last line "Hopefully that makes sense." was about my writing, not some AI garbage. I try to include a thanks when I can because this is volunteer work, and programming feedback tends to be heavy on critique so the "thanks" is as much for you as it is for me (to not take it personally).
Aaaanyway, yeah, you're right about the function instancing. That modding override trick is great for single function rewrites, but I understand avoiding it here.
How about this solution in the body rather than inside __init? That would prevent all the instancing right?
---@class Text : moho.text_methods, Control, InternalObject
Text = ClassUI(...)
__init()..
--- Direct Engine SetText() that changes what text is displayed
---@type function
---@type fun(self: Text, str: string | number)
SetDisplayText = moho.text_methods.SetText,
--- Direct Engine GetText() for getting the current displayed value
---@type function
---@return string
GetDisplayText = moho.text_methods.GetText,
--- FAF extensible SetText() that uses SetDisplayText() but can retain it's original text for fancy text display setups like truncation
---@param text string | number
SetText = function(self, text)
self:SetDisplayText(text)
self._fullText = text
if self._truncationText ~= nil and self._truncationText ~= "" then
if self._initialized then
self:_applyTruncation()
end
end
end,
--- FAF extensible GetText() that retrieves raw original text that isn't modified for display
---@return string
GetText = function(self)
return self._fullText
end,
--- Sets custom truncation trailing characters like "..." or "-". Set to "" to disable.
---@param text string | number
SetTruncationText = function(...)...
Also, should I keep it as "truncation text" or call it "trailing characters" instead, for clarity?
self._truncationText would then be self._trailingCharacters
SetTruncationText() would then be SetTrailingCharacters()
Claude.ai showed me I could use "moho.text_methods.SetText" instead of "self.SetText()"
Although it defined local variables outside the class to reference it first and then assign the Display functions in the __init(), but I found that wasn't necessary.
(Hand written reply)
There was a problem hiding this comment.
Sorry if I offended you. I just don't like when people drop a lot of text especially made with ai not even understanding what's it about.
As I said, you are modifying class that is used everywhere in the code and any side effects are very unwanted. Please just create a separate class that inherits from Text. Like TruncatableText and instantiate it in scoreboard. It will make it easier to control scope of the change and will be easier to maintain.
There was a problem hiding this comment.
The latest commits remove a lot of unnecessary code. It updated fine even when I was messing with the text values in-game.
The only 2 checks that happen now are of the boolean self._truncationEnabled inside both SetText(), and the OnDirty from SetClipToWidth()
If it's not enabled, nothing else runs, so it shouldn't bother the rest of the backend text.
The additional fields are
Bool: self._truncationEnabled (default: false)
String: self._truncationText (default: "...")
String: self._fullText (default: nil)
Aside from the truncation stuff, I would still want to keep the main Text changes. So adding another class would just move the truncation stuff for not much gain, plus more inconvenience because any new text features would have to be added to both.
Ignoring all the truncation stuff, this is ALL the core changes to how Text operates
__init = function(...)
...
self._fullText = nil
end,
SetDisplayText = moho.text_methods.SetText,
GetDisplayText = moho.text_methods.GetText,
SetText = function(self, text)
self:SetDisplayText(text)
self._fullText = text
end,
GetText = function(self)
return self._fullText
end,
So while I understand being hesitant to make sweeping changes that might break things. I've been extremely careful to keep things compatible and only those few lines actually matter for the rest of the repository. The other changes in Text.lua are from the optional text truncation, which I've kept as unobtrusive as possible so it can stay as a standard text feature.
If you can think of any side effects of adding this, let me know, otherwise we'll just have to find out if it breaks.
Thanks for reading
| --- Direct Engine SetText() that changes what text is displayed | ||
| ---@type function | ||
| ---@param str string | number | ||
| self.SetDisplayText = self.SetText |
There was a problem hiding this comment.
Each Text instance has this field, not its meta.
|
|
||
| --- FAF extensible SetText() that uses SetDisplayText() but can retain it's original text for fancy text display setups like truncation | ||
| ---@param text string | number | ||
| self.SetText = function (self, text) |
There was a problem hiding this comment.
Each Text instance has this field, not its meta.
| --- Direct Engine GetText() for getting the current displayed value | ||
| ---@type function | ||
| ---@return string | ||
| self.GetDisplayText = self.GetText |
There was a problem hiding this comment.
Each Text instance has this field, not its meta.
|
|
||
| --- FAF extensible GetText() that retrieves raw original text that isn't modified for display | ||
| ---@return string | ||
| self.GetText = function (self) |
There was a problem hiding this comment.
Each Text instance has this field, not its meta.
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 `@lua/maui/text.lua`:
- Around line 60-63: The SetText function updates the display text and full text
but does not trigger truncation, so the custom truncation suffix will not appear
until the control's bounds change. Modify the SetText function to explicitly
call _applyTruncation() after updating the text values, ensuring the truncation
is applied immediately when text is set and truncation is enabled, rather than
waiting for a bounds change event.
🪄 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: a69c25ef-843e-4303-9db0-e87fff970b39
📒 Files selected for processing (2)
lua/maui/text.lualua/ui/game/score.lua
🚧 Files skipped from review as they are similar to previous changes (1)
- lua/ui/game/score.lua
3 participants
Description of the proposed changes
I wanted to improve scoreboard legibility.
I started by separating game speed, and game quality into their own controls to make modding and future changes easier. I added dropshadows, tooltips, and right aligned the ratings while making sure to crop player names if necessary. This led to the-
Implementation of SetTruncateText() which allows custom trailing strings like "...", "-", or even "NOFIT". This is executed through SetClipToWidth() and only activates if a truncation string is set.
To do this cleanly, and in a reusable way, I split SetText() into SetText() and its implicitly called SetDisplayText().
SetText() - controls the source text and behaves practically the same as before
SetDisplayText() - sends text changes to the engine without changing the Text object's internal string. This allows for better mod and source code capabilities for fancier text. The first example being custom text truncation.
Testing done on the proposed changes
I loaded up private AI matches and replays to check for console and visual errors. I got screenshots:
Original for reference (plus rank cropping issue)
Game start with lots of players
Full scoreboard
Replay scoreboard
Show full player names after hovering for a moment (in-case of cropped names)
Tooltips added for new players
Checklist
Summary by CodeRabbit