search

floooh / sokol-zig

Zig bindings for the sokol headers (https://github.com/floooh/sokol)
zlib License
341 stars 46 forks source link

Optional log/assert hooks implemented in Zig? #33

Open michaelbartnett opened 1 year ago

michaelbartnett commented 1 year ago

I've been hanging onto these local changes for a while and figured I should offer them forward.

Setting the Sokol log macro to use a log function implemented in Zig means that if you happen to use std.log.info your formatting will look all nice and consistent.

Even more helpful is implementing the assert hook in Zig, since then Sokol validation errors will hit the Zig panic handler and show you a nice stacktrace even if you aren't attached in VS.

I saw other PRs/issues talking about adding a config/options struct for buildSokol which I imagine would interfere, so this is more of a suggestion/feature-request PR 🙂

floooh commented 1 year ago

It's better to wait with this until I got around to implement the new combined error reporting and logging from sokol_spine.h into all sokol headers (see here: https://github.com/floooh/sokol/blob/2236713b5eb247907a1a0a8c0b4c44f16e472909/util/sokol_spine.h#L827-L875)

...in debug mode, this allows to output 'clickable' and human-readable error messages with file path and line number, and in release mode, it won't contain human readable strings (only error code) to not bloat the executable with unnecessary string data.

Darkyenus commented 3 months ago

It looks like the logging functions are in place now, so it might be a good time to return to this.

Given the logging changes, the logging part of the PR is probably no longer necessary and in my implementation I actually go in the other direction (redirecting zig logging to sokol_log), because sokol_log is better suited to various platform output functions, while std.log just goes to stderr. But the panic/assert redirect is still valuable, because Zig's @panic prints a nice stacktrace, while the current code just aborts.

It would also be nice if sokol_log's panic, which currently aborts, redirected to @panic as well.

Regarding the implementation, just a small nitpick about this line: #define SOKOL_ASSERT(c) sokol_zig_assert((unsigned int)c, #c) wouldn't it be better to do the if check right in the macro? It might be sligtly more performant/optimizable that way.