Skip to content

started working on adding logger where useful#30

Merged
ChillerDragon merged 4 commits intoteeworlds-go:masterfrom
jessefromearth:improved-logging
Mar 24, 2026
Merged

started working on adding logger where useful#30
ChillerDragon merged 4 commits intoteeworlds-go:masterfrom
jessefromearth:improved-logging

Conversation

@jessefromearth
Copy link
Copy Markdown
Contributor

@jessefromearth jessefromearth commented Mar 17, 2026

This is a small first step in a larger task of improving debug logging throughout the project. What I've done here is a minimum / likely incomplete solution, where I've added the ability to specify a logger. I want to make it so that we can eventually keep these potentially useful logs for debugging but make them optional. Thoughts?

After this change, when creating a Client, it will use log.Default() as the default logger, or you could set it by setting Client.Logger to something like, log.New(io.Discard, "", 0). Maybe we should scrap this idea and either comment out the debug stuff since that seems like more of a library developer thing, OR keep them and set up some better logging system with variable verbosity?

@ChillerDragon
Copy link
Copy Markdown
Member

Yea some of the logs are there because the library still has bugs. Once they are fixed they can be removed. But I don't think that is gonna happen any time soon.

Your pr looks good to me, at least I trust you with it as I don't know go haha.

Is the log.Default() and io.Discard obvious to go developers? I wouldn't know it but that doesn't mean much. Do you think it would make sense to include examples of a silent and verbose version in the existing examples. For example silence it in example_trivia and use the real logger in the example_verbose

@jessefromearth
Copy link
Copy Markdown
Contributor Author

I think log.Default() is somewhat obvious but good call potentially on io.Discard not being that way. I'll add examples in. Those logs felt like they wouldn't need to be permanent, but it is interesting to me to imagine a scenario where we keep them as optional. Ill add examples like you suggested and think on this more later.

@jessefromearth
Copy link
Copy Markdown
Contributor Author

I just showed how to silence the logs in client_trivia. Since it uses log.Default() by default, there's no reason to show how to set that manually imo

go.mod Outdated

replace github.com/teeworlds-go/protocol => github.com/jessefromearth/protocol v0.0.0-20260317031039-4222acf8de33

replace github.com/jessefromearth/protocol v0.0.0-20260317031039-4222acf8de33 => github.com/jessefromearth/protocol v0.0.0-20260317031039-4222acf8de33
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

whats going on here oO was that an accident?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, my bad. That was an accident!

@ChillerDragon
Copy link
Copy Markdown
Member

I just showed how to silence the logs in client_trivia. Since it uses log.Default() by default, there's no reason to show how to set that manually imo

Sounds good, thanks

@ChillerDragon ChillerDragon merged commit 72fec01 into teeworlds-go:master Mar 24, 2026
5 checks passed
@ChillerDragon
Copy link
Copy Markdown
Member

Thanks

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.

2 participants