Route native logging through the log crate - #174
Open
ruccho wants to merge 10 commits into
Open
Conversation
Replace every println!/eprintln! across the unienc crates with the `log` facade, and install a single log::Log implementation in unienc_c that picks a sink per record: - Android: __android_log_write under the `unienc` tag, regardless of the `unity` feature. - Unity (non-Android): IUnityLog, once UnityPluginLoad has provided the plugin interfaces. - Otherwise: stdout/stderr. Remove set_stdout_redirect, which redirected the process-wide stdout and stderr into logcat from JNI_OnLoad. It captured output from every other library in the process and collided with anything else doing the same. Add unienc_set_log_level so the managed side can change the threshold; the default is Info for release builds and Debug for debug builds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The IUnityLog sink added in the previous commit was unreachable on Windows and Linux: build-unienc.yml passed only -F mimalloc for their `unity` variant, so those players fell back to println and nothing reached the Unity console. -F unity did not compile there. `PlatformEncodingSystem::unity_plugin_load` is called unconditionally from unienc_c, but `UnityPlugin` was implemented only by the Metal and Vulkan backends, which are the ones with a rendering integration. Add empty implementations for Media Foundation, FFmpeg, and WebCodecs so the trait's no-op defaults apply. Each backend gets its own `unity` feature forwarding to `unienc_common/unity` rather than depending on it unconditionally, so non-Unity builds of those crates keep excluding unity-native-plugin. wasm stays on --no-default-features: unity-native-plugin-sys 0.9.0 asserts 64-bit struct layouts and cannot compile for a 32-bit target. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolves three conflicts, all where main rewrote code this branch had only converted from println! to log::. - unienc_android_mc/src/common.rs: main moved the JNI calls onto the typed java_api! bindings and added API-level guards, and moved the output-buffer release into the pull loop. Took main's structure throughout and reapplied the log:: conversion on top, including the two println! calls main added that this branch had never seen. The per-frame trace dump moves above the CommonEncodedData construction, since main's version moves encoded_data into it. - unienc_ffmpeg/Cargo.toml and src/mux/mod.rs: main removed the tokio dependency; kept that removal and the log dependency. The harness crates keep println!. unienc_testkit::progress writes to stdout and flushes each line because a device harness runs behind a pipe and may hang, and unienc_harness_android's stdout is the adb shell that invoked it. Routing either through the facade would send them to logcat instead, where the CI step reading harness output would never see them. Adds a logger to unienc_testkit for the opposite reason. CI runs `cargo test -p unienc_testkit -p unienc_common -p <backend>` and only `cargo check`s unienc_c, so the logger the plugin installs is never linked and every backend record during a test would be discarded. The harness logger routes records through progress::report, so they interleave with the progress lines and survive a hang. Its threshold is Debug: Trace carries per-frame dumps that would bury a CI log. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Preferences > UniEnc gets a Native Log Level popup that applies to the Editor's own copy of the plugin and is pushed to every connected development player over PlayerConnection. "Default" stores nothing and calls nothing, so a session that never touches the encoder leaves the native library unloaded — and therefore replaceable on disk. The player asks for the level once it has registered its handler rather than relying on the Editor's connect push alone: a player that starts with the Editor already running is connected before its managed code runs, so that push lands before there is anything to receive it, and PlayerConnection.RegisterConnection only reports connections made from the point of registration on. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A player connection can only deliver a level once the player is running, which is already too late for the records the encoder emits while the first session starts up — on the sample scene that is the whole of MFT selection. Baking the level in covers those, and covers release builds, where there is no player connection at all. The level travels as a ScriptableObject under a generated Resources folder, created in preprocess and deleted in postprocess. Resources because it is the only load path that is synchronous on every platform; StreamingAssets is not, on Android, which is a platform this matters on. Assets/ because a package folder is read-only for anyone consuming this package from a registry. Baking is off by default and needs both a configured level and the toggle, so a level chosen to debug the Editor does not silently follow a build out of the door, and a build machine that never set one bakes nothing. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Both still told the reader that a release build of the plugin defaults to Info. The same claim in `unienc_set_log_level`'s own doc comment is left alone: csbindgen copies it into NativeMethods.g.cs, so correcting it there needs a native rebuild to keep the two in step. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The sink chain is now IUnityLog, then logcat, then stdio, with Android differing from every other platform only in what it falls back to. Android wrote to logcat unconditionally on the reasoning that logcat is where native failures get diagnosed. But Unity's own log destination on Android already is logcat, under the `Unity` tag, so preferring IUnityLog gives up no destination and gains the ones only Unity reaches: the Editor console, and the console of an Editor attached to a development player. The tag changes; the message body keeps its `[unienc]` prefix, so `adb logcat | grep '\[unienc\]'` replaces `adb logcat -s unienc`. The logcat write stays as the fallback rather than being deleted. `JNI_OnLoad` runs long before `UnityPluginLoad`, and Android discards a process's stdout now that set_stdout_redirect is gone, so falling through to stdio there would lose those records outright. Compile-checked for aarch64-linux-android and x86_64-pc-windows-msvc, with and without the feature, and cargo fmt is clean. Not yet exercised on a device. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ruccho
marked this pull request as ready for review
September 1, 2026 05:51
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #170.
What
Replaces every
println!/eprintln!in the native code (47 call sites across six crates) with thelogfacade plus a singlelog::Logimplementation inunienc_c, and adds the Unity-side controlsfor the resulting level.
Sinks
Per record, first match wins:
IUnityLog—unityfeature on andUnityPluginLoadhas run. Android included: Unity's logdestination on Android already is logcat, so this gives up no destination and adds the Editor
console and the console of an Editor attached to a development player.
__android_log_writeunder theunienctag — Android, outside the window above.Log level
Defaults to
Warnfor release builds of the native library andDebugfor debug builds.unienc_set_log_level, exposed asUniEnc.NativeLogging.SetLevel, changes it at runtime. Severalpreviously commented-out per-frame dumps are now live at
trace!.Preferences > UniEncdrives it:over the player connection, so a running player picks up a change.
ResourcesScriptableObject applied at
BeforeSplashScreen— the only way to catch the records a sessionemits while starting up, and the only way a release build gets a level at all.
Defaultstores nothing and calls nothing, leaving the plugin unloaded and replaceable on disk.Breaking changes
Windows and Linux Unity binaries change.
build-unienc.ymlnow passes-F unity,mimallocthereinstead of
-F mimallocalone — without it those builds would silentlyprintlninstead of usingIUnityLog. They now exportUnityPluginLoad/UnityPluginUnloadand Unity calls them, which alsoruns
mimalloc::unity::initand routes mimalloc throughIUnityMemoryManager. That path isexercised on macOS, iOS and Android but not previously on these two. Please validate on real
player builds for both before release.
Making
-F unitycompile there needed emptyUnityPluginimpls for Media Foundation, FFmpeg andWebCodecs, each behind its own
unityfeature sounity-native-pluginstays out of the NuGetbuilds. wasm keeps
--no-default-features.set_stdout_redirectis removed (Android). Itdup2'd stdout/stderr process-wide fromJNI_OnLoad, re-tagging every library's output in the process asunienc. Native code that reachedlogcat only through that redirect now needs its own logging.
UniEnc's own logcat tag changes to Unity's under the
unityfeature. The message body keeps its[unienc]prefix, soadb logcat | grep '\[unienc\]'replacesadb logcat -s unienc.Worth knowing: Unity captures a managed stack trace per record in a development build (
Noneinrelease), so high-volume
trace!on a device is not free. The mitigation,Application.SetStackTraceLogType, is application-global and so not this package's call.Verification
cargo check -p unienc_c --features unitypasses foraarch64-apple-darwin,aarch64-apple-ios,aarch64-linux-android,x86_64-pc-windows-msvcandx86_64-unknown-linux-gnu; without thefeature it passes for macOS, Windows, Linux, wasm and Android.
cargo fmt --checkis clean andCargo.lockgains no new dependencies, soTHIRD-PARTY-NOTICES.mdis unchanged.Exercised in a 6000.2.13f1 Editor on Windows against the
User Interfacessample: records reach theconsole at the right
LogType(including from Media Foundation's own worker threads) with nothingduplicated to the stdio fallback, each level filters as expected, the preference survives a domain
reload, a development player applies the level over the connection both at startup and on change,
and a release player with the level baked in logs the full MFT selection that the connection path
cannot reach.
Not verified on a real Android device — the sink order there is compile-checked only. Also
unverified: the Apple and WebCodecs backends.
Native binaries need a
build-unienc.ymlrun before this is usable from the package.