Skip to content

Commit

Permalink
Clean up to use more common C# code style
Browse files Browse the repository at this point in the history 
Change function spacing style, use type inference when possible, remove
excessive and mostly unhelpful function documentation comments.
  • Loading branch information
@mminer
mminer committed Feb 20, 2018
1 parent 7852a71 commit 3891cfa
Showing 1 changed file with 62 additions and 93 deletions.
155 changes: 62 additions & 93 deletions Console.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -71,43 +71,42 @@ class Console : MonoBehaviour

#region MonoBehaviour Messages

void OnDisable ()
void OnDisable()
{
Application.logMessageReceived -= HandleLog;
}

void OnEnable ()
void OnEnable()
{
Application.logMessageReceived += HandleLog;
}

void OnGUI ()
void OnGUI()
{
if (!isVisible) {
if (!isVisible)
{
return;
}

windowRect = GUILayout.Window(123456, windowRect, DrawWindow, windowTitle);
}

void Update ()
void Update()
{
if (Input.GetKeyDown(toggleKey)) {
if (Input.GetKeyDown(toggleKey))
{
isVisible = !isVisible;
}

if (shakeToOpen && Input.acceleration.sqrMagnitude > shakeAcceleration) {
if (shakeToOpen && Input.acceleration.sqrMagnitude > shakeAcceleration)
{
isVisible = true;
}
}

#endregion

/// <summary>
/// Displays a log entry with a badge indicating the number of times it's been consecutively recorded.
/// <summary>
/// <param name="log">Log information.</param>
void DrawCollapsedLog (Log log)
void DrawCollapsedLog(Log log)
{
GUILayout.BeginHorizontal();

Expand All @@ -118,76 +117,70 @@ void DrawCollapsedLog (Log log)
GUILayout.EndHorizontal();
}

/// <summary>
/// Displays a log entry with separate labels for consecutive recordings.
/// </summary>
/// <param name="log">Log information.</param>
void DrawExpandedLog (Log log)
void DrawExpandedLog(Log log)
{
for (int i = 0; i < log.count; i += 1) {
for (var i = 0; i < log.count; i += 1)
{
GUILayout.Label(log.message);
}
}

/// <summary>
/// Displays a log entry.
/// </summary>
/// <param name="log">Log information.</param>
void DrawLog (Log log)
void DrawLog(Log log)
{
GUI.contentColor = logTypeColors[log.type];

if (isCollapsed) {
if (isCollapsed)
{
DrawCollapsedLog(log);
} else {
}
else
{
DrawExpandedLog(log);
}
}

/// <summary>
/// Displays a scrollable list of logs.
/// </summary>
void DrawLogList ()
void DrawLogList()
{
scrollPosition = GUILayout.BeginScrollView(scrollPosition);

// Used to determine height of accumulated log labels.
GUILayout.BeginVertical();

IEnumerable<Log> visibleLogs = logs.Where(IsLogVisible);
var visibleLogs = logs.Where(IsLogVisible);

foreach (Log log in visibleLogs) {
foreach (Log log in visibleLogs)
{
DrawLog(log);
}

GUILayout.EndVertical();
Rect innerScrollRect = GUILayoutUtility.GetLastRect();
var innerScrollRect = GUILayoutUtility.GetLastRect();
GUILayout.EndScrollView();
Rect outerScrollRect = GUILayoutUtility.GetLastRect();
var outerScrollRect = GUILayoutUtility.GetLastRect();

// If we're scrolled to bottom now, guarantee that it continues to be in next cycle.
if (Event.current.type == EventType.Repaint && IsScrolledToBottom(innerScrollRect, outerScrollRect)) {
if (Event.current.type == EventType.Repaint && IsScrolledToBottom(innerScrollRect, outerScrollRect))
{
ScrollToBottom();
}

// Ensure GUI colour is reset before drawing other components.
GUI.contentColor = Color.white;
}

/// <summary>
/// Displays options for filtering and changing the logs list.
/// </summary>
void DrawToolbar ()
void DrawToolbar()
{
GUILayout.BeginHorizontal();

if (GUILayout.Button(clearLabel)) {
if (GUILayout.Button(clearLabel))
{
logs.Clear();
}

foreach (LogType logType in Enum.GetValues(typeof(LogType))) {
bool currentState = logTypeFilters[logType];
string label = logType.ToString();
foreach (LogType logType in Enum.GetValues(typeof(LogType)))
{
var currentState = logTypeFilters[logType];
var label = logType.ToString();
logTypeFilters[logType] = GUILayout.Toggle(currentState, label, GUILayout.ExpandWidth(false));
GUILayout.Space(20);
}
Expand All @@ -197,11 +190,7 @@ void DrawToolbar ()
GUILayout.EndHorizontal();
}

/// <summary>
/// Displays a window that lists the recorded logs.
/// </summary>
/// <param name="windowID">Window ID.</param>
void DrawWindow (int windowID)
void DrawWindow(int windowID)
{
DrawLogList();
DrawToolbar();
Expand All @@ -210,99 +199,79 @@ void DrawWindow (int windowID)
GUI.DragWindow(titleBarRect);
}

/// <summary>
/// Finds the last recorded log.
/// <summary>
/// <returns>The last recorded log, or null if the list is empty.</returns>
Log? GetLastLog ()
Log? GetLastLog()
{
if (logs.Count == 0) {
if (logs.Count == 0)
{
return null;
}

return logs.Last();
}

/// <summary>
/// Records a log from the log callback.
/// </summary>
/// <param name="message">Message.</param>
/// <param name="stackTrace">Trace of where the message came from.</param>
/// <param name="type">Type of message (error, exception, warning, assert).</param>
void HandleLog (string message, string stackTrace, LogType type)
void HandleLog(string message, string stackTrace, LogType type)
{
Log log = new Log {
var log = new Log {
count = 1,
message = message,
stackTrace = stackTrace,
type = type,
};

Log? lastLog = GetLastLog();
bool isDuplicateOfLastLog = lastLog.HasValue && log.Equals(lastLog.Value);
var lastLog = GetLastLog();
var isDuplicateOfLastLog = lastLog.HasValue && log.Equals(lastLog.Value);

if (isDuplicateOfLastLog) {
if (isDuplicateOfLastLog)
{
// Replace previous log with incremented count instead of adding a new one.
log.count = lastLog.Value.count + 1;
logs[logs.Count - 1] = log;
} else {
}
else
{
logs.Add(log);
TrimExcessLogs();
}
}

/// <summary>
/// Determines whether the user has chosen to hide the given log.
/// </summary>
/// <param name="log">Log information.</param>
/// <returns>Whether the log hasn't been filtered out.</returns>
bool IsLogVisible (Log log)
bool IsLogVisible(Log log)
{
return logTypeFilters[log.type];
}

/// <summary>
/// Determines whether the scroll view is scrolled to the bottom.
/// </summary>
/// <param name="innerScrollRect">Rect surrounding scroll view content.</param>
/// <param name="outerScrollRect">Scroll view container.</param>
/// <returns>Whether scroll view is scrolled to bottom.</returns>
bool IsScrolledToBottom (Rect innerScrollRect, Rect outerScrollRect)
bool IsScrolledToBottom(Rect innerScrollRect, Rect outerScrollRect)
{
float innerScrollHeight = innerScrollRect.height;
var innerScrollHeight = innerScrollRect.height;

// Take into account extra padding added to the scroll container.
float outerScrollHeight = outerScrollRect.height - GUI.skin.box.padding.vertical;
var outerScrollHeight = outerScrollRect.height - GUI.skin.box.padding.vertical;

// If contents of scroll view haven't exceeded outer container, treat it as scrolled to bottom.
if (outerScrollHeight > innerScrollHeight) {
if (outerScrollHeight > innerScrollHeight)
{
return true;
}

// Scrolled to bottom (with error margin for float math)
return Mathf.Approximately(innerScrollHeight, scrollPosition.y + outerScrollHeight);
}

/// <summary>
/// Moves the scroll view down so that the last log is visible.
/// </summary>
void ScrollToBottom ()
void ScrollToBottom()
{
scrollPosition = new Vector2(0, Int32.MaxValue);
}

/// <summary>
/// Removes old logs that exceed the maximum number allowed if log count restriction is enabled.
/// </summary>
void TrimExcessLogs ()
void TrimExcessLogs()
{
if (!restrictLogCount) {
if (!restrictLogCount)
{
return;
}

int amountToRemove = logs.Count - maxLogCount;
var amountToRemove = logs.Count - maxLogCount;

if (amountToRemove <= 0) {
if (amountToRemove <= 0)
{
return;
}

Expand All @@ -320,7 +289,7 @@ struct Log
public string stackTrace;
public LogType type;

public bool Equals (Log log)
public bool Equals(Log log)
{
return message == log.message && stackTrace == log.stackTrace && type == log.type;
}
Expand Down

2 comments on commit 3891cfa

@samipfjo
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies for the messiness of my pull requests; I'm fairly new to C#, and it appears that my styling ideals clash with yours.

In the future, it would be helpful if you could provide constructive criticism in the pull request comments and let the contributors update their code, rather than merging in their code and revising it on your own later. This helps their future contributions better match the project's style, and saves everyone some time.

Overall, I like what you've done with the project's structure in your recent commits. Just some thoughts on helping us help the project :)

@mminer
Copy link
Owner Author

@mminer mminer commented on 3891cfa Feb 25, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No apologies necessary, I really appreciate your pull requests — more are very welcome! I didn't want to discourage them by nitpicking over code style, particularly when I hadn't settled on a definitive one myself (indeed, I changed minor things in this commit like function declaration spacing only because it matches the C# I've been writing lately).

I'll work on providing better constructive criticism for pull requests in the future. My intent wasn't originally to revise your code after merging, I just got into code golf mood when I was looking to beautify the log count badge. It's a compulsion of mine. A terrible, terrible compulsion. But feedback noted.

Please sign in to comment.