Skip to content

playing with adfighter#218

Draft
skyl wants to merge 1 commit into
mainfrom
adfighter
Draft

playing with adfighter#218
skyl wants to merge 1 commit into
mainfrom
adfighter

Conversation

@skyl

@skyl skyl commented Mar 20, 2026

Copy link
Copy Markdown
Contributor

User description

meh, IDK


PR Type

Enhancement


Description

  • Add immersive ad-world Babylon pack

  • Add interactive billboards and mobile controls

  • Integrate GPT web ads and AdMob

  • Bridge native ads through Tauri plugin


Diagram Walkthrough

flowchart LR
  controls["First-person controls and HUD"]
  world["Ad World scene orchestration"]
  boards["Interactive billboard surfaces"]
  webads["GPT and Google H5 ads"]
  native["Host API and AdMob plugin"]
  controls -- "drive" --> world
  world -- "renders" --> boards
  boards -- "load content from" --> webads
  world -- "requests ads via" --> native
Loading

File Walkthrough

Relevant files
Enhancement
17 files
world.ts
Compose world systems and ad lifecycle                                     
+113/-0 
FirstPersonController.ts
Implement FPS keyboard, mouse, touch controls                       
+226/-0 
Interaction.ts
Add billboard targeting and ad triggering                               
+139/-0 
MobileHud.ts
Render joystick, hint, and interact button                             
+67/-0   
Environment.ts
Create neon environment lighting and effects                         
+144/-0 
Atmosphere.ts
Add rain and ember particle ambience                                         
+93/-0   
BillboardAdManager.ts
Load GPT ads or billboard placeholders                                     
+79/-0   
InterstitialTrigger.ts
Trigger interstitials from movement zone crossings             
+50/-0   
styles.css
Style HUD, billboards, and mobile controls                             
+132/-0 
googleH5.ts
Integrate Google H5 ad break provider                                       
+141/-0 
gptProvider.ts
Add GPT display slot loading and refresh                                 
+139/-0 
hostProvider.ts
Delegate ad requests to native host                                           
+54/-0   
types.ts
Extend shared host API with ads                                                   
+4/-0     
hostApi.ts
Initialize AdMob and expose ad methods                                     
+38/-0   
lib.rs
Register AdMob plugin commands and setup                                 
+68/-0   
AdmobPlugin.kt
Implement Android interstitial, rewarded, and banner ads 
+288/-0 
AdmobPlugin.swift
Implement iOS interstitial, rewarded, and banner ads         
+275/-0 
Error handling
1 files
Billboards.ts
Build neon billboards with resilient HtmlMesh                       
+144/-0 
Configuration changes
2 files
AdSlotConfig.ts
Define billboard layout, colors, and sizes                             
+128/-0 
lib.rs
Load AdMob plugin into Tauri app                                                 
+1/-0     
Additional files
42 files
release_notes.txt +40/-0   
Cargo.toml +1/-0     
default.json +8/-1     
types.ts +4/-0     
app.css +1/-0     
app.js +6687/-0
index.html +17/-0   
manifest.json +16/-0   
package.json +21/-0   
dev-corpan.mjs +83/-0   
main.ts +41/-0   
vite-env.d.ts +1/-0     
tsconfig.json +18/-0   
vite.config.ts +55/-0   
AdManager.ts +45/-0   
LanguageSignal.ts +46/-0   
adConfig.ts +33/-0   
config.ts +53/-0   
index.ts +2/-0     
index.ts +8/-0     
index.ts +3/-0     
mockProvider.ts +32/-0   
types.ts +28/-0   
Cargo.toml +17/-0   
build.gradle.kts +39/-0   
AndroidManifest.xml +8/-0     
build.rs +16/-0   
Package.swift +32/-0   
hide_banner.toml +13/-0   
init_admob.toml +13/-0   
load_interstitial.toml +13/-0   
load_rewarded.toml +13/-0   
show_banner.toml +13/-0   
show_interstitial.toml +13/-0   
show_rewarded.toml +13/-0   
reference.md +191/-0 
schema.json +384/-0 
commands.rs +58/-0   
desktop.rs +60/-0   
error.rs +21/-0   
mobile.rs +88/-0   
models.rs +36/-0   

@github-actions

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
🧪 No relevant tests
🔒 Security concerns

Sensitive configuration exposure:
corpan/plugins/tauri-plugin-admob/android/src/main/AndroidManifest.xml hardcodes what appears to be a live AdMob application ID. While this is not a secret like a password, it still exposes production monetization identifiers in source control and makes environment separation harder. Please confirm this is intentional and consider injecting environment-specific IDs at build time instead of committing them directly.

⚡ Recommended focus areas for review

Rotation bug

