Skip to content

Rework debug prints#1297

Open
sarahsturgeon wants to merge 3 commits intoCapsAdmin:developfrom
CFC-Servers:feature/rework-debug-printing
Open

Rework debug prints#1297
sarahsturgeon wants to merge 3 commits intoCapsAdmin:developfrom
CFC-Servers:feature/rework-debug-printing

Conversation

@sarahsturgeon
Copy link
Contributor

@sarahsturgeon sarahsturgeon commented Jul 19, 2023

Debug prints seem pretty neglected in PAC3. This PR aims to make it slightly more usable.

Summary

Remove pace.dprint

pac.dprint and pace.dprint were identical and had multiple declarations.
Instead, we just use pac.dprint for everything, even if it happens in the editor realm.

This also means we only have a single pac.dprint defined in the Shared util file.

Create a new pac_debug convar

Instead of relying on vars set on the pac/pace tables, this new convar determines if the debug prints should happen. It has three levels:

  • 0: Off
  • 1: Debug Logs
  • 2: Debug Logs + Trace

This closely matches the original behavior, but makes it easier to use.

Format

I think the current format is pretty gross, but I left it as-is. If you'd like me to change that here, I'd be happy to do that.

Problems

pac.debug vs. pac_debug

There are other cases around the addon that rely on pac.debug to change the behavior, so without changing those, a developer would need to set pac.debug = true and pac_debug 1.

We could change all if pac.debug to if debugConVar:GetInt() > 0 (or whatever) and then we could just have the single convar.
If that's preferable, I can do that in this PR.

Webaudio

The webaudio file has its own dprint function that uses the webaudio.debug var to decide whether or not to print.

I left it alone because I wasn't sure why they were separate, but if you think it's within the scope of this PR, I can make it use the standard pac.dprint as well.

@CapsAdmin
Copy link
Owner

The idea with webaudio and other lua scripts in the same folder is that they're supposed to be independent from pac. You could just check if pac exists in its dprint function though.

@CapsAdmin
Copy link
Owner

Feel free to change the format too, it's not like anything relies on it.

@sarahsturgeon sarahsturgeon changed the base branch from master to develop August 6, 2023 06:13
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