Skip to content

Feat: Added a simple logger implementation #1777

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

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

G4PLS
Copy link

@G4PLS G4PLS commented Feb 23, 2025

With this I want to make it nicer for Addon devs to include logging

Every instance of the logger can contain log levels which can be toggled on/off independantly of each other.
This should result in addons being able to create theyre own logger and just being able to call self defined methods.

Some things that probably should be added are:

  • Being able to assign convars to the different fields so that they can be change over them
  • Maybe just a general GUI in the F1 Window so that users/admins can configure if they want logging or not

@TimGoll
Copy link
Member

TimGoll commented Feb 23, 2025

Just to ask what your goal is: are you aware of Dev() with its log levels?

@G4PLS
Copy link
Author

G4PLS commented Feb 24, 2025

Wait something like that exists?
I didnt see anything like that in the gmod or TTT2 docs

@TimGoll
Copy link
Member

TimGoll commented Feb 24, 2025

function Dev(level, ...)

@G4PLS
Copy link
Author

G4PLS commented Mar 2, 2025

Thats good to know, so the logger is basically not needed or would that be something thats interesting?

@TimGoll
Copy link
Member

TimGoll commented Mar 2, 2025

I think I personally don't need it - but what was your motivator when developing this module? Do you need it? Does the Dev() function satisfy your requirements?

@G4PLS
Copy link
Author

G4PLS commented Mar 2, 2025

Generally I wanted to create a logger where I can add multiple log levels and be able to toggle them on/off.
Like having Info, Debug, Error, Critical.

Then I went in the direction of: This would be nice to have for every addon, just having a simple logger where you can either create a logger or add your own log levels.

Overall Dev() does not 100% solve the issue I personally had because I cant change individual log levels and things like that

@TimGoll
Copy link
Member

TimGoll commented Mar 10, 2025

maybe somone else from the dev team can voice their opinion here. @saibotk or @Histalek?

I personally am not against this, but I'm also not sure if we need it

@saibotk
Copy link
Member

saibotk commented Mar 10, 2025

With Dev being a thing id tend to not add this additional logger implementation. Lets keep this simple for now.

But thanks for the high quality PR 🫶

@TimGoll
Copy link
Member

TimGoll commented Mar 11, 2025

Maybe dev can be expanded by what you wanted to add here?

@Histalek
Copy link
Member

While i agree that Dev() is less than ideal i don't think an implementation like the one proposed here would be that helpful either.

The most prominent problem i have with this is that Critical and Error log levels should not exist without doing specific things in these two cases:

  • Something has gone completely wrong and you can't keep going? -> please do the honest thing and throw a lua error [1].
  • Something has gone wrong, but you can keep going with limited or no functionality? -> please make that clear with a ErrorNoHalt/WithStack [2]

Further i can't really see an Info loglevel do something different than what print or Msg would get you currently.
And well Dev() essentially already is a debug loglevel with a near infinite amount of granularity.

Additionally with the currently proposed 'blanket' implementation with unlimited log levels you also overload addon devs with decisions to make:

  • what loglevel should i use?
  • what and how much additional info should i provide?
  • should i use this logging interface at all or should i just use error/print/Msg/Dev?

While i'm opposed to the currently proposed implementation, i would be open to a different implementation with static log levels and more defined traits.
Maybe these static levels/functions could:

  • have a simple interface
    • something like logger.info(message, addon?) is probably already enough
  • wrap the current tools we have available
  • structure logs in a consistent way
  • have a well defined usecase presented in their doc strings
  • automagically supply additional information (maybe we can get the originating addon somehow? debug.getinfo() can at least get the source lua file; i've played around a bit with it here [3]).

What do you think? Would an implementation like the one i outlined above be something you'd be interested in?

[1] https://wiki.facepunch.com/gmod/Global.error(lowercase)

[2] https://wiki.facepunch.com/gmod/Global.ErrorNoHalt

[3] 4e1156a#diff-ba99f6bc4916e6a08593f9f6829511757f30cf221e543a01bf5556533c50cc89R92-R113

@G4PLS
Copy link
Author

G4PLS commented Mar 11, 2025

That sounds like a good Idea, overall this should make logging easier as using simple prints is quite annoying to use, especially when you want to also include things like the name of the addon

For example I didnt know Dev(), Msg() existed at all, mostly because there isnt a lot described about how to handle logging and you just either find the things you need or you dont.
Having a general logging library should make this easier as everything is bundled in one place.

If I have the time I will take a look how to implement a better logging approach with the things described above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants