ENG-2764: Upgrade Unity Plugin to improve automotive UX#159
ENG-2764: Upgrade Unity Plugin to improve automotive UX#159marc-n-dream wants to merge 1 commit into
Conversation
marc-n-dream
left a comment
There was a problem hiding this comment.
🟢 Jira ENG-2764 fetched — diff checked against its scope.
Nothing blocking. The render-process-gone recovery is genuinely careful — suppress flag, pending-on-disable replay, full state re-application, and a reload cap that resets on a real load are all thought through. Three things worth an eyeball before a human signs off: a dev scripting-define that hitched a ride, one asymmetry in the pause path (inline), and an entire platform quietly leaving the building.
Scope vs ENG-2764. All three ticket items land: recreate on CallOnRenderProcessGone, the WebViewCallback.onRenderProcessGone → CallOnRenderProcessGone bridge, and teardown suppression on Destroy / OnDisable / OnApplicationQuit. It compiles and no in-repo caller breaks — AirConsole.Init(...)'s trailing true, false still map to transparent / zoom after the iOS params were dropped.
Out of scope — iOS removed wholesale. The PR also deletes all iOS support: WebView.mm, WebViewWithUIWebView.mm, every UNITY_IPHONE branch, the public LoadHTML, the iOS Init params, and the iOS block in the build postprocessor. Nothing in this repo calls those, so it builds — but it is a backward-incompatible API change that appears in neither the ticket nor the PR body. Anyone consuming this plugin on iOS loses it silently. If deliberate, say so in the PR body; a one-sentence body for a ~4.5k-line diff that removes a platform is doing a lot of quiet work.
Title is fine (ticket prefix + summary). Human approval still required — this review only comments.
| webGLPowerPreference: 2 | ||
| scriptingDefineSymbols: | ||
| Android: | ||
| Android: AIRCONSOLE_DEVELOPMENT |
There was a problem hiding this comment.
AIRCONSOLE_DEVELOPMENT ships in the project's Android scripting defines, which flips _logPlatformMessages = true unconditionally in AirConsole.cs:2281. Reads like a local-testing leftover rather than an intended default for the example project. Drop it unless every Android build here is meant to force platform-message logging.
| Android: AIRCONSOLE_DEVELOPMENT | |
| Android: |
| /// Called when the component is disabled. Flushes any remaining events in the queue. | ||
| /// </summary> | ||
| private void OnDisable() { | ||
| mSuppressRenderProcessRecovery = true; |
There was a problem hiding this comment.
OnDisable (this line) and OnApplicationQuit set this flag, but OnApplicationPause (lines 302–312) does not. The pause handler force-drains the queue (this.paused = false; ProcessEventQueue();), so a renderer-death event queued as the app backgrounds is dispatched right here and triggers a full native WebView recreate + LoadURL mid-pause — the OnApplicationPause↔native overlap that scope item #2 set out to avoid, and the exact window the Update() NOTE warns webView.Call(...) can crash. If recovery should defer during pause like it does on disable, set the flag here too and replay on resume — note the pending-replay currently only fires in OnEnable, not on OnApplicationPause(false), so a naive suppress would strand the recovery. Can't demonstrate a crash, so non-blocking.
This addresses multiple identified limitations with the AirConsole - Game navigation in specific automotive environments.