Skip to content
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

Rework debug prints #1297

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

brandonsturgeon
Copy link
Contributor

@brandonsturgeon brandonsturgeon 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.

@brandonsturgeon brandonsturgeon 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