The HtmlMesh offset is computed before applying rotationY, so angled billboards are pushed along world -Z instead of their own facing direction. This can leave the ad surface coplanar with the frame or visibly offset on rotated signs.

htmlMesh.position = new Vector3(cfg.x, cfg.y, cfg.z)
// Slightly in front of frame to prevent z-fighting
const forward = new Vector3(0, 0, -0.02)
forward.rotateByQuaternionAroundPointToRef(
  htmlMesh.rotationQuaternion ?? htmlMesh.rotation.toQuaternion(),
  Vector3.Zero(),
  forward,
)
htmlMesh.position.addInPlace(forward)
htmlMesh.rotation = new Vector3(0, cfg.rotationY, 0)
Global teardown

Cleanup currently destroys all GPT slots on the page, not just the ones created by this provider instance. That can break other packs or components that also use GPT, and it should be validated that disposal is scoped to this provider's own slots and DOM nodes.

const googletag = window.googletag
if (googletag) {
  googletag.cmd.push(() => {
    googletag.destroySlots()
  })
False success

Presentation failures and normal dismissals share the same completion path, so fullscreen ads can be reported as successfully shown even when they failed to present. That can corrupt reward handling and upstream cooldown/state logic.

    func adDidDismissFullScreenContent(_ ad: GADFullScreenPresentingAd) {
        onDismiss()
    }

    func ad(_ ad: GADFullScreenPresentingAd, didFailToPresentFullScreenContentWithError error: Error) {
        print("[ADMOB] Failed to present: \(error.localizedDescription)")
        onDismiss()
    }
}

private class RewardedDelegate: NSObject, GADFullScreenContentDelegate {
    let onDismiss: () -> Void

    init(onDismiss: @escaping () -> Void) {
        self.onDismiss = onDismiss
    }

    func adDidDismissFullScreenContent(_ ad: GADFullScreenPresentingAd) {
        onDismiss()
    }

