Public log tracing functionality

[mvaligursky]

changes:

• Debug class is no longer exported, and it is completely stripped in non-debug builds
• Tracing class is public and exported, with new public API (see below)
• tracing channel constants were moved to core/constants.js and exported

new API:

// enable / disable tracing of channel
pc.Tracing.set(pc.TRACEID_RENDER_PASS, true);

// query status of the tracing channel - true or false
pc.Tracing.get(pc.TRACEID_RENDER_PASS);

[slimbuck]

Should they be exported in debug build only?

[mvaligursky]

Ideally not, for the same reason as Debug.setTrace is enabled in all builds.

from outside, people should be able to have code like

pc.Debug.setTracing(pc.TRACEID_BLAH, true);

in both builds, but it should be a no cost at release. I don't want them to have to comment out the code to run using release.

[slimbuck]

Right, yes of course.

Could be confusing for devs that attempt to use this API and don't realise it only works in debug build.

[slimbuck]

Just checking debug.js I see that actually nothing apart from setTrace is removed in debug builds. We remove the calls to Debug.*** when building the engine itself, but don't remove the debug code.

I actually think all the Debug function implementations should have #if _DEBUG added to them so applications behave the same way.

[mvaligursky]

This part of the build process is responsible for removing the functions, and calls to the functions, from the code base, so there is no need to wrap them using #if _DEBUG:

engine/rollup.config.js

Lines 129 to 144 in d7884b0

	const stripFunctions = [
	'Debug.assert',
	'Debug.call',
	'Debug.deprecated',
	'Debug.warn',
	'Debug.warnOnce',
	'Debug.error',
	'Debug.errorOnce',
	'Debug.log',
	'Debug.logOnce',
	'Debug.trace',
	'DebugHelper.setName',
	'DebugGraphics.pushGpuMarker',
	'DebugGraphics.popGpuMarker',
	'WorldClustersDebug.render'
	];

setTrace are done differently as we don't want to remove them from non-debug builds.

[slimbuck]

Looking at the built build/playcanvas.js - Debug.warn, Debug.asset etc are all still intact?

[slimbuck]

(strip removes the call sites, not the functions themselves).

[mvaligursky]

Hey you're right. I've just tested it .. it strips the functions themselves if I don't export Debug module from the index.js. But as soon as I do that, they're no longer stripped - which kinda makes sense.
I'll add ifdefs now that we need to export it.

[mvaligursky]

For example, class DebugHelper in the same file is not exported from index, and is completely stripped.

[mvaligursky]

What I'm thinking is .. keep class Debug not exported by index, so that it gets completely stripped without ifdefs.
And add exported class which is never stripped .. and is exported and is the public interface for this.

pc.Tracing.enable(channel);
pc.Tracing.disable(channel);

@slimbuck @willeastcott - what do you think?

[slimbuck]

TBH in general I don't think we should expose empty/non-functional API.

I actually think we should either:

• keep the Debug API in all builds and expose it publicly (but continue to remove engine usage in non-debug builds)
• remove the Debug API from public interface entirely and let apps roll their own. Trace control only exported in debug build (users shouldn't include calls to trace control in release code anyway).

[mvaligursky]

• remove the Debug API from public interface entirely and let apps roll their own. Trace control only exported in debug build (users shouldn't include calls to trace control in release code anyway).

we only export Debug API set/getTrace, nothing else. Its purpose is to allow them to 'see' under the hood of the engine, to help them understand what happens. It does not give them a logging API they can use to log from their scripts at all.

And I think it's very convenient to be able to use those functions in any build (even though in non-debug build they are ignored). Even I use them in the engine when I work on things (that way they work with any example I run), and the code does not fail to run in release build.

[slimbuck]

Surely enabling trace in an application is a something temporary you do when developing and not something you'd wish to keep in application code?

[mvaligursky]

It is, but having it in the code should not stop you running the build with the release engine. Having to go and comment it out is not user friendly, especially if you have few in different places.

So far all the APIs we have work with any engine build. For example even the profiling stats classes exist .. but their members (timings) are simply not updated.

[mvaligursky]

I've made changes I suggested. Opinions?
@slimbuck @willeastcott

[willeastcott]

Lovely. ❤️