Merged
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughREADME Quick Start was changed to defer scene acquisition and setup until a browser Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Ladypoly
approved these changes
Mar 23, 2026
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@README.md`:
- Around line 11-39: Wrap the initialization so it handles events that may have
already fired: call BS.BanterScene.GetInstance() and check scene.unityLoaded
first and run the setup immediately if true; otherwise register
scene.On("unity-loaded", ...) as before; likewise replace the single
window.addEventListener("bs-loaded", ...) usage with code that checks if the
"bs-loaded" event has already occurred (or if the global flag the library
exposes indicates readiness) and runs the same handler immediately, falling back
to window.addEventListener("bs-loaded", ...) when not yet fired—ensure the same
setup logic that creates the GameObject, adds components (BanterSphere,
BanterMaterial, SphereCollider, BanterRigidbody) and the click handler is
extracted to a single function and invoked from both the immediate check and the
event listeners.
Comment on lines
+11
to
39
| window.addEventListener("bs-loaded", ()=> { | ||
| const scene = BS.BanterScene.GetInstance(); | ||
|
|
||
| // Wait for the scene to be ready | ||
| scene.On("unity-loaded", () => { | ||
|
|
||
| // Create a simple object with a red sphere | ||
| const sphere = new BS.GameObject({ | ||
| name: "MySphere", | ||
| layer: BS.L.UI, // Layer 5 for UI. | ||
| localPosition: new BS.Vector3(0, 1.5, 2) | ||
| }); | ||
|
|
||
| // Add visual geometry | ||
| sphere.AddComponent(new BS.BanterSphere({ radius: 0.5 })); | ||
| sphere.AddComponent(new BS.BanterMaterial({ | ||
| color: new BS.Vector4(1, 0, 0, 1) | ||
| })); | ||
|
|
||
| // Add physics | ||
| sphere.AddComponent(new BS.SphereCollider({ radius: 0.5 })); | ||
| sphere.AddComponent(new BS.BanterRigidbody({ mass: 1, useGravity: true })); | ||
|
|
||
| // Handle clicks | ||
| sphere.On("click", (e) => { | ||
| console.log("Clicked at:", e.detail.point); | ||
| }); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Find bs-loaded/unity-loaded usage and readiness checks =="
rg -n -C2 'bs-loaded|unity-loaded|unityLoaded|BanterScene\.GetInstance\(' README.md
rg -n -C2 'bs-loaded|unity-loaded|unityLoaded|BanterScene\.GetInstance\(' .Repository: SideQuestVR/BanterSDK
Length of output: 4536
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Check for unityLoaded property usage patterns ==="
rg -n 'unityLoaded' --type md --type js
echo ""
echo "=== Look for resilient initialization patterns ==="
rg -n -B3 -A3 'scene\.On.*unity-loaded' --type md --type js | head -60
echo ""
echo "=== Check for once: true event listeners ==="
rg -n 'once.*true|addEventListener.*once' --type md --type jsRepository: SideQuestVR/BanterSDK
Length of output: 1160
Quick Start example lacks fallback for already-fired bs-loaded and unity-loaded events
The listener-only pattern at lines 11–39 will silently fail if either event fires before the listener is registered. The documented unityLoaded property provides a resilient fallback, but no code examples demonstrate this pattern.
Suggested fix:
Resilient Quick Start initialization
- window.addEventListener("bs-loaded", ()=> {
- const scene = BS.BanterScene.GetInstance();
-
- // Wait for the scene to be ready
- scene.On("unity-loaded", () => {
+ const init = () => {
+ const scene = BS.BanterScene.GetInstance();
+
+ const onUnityReady = () => {
// Create a simple object with a red sphere
const sphere = new BS.GameObject({
name: "MySphere",
layer: BS.L.UI, // Layer 5 for UI.
localPosition: new BS.Vector3(0, 1.5, 2)
});
// Add visual geometry
sphere.AddComponent(new BS.BanterSphere({ radius: 0.5 }));
sphere.AddComponent(new BS.BanterMaterial({
color: new BS.Vector4(1, 0, 0, 1)
}));
// Add physics
sphere.AddComponent(new BS.SphereCollider({ radius: 0.5 }));
sphere.AddComponent(new BS.BanterRigidbody({ mass: 1, useGravity: true }));
// Handle clicks
sphere.On("click", (e) => {
console.log("Clicked at:", e.detail.point);
});
- });
-});
+ };
+
+ if (scene.unityLoaded) onUnityReady();
+ else scene.On("unity-loaded", onUnityReady);
+ };
+
+ if (window.BS?.BanterScene) init();
+ else window.addEventListener("bs-loaded", init, { once: true });📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| window.addEventListener("bs-loaded", ()=> { | |
| const scene = BS.BanterScene.GetInstance(); | |
| // Wait for the scene to be ready | |
| scene.On("unity-loaded", () => { | |
| // Create a simple object with a red sphere | |
| const sphere = new BS.GameObject({ | |
| name: "MySphere", | |
| layer: BS.L.UI, // Layer 5 for UI. | |
| localPosition: new BS.Vector3(0, 1.5, 2) | |
| }); | |
| // Add visual geometry | |
| sphere.AddComponent(new BS.BanterSphere({ radius: 0.5 })); | |
| sphere.AddComponent(new BS.BanterMaterial({ | |
| color: new BS.Vector4(1, 0, 0, 1) | |
| })); | |
| // Add physics | |
| sphere.AddComponent(new BS.SphereCollider({ radius: 0.5 })); | |
| sphere.AddComponent(new BS.BanterRigidbody({ mass: 1, useGravity: true })); | |
| // Handle clicks | |
| sphere.On("click", (e) => { | |
| console.log("Clicked at:", e.detail.point); | |
| }); | |
| }); | |
| }); | |
| const init = () => { | |
| const scene = BS.BanterScene.GetInstance(); | |
| const onUnityReady = () => { | |
| // Create a simple object with a red sphere | |
| const sphere = new BS.GameObject({ | |
| name: "MySphere", | |
| layer: BS.L.UI, // Layer 5 for UI. | |
| localPosition: new BS.Vector3(0, 1.5, 2) | |
| }); | |
| // Add visual geometry | |
| sphere.AddComponent(new BS.BanterSphere({ radius: 0.5 })); | |
| sphere.AddComponent(new BS.BanterMaterial({ | |
| color: new BS.Vector4(1, 0, 0, 1) | |
| })); | |
| // Add physics | |
| sphere.AddComponent(new BS.SphereCollider({ radius: 0.5 })); | |
| sphere.AddComponent(new BS.BanterRigidbody({ mass: 1, useGravity: true })); | |
| // Handle clicks | |
| sphere.On("click", (e) => { | |
| console.log("Clicked at:", e.detail.point); | |
| }); | |
| }; | |
| if (scene.unityLoaded) onUnityReady(); | |
| else scene.On("unity-loaded", onUnityReady); | |
| }; | |
| if (window.BS?.BanterScene) init(); | |
| else window.addEventListener("bs-loaded", init, { once: true }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@README.md` around lines 11 - 39, Wrap the initialization so it handles events
that may have already fired: call BS.BanterScene.GetInstance() and check
scene.unityLoaded first and run the setup immediately if true; otherwise
register scene.On("unity-loaded", ...) as before; likewise replace the single
window.addEventListener("bs-loaded", ...) usage with code that checks if the
"bs-loaded" event has already occurred (or if the global flag the library
exposes indicates readiness) and runs the same handler immediately, falling back
to window.addEventListener("bs-loaded", ...) when not yet fired—ensure the same
setup logic that creates the GameObject, adds components (BanterSphere,
BanterMaterial, SphereCollider, BanterRigidbody) and the click handler is
extracted to a single function and invoked from both the immediate check and the
event listeners.
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
2 participants
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.
Summary by CodeRabbit