bbbscarter / UberLogger

Replacement logging framework for Unity, with a new editor and in-game console
MIT License
475 stars 62 forks source link

Message element height is controlled by largest message #13

Closed Kalmalyzer closed 7 years ago

Kalmalyzer commented 7 years ago

Currently, the editor computes the size of all message elements, and applies the size of the largest to all elements in the message view. This makes the maths for scrolling and selection easy, but it is disturbing from a presentation aspect.

One way to work around this without incurring a large performance hit would be to calculate the starting Y-offset and height of each individual element and store this in two arrays when updating the layout. Operations which used to translate between pixels and element IDs would then do weighted binary searches in these arrays to find the exact locations of elements, and use the information from the table when rendering the elements.

Would this be acceptable from a performance standpoint? Is there something particular that would need to be done to avoid lots of memory allocations/frees, like, always doubling the sizes of the arrays and never shrinking them?

bbbscarter commented 7 years ago

This is less about performance, and more a UI design preference - the top panel list of messages should all be one line high (for compactness) and AFAIK that line height should be consistent. e.g. like this https://github.com/bbbscarter/UberLogger/blob/master/Pics/UberConsoleEditor.png

Isn't that happening?

(Also - thanks for all the comments, much appreciated! Sorry if it takes a couple of days to reply to all of them in detail!)

Kalmalyzer commented 7 years ago

(No worries! Glad to see some response. This is a nice tool, reason for questions & PRs is because we hope to use it during internal development, and then I would like the UX to be of native Unity UI quality if possible.)

https://github.com/bbbscarter/UberLogger/blob/master/Pics/UberConsoleEditor.png is accurate in how it displays, if all messages in the log are single-line messages. Element height is consistent, the layout is compact, it's easy to browse through the log visually etc.

However, soon as there is at least one multi-line message, this affects the sizing of all elements. Assert.IsTrue(false, "free-text portion"); results in a two-line message. Debug.Log("Line 1\nLine 2\nLine 3\nLine 4"); results in a four-line message. The Plastic SCM plugin sometimes prints two-line messages.

See attached image for an example of what happens with a single four-line message in the stream.

What I find annoying with that design is that a single multiline message changes the layout for all elements in the entire log, even if it is a very long log, even if the multiline message is not in view. My experience is that I'm running my game, and suddenly I notice "hmm, the formatting of all messages in the log suddenly changed" and this is because the game suddenly printed its first two-line message. I think it would be better if the individual element height adapted to the individual message. What's your preference?

image

bbbscarter commented 7 years ago

Oh, yes, I see - multiline logging isn't something I ever use, which is why I never see this. I'll have a think.

bbbscarter commented 7 years ago

Sorry for the delay.

So, what the default Unity console does is show a maximum of two lines in the log list, and then it shows the entire message in the main view when you click on it.

UberLogger favours compactness over verbosity in the list view, and what it tries to do is concatenate the entire message into a single line in the log list. This is what it UnityEngine.Debug.Log("Hello\n this is \n a test"); looks like on mine:

screen shot 2017-03-01 at 6 57 46 am

Not sure why it doesn't look like that on yours - might be an OSX/Windows difference?

My current inclination is to continue with this behaviour (at least by default), and fixing it so it works on your platform, with the addition of showing the entire message in the details pane when you've clicked on a message. This path has the virtue of maintaining compactness (which I value) and requiring minimal changes.

If that's not enough for you, we could have an 'compact' option to toggle between proper multiline rendering and the compact view - but my worry is that it's going to complicate the rendering code somewhat (icon alignments, multiheight lines, having to iterate through all the offscreen lines to work out the proper scroll positions, etc).

Kalmalyzer commented 7 years ago

I'm happy as long as the bulk of messages become 1 line in height (so the current OSX behaviour would be great to have on Windows as well).

Found the difference: https://github.com/bbbscarter/UberLogger/blob/master/Editor/UberLoggerEditorWindow.cs#L302 converts multi-line strings to single-line strings on OSX but not on Windows.

The root problem is that any multiline strings which Unity exposes to UberLogger (both messages and callstacks) are terminated with \n on all platforms -- not System.Environment.NewLine. Best thing to do would be to define a static string UberLogger.Logger.UnityInternalNewLine, force that to "\n", and use that instead of hardcoding \n or System.Environment.NewLine in all places in UberLogger. I'll see if I can code this up later this week.

Kalmalyzer commented 7 years ago

This will be fixed by #21.