helix icon indicating copy to clipboard operation
helix copied to clipboard

Use utf8len functions instead of string.len in ix.util.WrapText

Open OffLegAz opened this issue 3 years ago • 3 comments

OffLegAz avatar Aug 29 '21 14:08 OffLegAz

@alexgrist I have tested and confirmed that this doesn't break anything at face value but it also doesn't change anything. Though, it does have a case where it CAN break something, I believe.

If there isn't a valid UTF-8 character, utf8len will not return a number, it will return false, so the logic here is not fully case safe. If clients find ways to input into displays or chat messages non-utf8 characters in some form, this will error completely, and as such, that needs to

@OffLegAz What is the purpose of using utf8len vs string.len?

Here is the test case I used:

local testcase_352 = { }

do
    testcase_352.wrapBefore = ix.util.WrapText

    testcase_352.wrapAfter = function( text, maxWidth, font )
        font = font or "ixChatFont"
        surface.SetFont( font )
        local words = string.Explode( "%s", text, true )
        local lines = { }
        local line = ""
        local lineWidth = 0 -- luacheck: ignore 231
        -- we don't need to calculate wrapping if we're under the max width
        if ( surface.GetTextSize( text ) <= maxWidth ) then return { text } end

        for i = 1, #words do
            local word = words[ i ]
            local wordWidth = surface.GetTextSize( word )

            -- this word is very long so we have to split it by character
            if ( wordWidth > maxWidth ) then
                local newWidth

                for i2 = 1, word:utf8len() do
                    local character = word[ i2 ]
                    newWidth = surface.GetTextSize( line .. character )

                    -- if current line + next character is too wide, we'll shove the next character onto the next line
                    if ( newWidth > maxWidth ) then
                        lines[ #lines + 1 ] = line
                        line = ""
                    end

                    line = line .. character
                end

                lineWidth = newWidth
                continue
            end

            local newLine = line .. " " .. word
            local newWidth = surface.GetTextSize( newLine )

            if ( newWidth > maxWidth ) then
                -- adding this word will bring us over the max width
                lines[ #lines + 1 ] = line
                line = word
                lineWidth = wordWidth
            else
                -- otherwise we tack on the new word and continue
                line = newLine
                lineWidth = newWidth
            end
        end

        if ( line ~= "" ) then
            lines[ #lines + 1 ] = line
        end

        return lines
    end

    if CLIENT then
        -- local frame = ix.gui.ext.QuickFrame( ScrW( ) * 0.75, ScrH( ) * 0.9, 'testcase_352' )
        --Parent Frame
        local frame = vgui.Create('DFrame')
        if IsValid(ix.gui.testcase_352) then
            ix.gui.testcase_352:Remove()
        end
        ix.gui.testcase_352 = frame
        frame:SetSize(ScrW() * 0.75, ScrH() * 0.9)
        frame:Center()
        frame:MakePopup()
        -- frame.display = { }
        -- local testLeft = ix.gui.ext.QuickDPanel( frame, LEFT, frame:GetWide( ) / 2 - 8 )
        local testLeft = frame:Add('DPanel')
        testLeft:Dock(LEFT)
        testLeft:SetWide(frame:GetWide() / 2 - 8)
        testLeft.wrap = testcase_352.wrapAfter
        testLeft.drawY = 0
        -- local testRight = ix.gui.ext.QuickDPanel( frame, RIGHT, frame:GetWide( ) / 2 - 8 )
        local testRight = frame:Add('DPanel')
        testRight:Dock(RIGHT)
        testRight:SetWide(frame:GetWide() / 2 - 8)
        testRight.wrap = testcase_352.wrapBefore
        testRight.drawY = 0
        -- local headerNew = ix.gui.ext.QuickDockedPanel( testLeft, 'DLabel', TOP )
        local headerNew = testLeft:Add('DLabel')
        headerNew:Dock(TOP)
        headerNew:SetText( 'New, using: utf8len' )
        headerNew:SetContentAlignment( 5 )
        headerNew:SetFont( 'ixMenuButtonFont' )
        headerNew:SizeToContents( )
        -- local headerOld = ix.gui.ext.QuickDockedPanel( testRight, 'DLabel', TOP )
        local headerOld = testRight:Add('DLabel')
        headerOld:Dock(TOP)
        headerOld:SetText( 'Old, using: string.len' )
        headerOld:SetContentAlignment( 5 )
        headerOld:SetFont( 'ixMenuButtonFont' )
        headerOld:SizeToContents( )

        function frame:StringLogic( )
            if frame.halt then return end
            local switchTime = self.nextSwitch or 0
            local newValue, key = table.Random( ix.lang.stored.english )
            if switchTime < CurTime( ) and key then
                if ix.lang.stored.english[ key ] and string.len( ix.lang.stored.english[ key ] ) > 32 then
                    self.displayValue = key
                    self.nextSwitch = CurTime( ) + .2
                    key = nil

                    return self.displayValue
                end
            end
            return self.displayValue
        end

        local offsetY = 128

        local function DrawTextLang( panel, wide, langID, langStr, ft )
            local str = Format( '%s: ', langID:upper( ) ) .. ( ix.lang.stored[ langID ][ langStr ] or 'NoTranslation' .. langID )
            local wrapTab = panel.wrap( str, wide, ft )
            local x = ( wide / 2 ) - 4
            if #testcase_352.wrapBefore(str, wide, ft) ~= #testcase_352.wrapAfter(str, wide,ft) then
                frame.halt = true
            end
            for i = 1, #wrapTab do
                draw.SimpleText( wrapTab[ i ], ft, x, panel.drawY + offsetY, color_white, TEXT_ALIGN_CENTER, TEXT_ALIGN_CENTER )
                panel.drawY = panel.drawY + 32
            end
        end

        local PaintFunction = function( panel, width, height )
            surface.SetDrawColor(frame.halt and Color(255,0,0,128) or ColorAlpha(color_black, 128))
            panel:DrawFilledRect()
            local index = 0
            local txt = frame:StringLogic( )
            draw.SimpleText( txt, 'ixMediumFont', width / 2, 64, color_white, TEXT_ALIGN_CENTER, TEXT_ALIGN_CENTER )
            panel.drawY = 0

            if panel then
                local x, y = ( panel:GetWide( ) / 2 ) - 4, offsetY + ( 32 * index )

                for k, v in pairs( ix.lang.stored ) do
                    DrawTextLang( panel, width, k, txt, 'ixMenuButtonFont')
                    index = index + 1
                    panel.drawY = panel.drawY + 32
                end
            end

        end

        testLeft.Paint = PaintFunction
        testRight.Paint = PaintFunction
    end
end

ZeMysticalTaco avatar Sep 01 '21 21:09 ZeMysticalTaco

@ZeMysticalTaco the purpose of using utf8len is simple - string.len counts non-latin characters incorrectly. Bellow I've printed length of string wich contains 3 cyrillic А's: image

By the way, previously I've made PR similar to this one and it was merged.

OffLegAz avatar Sep 01 '21 22:09 OffLegAz

@ZeMysticalTaco the purpose of using utf8len is simple - string.len counts non-latin characters incorrectly. Bellow I've printed length of string wich contains 3 cyrillic А's: image

By the way, previously I've made PR similar to this one and it was merged.

https://i.imgur.com/HrKPi3o.png It does indeed appear to be that way, thank you for clarifying.

Other than a non-utf8 sequence existing (which shouldn't ever happen anyway.] I don't really see any way this code will break anything.

ZeMysticalTaco avatar Sep 02 '21 18:09 ZeMysticalTaco