TTT2 icon indicating copy to clipboard operation
TTT2 copied to clipboard

Feat: Added a simple logger implementation

Open G4PLS opened this issue 9 months ago • 11 comments

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

G4PLS avatar Feb 23 '25 17:02 G4PLS

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

TimGoll avatar Feb 23 '25 21:02 TimGoll

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

G4PLS avatar Feb 24 '25 17:02 G4PLS

https://github.com/TTT-2/TTT2/blob/2b5d2e798c6d1eb63988fb3f282c19c753bccd63/lua/ttt2/libraries/none.lua#L70

TimGoll avatar Feb 24 '25 22:02 TimGoll

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

G4PLS avatar Mar 02 '25 16:03 G4PLS

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?

TimGoll avatar Mar 02 '25 17:03 TimGoll

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

G4PLS avatar Mar 02 '25 18:03 G4PLS

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

TimGoll avatar Mar 10 '25 22:03 TimGoll

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 🫶

saibotk avatar Mar 10 '25 22:03 saibotk

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

TimGoll avatar Mar 11 '25 08:03 TimGoll

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] https://github.com/TTT-2/TTT2/commit/4e1156af7b9a75290ed9c2cff6ebb89bda92a2d9#diff-ba99f6bc4916e6a08593f9f6829511757f30cf221e543a01bf5556533c50cc89R92-R113

Histalek avatar Mar 11 '25 17:03 Histalek

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

G4PLS avatar Mar 11 '25 18:03 G4PLS