Circular-Studios / Dash

A free and open 3D game engine written in D.
https://circularstudios.com/Dash
MIT License
422 stars 63 forks source link

Feature: New logging system #155

Closed NCrashed closed 10 years ago

NCrashed commented 10 years ago

Original issue: https://github.com/Circular-Studios/Dash/issues/149

Game:
    Verbosity: !Verbosity High # OutputManager 
Logging: # GlobalLogger options
    FilePath: "dash.log" # Phase 2 logname
    Debug: # Is taken in debug build
        OutputVerbosity: !Verbosity High
        LoggingVerbosity: !Verbosity High
    Release: # Is taken in release build
        OutputVerbosity: !Verbosity High
        LoggingVerbosity: !Verbosity High
ColdenCullen commented 10 years ago

I have to say I'm very impressed with this. I'm a huge fan of the quality of code here.

The only question I have is about how the Output vs. Logging works. Does a call to the log() function handle both the stdio logging and the file logging (assuming verbosity is proper)?

NCrashed commented 10 years ago

There is actually two log functions: Deprecated one (old behavior, prints to console only)

void log( A... )( OutputType type, A messages ) 

Logger one that actually should be used:

void log( A... )( LoggingLevel type, lazy A messages )

Last function prints to console and to file simultaneously . There is two parameters in logger to control output behavior:

ColdenCullen commented 10 years ago

Okay. The part where it prints to the console and to the file was the part I was curious about.

Having looked over all of the code, I can say LGTM :+1:

PxlBuzzard commented 10 years ago

Awesome stuff, this will make both the engine and game folks happier.

ColdenCullen commented 10 years ago

The only thing that I think I would suggest is to mark the logError and logInfo aliases as deprecated, so that way we can remove them in 0.8.0.

wozniakty commented 10 years ago

Looks great man. Awesome turnaround. :+1:

NCrashed commented 10 years ago

I found that my dlogg package also uses logInfo and logError names for wrappers.

Deprecating the names here also implies deprecating dlogg API (that affect a ton of code), I haven't yet decided how to synchronize wrappers names with loggging level names in the best way. Perhaps it just not a good idea.

ColdenCullen commented 10 years ago

In that case, I think it's fine to keep them.

brooklynlittell commented 10 years ago

Looks good! :+1: