Skip to content

Conversation

@97saundersj
Copy link
Owner

No description provided.

Copilot AI review requested due to automatic review settings October 18, 2025 14:54
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR integrates Firebase Storage uploads into the WebGL template and tweaks UI behavior for safer card viewing flow.

  • Add Firebase SDK (compat) initialization and client-side image upload to Firebase Storage
  • Add file input UI and loading indicator; disable/enable relevant buttons during async operations
  • Update WebGL template selection and minor Unity/HTML template adjustments

Reviewed Changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 6 comments.

Show a summary per file
File Description
README.md Adds local WebGL serving instructions with CORS, aiding local development.
ProjectSettings/TimelineSettings.asset Adds Unity Timeline settings; project metadata.
ProjectSettings/ProjectSettings.asset Switches WebGL template to built-in Minimal, affecting which HTML template is used at build time.
Assets/WebGLTemplates/XRCardTemplate/viewCard.html Disables View Card button until Unity has initialized.
Assets/WebGLTemplates/XRCardTemplate/index.html Adds Firebase SDK/config, file upload logic, and related UI.
Assets/Resources/Card/CardTextController.cs Removes CORS proxy; now requests the image URL directly.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

webGLEmscriptenArgs:
webGLModulesDirectory:
webGLTemplate: PROJECT:XRCardTemplate
webGLTemplate: APPLICATION:Minimal
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing the WebGL template to APPLICATION:Minimal means the build will use the built-in Minimal template and ignore your custom Assets/WebGLTemplates/XRCardTemplate/index.html and viewCard.html (including the Firebase scripts and UI changes). Switch this back to the project template so your updated HTML is applied at runtime.

Suggested change
webGLTemplate: APPLICATION:Minimal
webGLTemplate: PROJECT:XRCardTemplate

Copilot uses AI. Check for mistakes.
Comment on lines +16 to +26
const firebaseConfig = {
apiKey: "AIzaSyBIuTnTru0iILMqOyKRzu2mZtIBnxEhdug",
authDomain: "fir-test-d319a.firebaseapp.com",
projectId: "fir-test-d319a",
storageBucket: "fir-test-d319a.firebasestorage.app",
messagingSenderId: "913686234554",
appId: "1:913686234554:android:0b20d5d7e441a710"
};
firebase.initializeApp(firebaseConfig);
const storage = firebase.storage();
</script>
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Firebase config is not valid for the web SDK: storageBucket should be the bucket name (e.g., "fir-test-d319a.appspot.com"), not the download domain, and appId should be the Web App ID (ends with ":web:..."), not an Android app ID. Use the exact Web App config from the Firebase Console; otherwise firebase.storage() operations will fail at runtime.

Copilot uses AI. Check for mistakes.
Comment on lines +181 to +182
const storageRef = storage.ref('images/' + file.name);
const uploadTask = storageRef.put(file);
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the raw client file name as the Storage object key risks collisions and exposes user-provided filenames. Generate a unique object name (for example, prefix with a UUID/timestamp) and normalize the filename before upload.

Copilot uses AI. Check for mistakes.
Comment on lines +171 to +173
const generateCardButton = document.querySelector('.btn-primary');
const loadingIcon = document.getElementById('loadingIcon');
if (!file) {
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Selecting the action button via the generic .btn-primary selector is brittle and may target the wrong element if there are multiple primary buttons. Assign a dedicated id (e.g., "generateCardButton") to the intended button and select it with getElementById for reliability.

Copilot uses AI. Check for mistakes.
using (UnityWebRequest uwr = UnityWebRequestTexture.GetTexture(url))
{
// Add the required headers
uwr.SetRequestHeader("Origin", "http://localhost");
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hardcoding the Origin header to http://localhost will fail CORS checks when deployed to any other host and, in WebGL, browsers ignore attempts to override the Origin header. Remove this header override and rely on proper CORS on the image host (Firebase Storage download URLs already return appropriate CORS headers).

Suggested change
uwr.SetRequestHeader("Origin", "http://localhost");

Copilot uses AI. Check for mistakes.
Comment on lines +168 to +176
function uploadImage() {
const fileInput = document.getElementById('customImageFile');
const file = fileInput.files[0];
const generateCardButton = document.querySelector('.btn-primary');
const loadingIcon = document.getElementById('loadingIcon');
if (!file) {
alert('Please select an image file to upload.');
return;
}
Copy link

Copilot AI Oct 18, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The upload runs unauthenticated; with default Firebase Storage rules this will fail, and loosening rules to allow public writes would expose the bucket to abuse. Add Firebase Auth (e.g., signInAnonymously) before uploads and enforce restrictive Storage rules (path scoping, file type/size limits) to protect the bucket.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants