Fix for reading out text on characters#528
Open
essymbugua wants to merge 7 commits intoflipcomputing:mainfrom
Open
Fix for reading out text on characters#528essymbugua wants to merge 7 commits intoflipcomputing:mainfrom
essymbugua wants to merge 7 commits intoflipcomputing:mainfrom
Conversation
📝 WalkthroughWalkthroughAdds translation-normalized spoken-text resolution, per-object and world text caching, new announcement APIs, intro/suppression windows, and scene-anchor/summary refinements; removes mute controls and adjusts canvas focus/tab behavior and flock integration monkey-patches to route speak/print through the new APIs. Changes
Sequence Diagram(s)sequenceDiagram
participant UserCode as User Code (flock.say / printText)
participant FlockInt as Flock Integration
participant A11y as Accessibility Module
participant Cache as Text Cache
participant Speech as Speech/Announce System
UserCode->>FlockInt: flock.say(target, options)
FlockInt->>A11y: recordObjectPromptText(targetName, text)
A11y->>A11y: cleanSpokenAnnouncement(text)
A11y->>Cache: store cleaned prompt text
UserCode->>FlockInt: flock.printText(message)
FlockInt->>A11y: recordWorldInstructionText(text)
A11y->>A11y: resolveSpokenText(text)
A11y->>Speech: announceSayText(resolved)
Note over UserCode,Speech: Later, on pointer click
UserCode->>A11y: pointer click on object
A11y->>Cache: lookup cached say/prompt text
alt cached text found
Cache-->>A11y: return text
A11y->>Speech: announceSayText(text)
else no cache
A11y->>A11y: getInteractionHint() / describeScene()
A11y->>Speech: announce(resolved)
end
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@accessibility/accessibility.js`:
- Around line 23-29: queuedIntroSayText is declared but never used; either
remove it or wire it into the intro-speech flow. To fix, choose one: (A) delete
the queuedIntroSayText declaration and any related unused introInProgress
handling so ESLint passes; or (B) implement buffering by updating
scheduleInitialIntro() to set queuedIntroSayText when an intro is requested
while introInProgress is true, and modify announceSayText() (or the function
that actually speaks) to prefer draining queuedIntroSayText after the current
intro completes (clearing queuedIntroSayText and setting
hasSpokenInitialPageIntro accordingly). Reference queuedIntroSayText,
introInProgress, announceSayText, and scheduleInitialIntro in your change.
- Around line 673-697: The enrichEnvironmentLabel function is never used, so
either apply it where labels are grouped or remove it; update
buildEnvironmentSummary to call enrichEnvironmentLabel(obj) instead of reading
raw obj.label so the vague-label filtering and nicer formatting run, and filter
out any empty returns before counting/grouping (e.g., skip objects where
enrichEnvironmentLabel(obj) === ""); this will also resolve the ESLint
unused-function complaint for enrichEnvironmentLabel.
- Around line 488-523: The fallback loop currently returns the nearest visible
object as kind: "character" which mislabels props as avatars; update the final
returned object so it does not claim to be a character — e.g., return kind:
"object" (or "prop") with mesh: fallbackMesh and position: fallbackPos instead
of "character", and ensure any caller that treats characters specially (such as
describeCharacterIntro) will only be invoked for true player/character results
(leave callers unchanged here, just change the returned kind from the
accessibility function that uses fallbackMesh/fallbackPos).
In `@flock.js`:
- Around line 1895-1941: The wrapper block that patches flock.say and
flock.printText (guarded by flock._a11yTextWrapped) has inconsistent indentation
and line-wrapping causing Prettier failures; reformat this entire if-block to
match the project's Prettier style (proper indentation, consistent use of
optional chaining and ternaries, spacing, and line breaks) so
flock._a11yTextWrapped, flock.say, originalSay, recordObjectPromptText,
recordObjectSayText, flock.printText, originalPrintText, and announceSayText are
aligned with the rest of the file — or simply run the repository's formatter
(Prettier) on this file to normalize the block.
- Around line 1237-1238: The canvas is being removed from the tab order and
loses its accessible name; restore keyboard focusability by not setting
flock.canvas.tabIndex = -1 (use tabIndex = 0 or remove that assignment) so the
canvas can receive keydown handlers, and re-add the accessible name by
uncommenting or reintroducing flock.canvas.setAttribute("aria-label", "Flock 3D
world canvas") (or another descriptive label) to ensure screen readers and
keyboard users can discover and focus the scene; update any focus/blur handlers
if they rely on a specific tabIndex value.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 6ba0ad44-3ac0-4519-92e1-7378b525bf3d
📒 Files selected for processing (3)
accessibility/accessibility.jsflock.jsindex.html
Comment on lines
+23
to
+29
| let introInProgress = false; | ||
| let queuedIntroSayText = ""; | ||
| let suppressPointerUntil = 0; | ||
| let suppressRuntimeTextUntil = 0; | ||
| let objectSayTextCache = new Map(); | ||
| let objectPromptTextCache = new Map(); | ||
| let hasSpokenInitialPageIntro = false; |
There was a problem hiding this comment.
Remove or implement the intro-speech queue.
queuedIntroSayText is introduced here but never read, so this hunk currently fails ESLint on Line 24. If intro-time speech is meant to be buffered, wire it into announceSayText() / scheduleInitialIntro(); otherwise delete the state.
🧰 Tools
🪛 GitHub Actions: ESLint
[error] 24-24: ESLint: 'queuedIntroSayText' is assigned a value but never used. Allowed unused vars must match /^_/u (no-unused-vars)
🪛 GitHub Check: eslint
[failure] 24-24:
'queuedIntroSayText' is assigned a value but never used. Allowed unused vars must match /^_/u
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@accessibility/accessibility.js` around lines 23 - 29, queuedIntroSayText is
declared but never used; either remove it or wire it into the intro-speech flow.
To fix, choose one: (A) delete the queuedIntroSayText declaration and any
related unused introInProgress handling so ESLint passes; or (B) implement
buffering by updating scheduleInitialIntro() to set queuedIntroSayText when an
intro is requested while introInProgress is true, and modify announceSayText()
(or the function that actually speaks) to prefer draining queuedIntroSayText
after the current intro completes (clearing queuedIntroSayText and setting
hasSpokenInitialPageIntro accordingly). Reference queuedIntroSayText,
introInProgress, announceSayText, and scheduleInitialIntro in your change.
Comment on lines
+488
to
+523
| // Better fallback: nearest non-environment visible object to the camera | ||
| let fallbackMesh = null; | ||
| let fallbackPos = null; | ||
| let bestFallbackDistance = Infinity; | ||
|
|
||
| for (const mesh of scene?.meshes || []) { | ||
| if (!mesh || !mesh.isVisible || !mesh.name) continue; | ||
| if (looksLikeInternalMeshName(mesh.name)) continue; | ||
|
|
||
| const root = getEntityRoot(mesh); | ||
| if (!root || !root.isVisible) continue; | ||
|
|
||
| const label = getObjectLabel(root).toLowerCase(); | ||
| if (isEnvironmentObject(label)) continue; | ||
|
|
||
| const p = getRepresentativePosition(root, mesh); | ||
| if (!p || !cameraPos) continue; | ||
|
|
||
| const dx = p.x - cameraPos.x; | ||
| const dy = p.y - cameraPos.y; | ||
| const dz = p.z - cameraPos.z; | ||
| const d = Math.sqrt(dx * dx + dy * dy + dz * dz); | ||
|
|
||
| if (d < bestFallbackDistance) { | ||
| bestFallbackDistance = d; | ||
| fallbackMesh = root; | ||
| fallbackPos = p; | ||
| } | ||
| } | ||
|
|
||
| if (fallbackMesh && fallbackPos) { | ||
| return { | ||
| kind: "character", | ||
| mesh: fallbackMesh, | ||
| position: fallbackPos | ||
| }; |
There was a problem hiding this comment.
Don’t classify the nearest fallback object as a character.
Lines 518-523 return kind: "character" even when no player/character was found and this is just the nearest visible prop. describeCharacterIntro() treats that as the avatar, so scenes without explicit character metadata can announce arbitrary scenery as the player.
Suggested fix
if (fallbackMesh && fallbackPos) {
return {
- kind: "character",
+ kind: "object",
mesh: fallbackMesh,
- position: fallbackPos
+ position: fallbackPos,
};
}📝 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
| // Better fallback: nearest non-environment visible object to the camera | |
| let fallbackMesh = null; | |
| let fallbackPos = null; | |
| let bestFallbackDistance = Infinity; | |
| for (const mesh of scene?.meshes || []) { | |
| if (!mesh || !mesh.isVisible || !mesh.name) continue; | |
| if (looksLikeInternalMeshName(mesh.name)) continue; | |
| const root = getEntityRoot(mesh); | |
| if (!root || !root.isVisible) continue; | |
| const label = getObjectLabel(root).toLowerCase(); | |
| if (isEnvironmentObject(label)) continue; | |
| const p = getRepresentativePosition(root, mesh); | |
| if (!p || !cameraPos) continue; | |
| const dx = p.x - cameraPos.x; | |
| const dy = p.y - cameraPos.y; | |
| const dz = p.z - cameraPos.z; | |
| const d = Math.sqrt(dx * dx + dy * dy + dz * dz); | |
| if (d < bestFallbackDistance) { | |
| bestFallbackDistance = d; | |
| fallbackMesh = root; | |
| fallbackPos = p; | |
| } | |
| } | |
| if (fallbackMesh && fallbackPos) { | |
| return { | |
| kind: "character", | |
| mesh: fallbackMesh, | |
| position: fallbackPos | |
| }; | |
| // Better fallback: nearest non-environment visible object to the camera | |
| let fallbackMesh = null; | |
| let fallbackPos = null; | |
| let bestFallbackDistance = Infinity; | |
| for (const mesh of scene?.meshes || []) { | |
| if (!mesh || !mesh.isVisible || !mesh.name) continue; | |
| if (looksLikeInternalMeshName(mesh.name)) continue; | |
| const root = getEntityRoot(mesh); | |
| if (!root || !root.isVisible) continue; | |
| const label = getObjectLabel(root).toLowerCase(); | |
| if (isEnvironmentObject(label)) continue; | |
| const p = getRepresentativePosition(root, mesh); | |
| if (!p || !cameraPos) continue; | |
| const dx = p.x - cameraPos.x; | |
| const dy = p.y - cameraPos.y; | |
| const dz = p.z - cameraPos.z; | |
| const d = Math.sqrt(dx * dx + dy * dy + dz * dz); | |
| if (d < bestFallbackDistance) { | |
| bestFallbackDistance = d; | |
| fallbackMesh = root; | |
| fallbackPos = p; | |
| } | |
| } | |
| if (fallbackMesh && fallbackPos) { | |
| return { | |
| kind: "object", | |
| mesh: fallbackMesh, | |
| position: fallbackPos, | |
| }; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@accessibility/accessibility.js` around lines 488 - 523, The fallback loop
currently returns the nearest visible object as kind: "character" which
mislabels props as avatars; update the final returned object so it does not
claim to be a character — e.g., return kind: "object" (or "prop") with mesh:
fallbackMesh and position: fallbackPos instead of "character", and ensure any
caller that treats characters specially (such as describeCharacterIntro) will
only be invoked for true player/character results (leave callers unchanged here,
just change the returned kind from the accessibility function that uses
fallbackMesh/fallbackPos).
Comment on lines
+673
to
+697
| function enrichEnvironmentLabel(obj) { | ||
| const raw = String(obj?.label || "").trim(); | ||
| const label = raw.toLowerCase(); | ||
|
|
||
| // Hide vague or unhelpful labels | ||
| if (!label) return ""; | ||
| if ( | ||
| label === "environment" || | ||
| label === "scene" || | ||
| label === "object" || | ||
| label === "mesh" || | ||
| label === "ground" || | ||
| label === "floor" || | ||
| label === "terrain" || | ||
| label === "sky" | ||
| ) { | ||
| return ""; | ||
| } | ||
|
|
||
| // Make some common labels sound nicer | ||
| if (obj?.isSkyLike && raw) return raw; | ||
| if (obj?.isGroundLike && raw) return raw; | ||
|
|
||
| return raw; | ||
| } |
There was a problem hiding this comment.
Apply enrichEnvironmentLabel() or drop it.
buildEnvironmentSummary() still groups raw obj.label values, so the new cleanup logic never runs. That leaves the vague-label filtering inactive and also keeps the ESLint failure on Line 673 for an unused function.
Suggested fix
for (const obj of objects) {
- const label = normalise(obj.label);
+ const label = normalise(enrichEnvironmentLabel(obj));
const labelLower = lower(label);
if (!label) continue;Also applies to: 699-775
🧰 Tools
🪛 GitHub Check: eslint
[failure] 673-673:
'enrichEnvironmentLabel' is defined but never used. Allowed unused vars must match /^_/u
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@accessibility/accessibility.js` around lines 673 - 697, The
enrichEnvironmentLabel function is never used, so either apply it where labels
are grouped or remove it; update buildEnvironmentSummary to call
enrichEnvironmentLabel(obj) instead of reading raw obj.label so the vague-label
filtering and nicer formatting run, and filter out any empty returns before
counting/grouping (e.g., skip objects where enrichEnvironmentLabel(obj) === "");
this will also resolve the ESLint unused-function complaint for
enrichEnvironmentLabel.
Comment on lines
+1237
to
+1238
| flock.canvas.tabIndex = -1; | ||
| //flock.canvas.setAttribute("aria-label", "Flock 3D world canvas"); |
There was a problem hiding this comment.
Keep the scene canvas keyboard-focusable.
Line 1237 removes the only element receiving the canvas keydown handlers from the tab order, and Line 1238 drops its accessible name at the same time. That breaks the Arrow/WASD/Space flow the accessibility help text still advertises and leaves keyboard-only users with no reliable way to reach the scene.
Suggested fix
- flock.canvas.tabIndex = -1;
- //flock.canvas.setAttribute("aria-label", "Flock 3D world canvas");
+ flock.canvas.tabIndex = 0;
+ flock.canvas.setAttribute("aria-label", "Flock 3D world canvas");📝 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
| flock.canvas.tabIndex = -1; | |
| //flock.canvas.setAttribute("aria-label", "Flock 3D world canvas"); | |
| flock.canvas.tabIndex = 0; | |
| flock.canvas.setAttribute("aria-label", "Flock 3D world canvas"); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flock.js` around lines 1237 - 1238, The canvas is being removed from the tab
order and loses its accessible name; restore keyboard focusability by not
setting flock.canvas.tabIndex = -1 (use tabIndex = 0 or remove that assignment)
so the canvas can receive keydown handlers, and re-add the accessible name by
uncommenting or reintroducing flock.canvas.setAttribute("aria-label", "Flock 3D
world canvas") (or another descriptive label) to ensure screen readers and
keyboard users can discover and focus the scene; update any focus/blur handlers
if they rely on a specific tabIndex value.
Comment on lines
+1895
to
+1941
| // Announce "say" and "printText" outputs so NVDA reads Blockly say blocks reliably. | ||
| if (!flock._a11yTextWrapped) { | ||
| // Wrap say(text, ...) | ||
| if (typeof flock.say === "function") { | ||
| const originalSay = flock.say.bind(flock); | ||
| flock.say = (...args) => { | ||
| const result = originalSay(...args); | ||
|
|
||
| const targetName = args?.[0]; | ||
| const options = args?.[1]; | ||
| const text = | ||
| options && typeof options.text === "string" ? options.text : ""; | ||
|
|
||
| if (text.trim()) { | ||
| // Keep the first prompt text, e.g. "Click or tap me" | ||
| recordObjectPromptText(targetName, text); | ||
|
|
||
| // Keep general say text too | ||
| recordObjectSayText(targetName, text); | ||
|
|
||
| } | ||
|
|
||
| return result; | ||
| }; | ||
| } | ||
|
|
||
| // Wrap printText({ text: "..." }) | ||
| if (typeof flock.printText === "function") { | ||
| const originalPrintText = flock.printText.bind(flock); | ||
| flock.printText = (...args) => { | ||
| const result = originalPrintText(...args); | ||
|
|
||
| const payload = args?.[0]; | ||
| const text = | ||
| typeof payload === "string" | ||
| ? payload | ||
| : (payload && typeof payload.text === "string" ? payload.text : ""); | ||
|
|
||
| if (text && text.trim()) { | ||
| announceSayText(text); | ||
| } | ||
| return result; | ||
| }; | ||
| } | ||
|
|
||
| flock._a11yTextWrapped = true; | ||
| } |
There was a problem hiding this comment.
Run the formatter on this wrapper block.
This hunk is very likely the current Prettier failure: the indentation and wrapping are outside the rest of the file’s format.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@flock.js` around lines 1895 - 1941, The wrapper block that patches flock.say
and flock.printText (guarded by flock._a11yTextWrapped) has inconsistent
indentation and line-wrapping causing Prettier failures; reformat this entire
if-block to match the project's Prettier style (proper indentation, consistent
use of optional chaining and ternaries, spacing, and line breaks) so
flock._a11yTextWrapped, flock.say, originalSay, recordObjectPromptText,
recordObjectSayText, flock.printText, originalPrintText, and announceSayText are
aligned with the rest of the file — or simply run the repository's formatter
(Prettier) on this file to normalize the block.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
accessibility/accessibility.js (3)
703-703:⚠️ Potential issue | 🟠 MajorApply
enrichEnvironmentLabel()in summary grouping.Line 703 defines
enrichEnvironmentLabel()but Line 744 still groups rawobj.label, leaving the cleanup logic inactive and function unused.Suggested fix
for (const obj of objects) { - const label = normalise(obj.label); + const label = normalise(enrichEnvironmentLabel(obj)); const labelLower = lower(label); if (!label) continue;Also applies to: 744-746
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@accessibility/accessibility.js` at line 703, The summary grouping still uses raw obj.label instead of the cleanup helper; update the summary/grouping code to call enrichEnvironmentLabel(obj) (or use its return value) wherever obj.label is used for grouping so the normalized label is applied (replace instances of obj.label in the summary grouping logic with the result of enrichEnvironmentLabel(obj)); ensure enrichEnvironmentLabel returns the cleaned label and is visible to the grouping code.
548-553:⚠️ Potential issue | 🟠 MajorReturn fallback anchor as an object, not a character.
Line 550 currently marks the nearest fallback prop as
kind: "character", which can misidentify scenery as the player/avatar.Suggested fix
if (fallbackMesh && fallbackPos) { return { - kind: "character", + kind: "object", mesh: fallbackMesh, - position: fallbackPos + position: fallbackPos, }; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@accessibility/accessibility.js` around lines 548 - 553, The return for the fallback anchor is incorrectly labeled as kind: "character"; in the block that checks fallbackMesh && fallbackPos (the object returning mesh: fallbackMesh and position: fallbackPos), change the kind field to the correct non-character type (e.g., "object" or "anchor" consistent with your codebase) so scenery/props aren't misidentified as the player; update any related consumers if they expect a specific string constant.
24-24:⚠️ Potential issue | 🟠 MajorWire
queuedIntroSayTextinto the intro flow (or remove it).Line 24 defines
queuedIntroSayTextbut it is never read, and Line 1115 drops runtime speech during intro/suppression windows. That can lose character/instruction text emitted at startup.Suggested fix
export function announceSayText(text, options = {}) { const resolved = resolveSpokenText(text); const spoken = cleanSpokenAnnouncement(resolved); if (!spoken) return; @@ - if (introInProgress || Date.now() < suppressRuntimeTextUntil) return; + if (introInProgress || Date.now() < suppressRuntimeTextUntil) { + queuedIntroSayText = spoken; + return; + } announce(spoken, options); } @@ const finishIntro = () => { + if (queuedIntroSayText) { + announce(queuedIntroSayText, { force: true }); + queuedIntroSayText = ""; + } introInProgress = false; suppressPointerUntil = 0; suppressRuntimeTextUntil = 0; };Also applies to: 1115-1116, 1141-1145
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@accessibility/accessibility.js` at line 24, queuedIntroSayText is declared but never used, causing startup speech to be dropped during the intro/suppression windows; either remove the unused variable or wire it into the intro flow by buffering any speech emitted while suppression is active into queuedIntroSayText and flushing it once suppression ends. Locate the code that suppresses runtime speech (the intro/suppression logic that currently discards output) and change it to append suppressed text to queuedIntroSayText (or a small queue if multiple messages are possible), then at the end of the intro/suppression window call the existing speak/display routine to emit queuedIntroSayText and clear it; if you prefer removal, delete the queuedIntroSayText declaration and ensure the suppression branch no longer references an unused buffer. Ensure you use the exact symbol queuedIntroSayText when implementing the buffer and call the same speak/display method used elsewhere so behavior remains consistent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@accessibility/accessibility.js`:
- Around line 999-1001: The empty catch in the block that calls
pointerObserverScene.onPointerObservable.remove(pointerObserverRef) should be
replaced with an explicit no-op that documents intent; update the catch to
accept the error (e) and include a short comment or noop rationale (e.g.,
"ignore errors if observer already removed or scene disposed") so ESLint no
longer flags an empty block, keeping pointerObserverRef = null afterward; locate
this in the code paths around pointerObserverScene, pointerObserverRef and
onPointerObservable.remove.
---
Duplicate comments:
In `@accessibility/accessibility.js`:
- Line 703: The summary grouping still uses raw obj.label instead of the cleanup
helper; update the summary/grouping code to call enrichEnvironmentLabel(obj) (or
use its return value) wherever obj.label is used for grouping so the normalized
label is applied (replace instances of obj.label in the summary grouping logic
with the result of enrichEnvironmentLabel(obj)); ensure enrichEnvironmentLabel
returns the cleaned label and is visible to the grouping code.
- Around line 548-553: The return for the fallback anchor is incorrectly labeled
as kind: "character"; in the block that checks fallbackMesh && fallbackPos (the
object returning mesh: fallbackMesh and position: fallbackPos), change the kind
field to the correct non-character type (e.g., "object" or "anchor" consistent
with your codebase) so scenery/props aren't misidentified as the player; update
any related consumers if they expect a specific string constant.
- Line 24: queuedIntroSayText is declared but never used, causing startup speech
to be dropped during the intro/suppression windows; either remove the unused
variable or wire it into the intro flow by buffering any speech emitted while
suppression is active into queuedIntroSayText and flushing it once suppression
ends. Locate the code that suppresses runtime speech (the intro/suppression
logic that currently discards output) and change it to append suppressed text to
queuedIntroSayText (or a small queue if multiple messages are possible), then at
the end of the intro/suppression window call the existing speak/display routine
to emit queuedIntroSayText and clear it; if you prefer removal, delete the
queuedIntroSayText declaration and ensure the suppression branch no longer
references an unused buffer. Ensure you use the exact symbol queuedIntroSayText
when implementing the buffer and call the same speak/display method used
elsewhere so behavior remains consistent.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: f28c2501-5f25-426e-af64-b57c5e5fa00d
📒 Files selected for processing (3)
accessibility/accessibility.jsflock.jsindex.html
✅ Files skipped from review due to trivial changes (1)
- index.html
🚧 Files skipped from review as they are similar to previous changes (1)
- flock.js
Comment on lines
999
to
1001
| pointerObserverScene.onPointerObservable.remove(pointerObserverRef); | ||
| } catch (error) { | ||
| console.warn("Suppressed non-critical error:", error); | ||
| } | ||
| } catch {} | ||
| pointerObserverRef = null; |
There was a problem hiding this comment.
Replace the empty catch block with an explicit no-op rationale.
Line 1000 has an empty catch block, which triggers ESLint and hides intent.
Suggested fix
if (pointerObserverScene && pointerObserverRef) {
try {
pointerObserverScene.onPointerObservable.remove(pointerObserverRef);
- } catch {}
+ } catch (_error) {
+ // Observer can already be disposed during scene transitions.
+ }
pointerObserverRef = null;
pointerObserverScene = null;
}📝 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
| pointerObserverScene.onPointerObservable.remove(pointerObserverRef); | |
| } catch (error) { | |
| console.warn("Suppressed non-critical error:", error); | |
| } | |
| } catch {} | |
| pointerObserverRef = null; | |
| pointerObserverScene.onPointerObservable.remove(pointerObserverRef); | |
| } catch (_error) { | |
| // Observer can already be disposed during scene transitions. | |
| } | |
| pointerObserverRef = null; |
🧰 Tools
🪛 GitHub Check: eslint
[failure] 1000-1000:
Empty block statement
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@accessibility/accessibility.js` around lines 999 - 1001, The empty catch in
the block that calls
pointerObserverScene.onPointerObservable.remove(pointerObserverRef) should be
replaced with an explicit no-op that documents intent; update the catch to
accept the error (e) and include a short comment or noop rationale (e.g.,
"ignore errors if observer already removed or scene disposed") so ESLint no
longer flags an empty block, keeping pointerObserverRef = null afterward; locate
this in the code paths around pointerObserverScene, pointerObserverRef and
onPointerObservable.remove.
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
1 participant
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.
I used Chat GPT for a comparison between the previous commit that had the working documents and the local version to diagnose the issue.
Summary by CodeRabbit