Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The plugin's diff view sometimes displays incorrectly #968

Open
Hexcede opened this issue Aug 30, 2024 · 8 comments · May be fixed by #994
Open

The plugin's diff view sometimes displays incorrectly #968

Hexcede opened this issue Aug 30, 2024 · 8 comments · May be fixed by #994
Assignees
Labels
scope: plugin Relevant to the Roblox Studio plugin type: roblox bug Something happens that shouldn't happen, and it's Roblox's fault

Comments

@Hexcede
Copy link

Hexcede commented Aug 30, 2024

Occasionally the plugin's diff view displays scripts with a lot of text overlapping, and the line height in the diff view seems to become really small.

image

There is an uncolored layer of text underneath, and a colored layer of text above, and the syntax colored layer of text is the one that suffers from this issue while the layer underneath appears to be in the correct location.

I am not sure what exactly causes this issue and I see it infrequently, but I have a hunch that it may be related to the size of the script. It happens fully consistently on the same large scripts when syncing, persists over restarts of Studio, etc so its clear that the issue is dependent on the content of the script file, likely the length since that's the only notable difference between scripts that display incorrectly and scripts that don't.

This issue occurs on the latest version of the plugin but isn't new and has happened to me once or twice in the past.

@kennethloeffler kennethloeffler added type: bug Something happens that shouldn't happen scope: plugin Relevant to the Roblox Studio plugin status: needs repro We need more information on how to reproduce this issue. labels Sep 1, 2024
@kennethloeffler kennethloeffler removed the status: needs repro We need more information on how to reproduce this issue. label Nov 6, 2024
@kennethloeffler
Copy link
Member

kennethloeffler commented Nov 6, 2024

This issue is consistently reproducible with very large scripts - I can get it to happen every time with a very large script. I'll attach a project that it happens with in a moment!

@kennethloeffler
Copy link
Member

This issue is reproducible with the following project: massive-script.zip

@boatbomber
Copy link
Member

boatbomber commented Nov 9, 2024

Thanks for the repro! It appears that TextLabel.TextBounds is just straight up lying to us. Fun! I've reported this here: https://devforum.roblox.com/t/textbounds-is-completely-incorrect-for-text-with-many-lines/3252070

Your script has 13,262 lines. TextLabel.TextBounds (and TextService:GetTextBoundsAsync) both give a height of 17,488px.
That would mean each line is 1.3 pixels tall, which is why the syntax highlights are so cramped together.

@boatbomber
Copy link
Member

Even when I compute the text bounds manually, it's broken because UDim2 Scale doesn't have the precision for the tiny values needed. If I use Offset, it won't work properly on High DPI screens... This sucks.

@boatbomber
Copy link
Member

I believe there are more high-dpi users than massive script authors, so I'm inclined to say that we keep this as the lesser of two evils.

@kennethloeffler kennethloeffler added status: on hold This is something we're not going to do right now, but might do later. type: roblox bug Something happens that shouldn't happen, and it's Roblox's fault and removed type: bug Something happens that shouldn't happen status: on hold This is something we're not going to do right now, but might do later. labels Nov 9, 2024
@boatbomber
Copy link
Member

I actually just thought of a half-decent solution to this that'll also improve performance- I can make the diff view into a virtualized list and highlight each line individually, so that there's fewer labels and no textbounds issue

@kennethloeffler
Copy link
Member

kennethloeffler commented Nov 10, 2024

Sounds okay to me, but would it work for syntactical constructs that span multiple lines? For example multi-line comments:

--[[
    This is a multi-line doc comment.
  
    Say hi!
]]
local function sayHi()
end

or multi-line strings:

local s = [[
This 
is 
a 
multi-line 
string
]]

@boatbomber
Copy link
Member

boatbomber commented Nov 10, 2024

The highlights can be computed from the full string, but they'll be rendered line by line in a virtualized list. I didn't explain that well, so thanks for calling it out!

@boatbomber boatbomber linked a pull request Nov 26, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scope: plugin Relevant to the Roblox Studio plugin type: roblox bug Something happens that shouldn't happen, and it's Roblox's fault
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants