Skip to content

Conversation

@bmgxyz
Copy link

@bmgxyz bmgxyz commented Feb 4, 2025

It seems like this is already mostly done. There were only two GD.Prints, and all of the Console.WriteLines are in places where they should be (BuildDevSave and C7GameDataTests). It also seems like GodotSink isn't needed anymore, so I removed it.

Closes #258

@WildWeazel
Copy link
Member

It's been a long time since I looked at logging, but how did you conclude that GodotSink is unused? I thought it was providing some kind of global service.

@bmgxyz
Copy link
Author

bmgxyz commented Feb 4, 2025

how did you conclude that GodotSink is unused?

My reasoning was that the GodotSink identifier doesn't appear anywhere except in its own file and one comment in LogManager.cs. But now that I read the comment more closely, it seems like it's supposed to be there and enabled when needed (?).

I guess the question here is whether we still need GodotSink to serve its original purpose, which was to pipe Serilog messages to the Godot editor. If we do, then maybe it should be a configuration option somewhere instead of a comment. If we don't, then we should probably remove it.

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.

Migrate GD.Print and Console.Write to Serilog

2 participants