    func ad(_ ad: GADFullScreenPresentingAd, didFailToPresentFullScreenContentWithError error: Error) {
        print("[ADMOB] Failed to present rewarded: \(error.localizedDescription)")
        onDismiss()

@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Possible issue
Dispose input listeners properly

Call controller.dispose() during teardown so the global keyboard, mouse, and touch
listeners added by createFirstPersonController are actually removed. Without this,
remounting the pack can duplicate input handlers and keep stale listeners alive
after unmount.

corpan/packs/ad-world/src/world.ts [97-110]

 const dispose = () => {
   window.removeEventListener("resize", onResize)
   if (hostApi.hideBanner) {
     hostApi.hideBanner().catch(() => {})
   }
+  controller.dispose()
   mobileHud.dispose()
   adManager.dispose()
   interaction.dispose()
   engine.stopRenderLoop()
   scene.dispose()
   engine.dispose()
   canvas.remove()
   hud.remove()
 }
Suggestion importance[1-10]: 8

__

Why: This is a correct lifecycle fix: createFirstPersonController() registers global input listeners, but world.ts never calls controller.dispose(). Without that cleanup, remounting can leak stale handlers and duplicate keyboard/mouse/touch behavior.

Medium
Wait for existing script

Don't mark the GPT script as loaded just because a matching <script> tag exists. If another
instance inserted the tag but it is still loading, init() can continue with
window.googletag still undefined and permanently skip ad setup.

corpan/packs/shared/ad/displayAds/providers/gptProvider.ts [43-46]

-if (document.querySelector(`script[src="${GPT_SCRIPT_URL}"]`)) {
-  scriptLoaded = true
-  return Promise.resolve()
+const existingScript = document.querySelector<HTMLScriptElement>(`script[src="${GPT_SCRIPT_URL}"]`)
+if (existingScript) {
+  return new Promise((resolve, reject) => {
+    if (window.googletag) {
+      scriptLoaded = true
+      resolve()
+      return
+    }
+
+    existingScript.addEventListener("load", () => {
+      scriptLoaded = true
+      resolve()
+    }, { once: true })
+
+    existingScript.addEventListener("error", () => {
+      reject(new Error("GPT script failed to load"))
+    }, { once: true })
+  })
 }
Suggestion importance[1-10]: 8

__

Why: The suggestion correctly identifies a race in loadScript(): a matching GPT <script> tag can exist before window.googletag is ready, causing init() to exit early and never finish setup. Waiting for the existing script to actually load materially improves reliability of gptProvider.ts.

Medium
Report presentation failures correctly

didFailToPresentFullScreenContentWithError currently routes through the same success
path as a normal dismissal, so failed interstitials are reported as shown: true.
Split failure handling into its own closure and resolve an error result instead; the
same pattern should also be applied to RewardedDelegate.

corpan/plugins/tauri-plugin-admob/ios/Sources/AdmobPlugin.swift [67-240]

-let delegate = InterstitialDelegate { [weak self] in
-    // Ad dismissed — preload next one
-    self?.interstitialAd = nil
-    self?.preloadInterstitial(adUnitId: nil)
-    invoke.resolve(["shown": true, "rewarded": false])
-}
+let delegate = InterstitialDelegate(
+    onDismiss: { [weak self] in
+        self?.interstitialAd = nil
+        self?.preloadInterstitial(adUnitId: nil)
+        invoke.resolve(["shown": true, "rewarded": false])
+    },
+    onFail: { [weak self] message in
+        self?.interstitialAd = nil
+        self?.preloadInterstitial(adUnitId: nil)
+        invoke.resolve(["shown": false, "rewarded": false, "error": message])
+    }
+)
 
 // Store delegate to prevent deallocation
 objc_setAssociatedObject(ad, "delegate", delegate, .OBJC_ASSOCIATION_RETAIN_NONATOMIC)
 ad.fullScreenContentDelegate = delegate
 
 ad.present(fromRootViewController: rootVC)
 ...
 private class InterstitialDelegate: NSObject, GADFullScreenContentDelegate {
     let onDismiss: () -> Void
+    let onFail: (String) -> Void
 
-    init(onDismiss: @escaping () -> Void) {
+    init(onDismiss: @escaping () -> Void, onFail: @escaping (String) -> Void) {
         self.onDismiss = onDismiss
+        self.onFail = onFail
     }
 
     func adDidDismissFullScreenContent(_ ad: GADFullScreenPresentingAd) {
         onDismiss()
     }
 
     func ad(_ ad: GADFullScreenPresentingAd, didFailToPresentFullScreenContentWithError error: Error) {
         print("[ADMOB] Failed to present: \(error.localizedDescription)")
-        onDismiss()
+        onFail(error.localizedDescription)
     }
 }
Suggestion importance[1-10]: 8

__

Why: This catches a real correctness bug in AdmobPlugin.swift: didFailToPresentFullScreenContentWithError currently funnels into the same onDismiss() path and reports shown: true on failure. Separating failure handling would make interstitial results accurate, and the same issue likely exists for RewardedDelegate as noted.

Medium
Externalize AdMob application ID

Hardcoding a real AdMob application ID in the plugin forces every consuming app and
environment to send traffic to the same account, including development builds. Use a
manifest placeholder so the host app must inject the correct ID per build variant.

corpan/plugins/tauri-plugin-admob/android/src/main/AndroidManifest.xml [4-6]

 <meta-data
     android:name="com.google.android.gms.ads.APPLICATION_ID"
-    android:value="ca-app-pub-8772856451250581~7684457708" />
+    android:value="${admobApplicationId}" />
Suggestion importance[1-10]: 8

__

Why: The hardcoded com.google.android.gms.ads.APPLICATION_ID makes the plugin unusably tied to a single AdMob account across all consuming apps and environments. Using a manifest placeholder is a strong correctness and configurability improvement for tauri-plugin-admob.

Medium
Security
Restrict development server exposure

Binding the dev server to 0.0.0.0 exposes the entire packsRoot directory to every
machine on the local network. Default this to 127.0.0.1 and only allow broader
exposure through an explicit environment override.

corpan/packs/ad-world/scripts/dev-corpan.mjs [66-71]

+const serverHost = process.env.CORPAN_DEV_HOST ?? "127.0.0.1"
 const server = run(
   "python3",
-  ["-m", "http.server", "8989", "--bind", "0.0.0.0"],
+  ["-m", "http.server", "8989", "--bind", serverHost],
   packsRoot,
   "server"
 )
Suggestion importance[1-10]: 7

__

Why: This correctly identifies that binding the Python dev server to 0.0.0.0 exposes packsRoot to the local network by default. Switching to 127.0.0.1 with an opt-in override is a meaningful hardening change for a development-only script.

Medium
Remove global ad permissions

Adding every admob:* permission to the global default capability gives all windows
access to ad-loading and ad-display commands. Keep these permissions out of
default.json and grant them only from a dedicated capability attached to the
specific trusted window that should use ads.

corpan/corpan-app/src-tauri/capabilities/default.json [13-20]

-"audio-keepalive:default",
-"admob:allow-init-admob",
-"admob:allow-load-interstitial",
-"admob:allow-show-interstitial",
-"admob:allow-load-rewarded",
-"admob:allow-show-rewarded",
-"admob:allow-show-banner",
-"admob:allow-hide-banner"
+"audio-keepalive:default"
Suggestion importance[1-10]: 6

__

Why: This is a valid least-privilege concern because putting all admob:* permissions in default.json grants them to every window using the default capability. It improves security posture, though whether it is necessary depends on the app's intended window trust model.

Low

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant