Skip to content

fix: comment out debug console.logs and tighten TypeScript any types#5

Open
yassinsolim wants to merge 2 commits into
mainfrom
bot/audit-fixes
Open

fix: comment out debug console.logs and tighten TypeScript any types#5
yassinsolim wants to merge 2 commits into
mainfrom
bot/audit-fixes

Conversation

@yassinsolim

Copy link
Copy Markdown
Owner

Summary

Cleans up debug output and improves TypeScript type safety across the codebase.

Changes

  • src/Application/World/Hitboxes.ts: Comment out 4x debug console.log calls in mousedown handler
  • src/Application/Utils/Mouse.ts: event: anyevent: MouseEvent & { inComputer?: boolean }
  • src/Application/UI/EventBus.ts: dispatch data: anyunknown
  • src/Application/World/CoffeeSteam.ts: model: anyRecord<string, unknown>
  • src/Application/Camera/Camera.ts: easing?: anyeasing?: (k: number) => number
  • .gitignore: Add .env and .env.local entries

Validation

  • No logic changes — debug logs commented out, type narrowing applied where straightforward
  • EventEmitter.ts any types left untouched (complex generic infrastructure — needs deeper refactor)

Risks / Notes

  • Very low risk — type changes are compatible narrowings
  • console.logs were in production mousedown handler (not tests)

⚠️ DO NOT MERGE — festive wants to review this PR personally.

- Comment out 4x console.log debug calls in Hitboxes.ts (non-test file)
- Mouse.ts: event: any -> event: MouseEvent & { inComputer?: boolean }
- EventBus.ts: dispatch data: any -> unknown
- CoffeeSteam.ts: model: any -> Record<string, unknown>
- Camera.ts: easing?: any -> easing?: (k: number) => number
- .gitignore: add .env and .env.local entries

Code quality audit finding.
Copilot AI review requested due to automatic review settings February 28, 2026 04:37

Copilot AI left a comment

Copy link
Copy Markdown

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 cleans up production debug output and tightens several TypeScript types to improve type safety, while also ensuring local environment files aren’t committed.

Changes:

  • Commented out debug console.log calls in the Hitboxes mousedown handler.
  • Narrowed several any types to more specific typings (MouseEvent intersection, unknown, easing function signature).
  • Updated .gitignore to exclude .env and .env.local.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/Application/World/Hitboxes.ts Comments out debug logging in the mousedown raycasting code path.
src/Application/World/CoffeeSteam.ts Attempts to replace any for model with a safer type (currently introduces type breakage).
src/Application/Utils/Mouse.ts Narrows the mousemove payload type to include inComputer on top of MouseEvent.
src/Application/UI/EventBus.ts Changes dispatch payload type from any to unknown (also exposes an existing remove/unsubscribe bug).
src/Application/Camera/Camera.ts Narrows easing to a tween-compatible (k: number) => number function.
.gitignore Ignores .env and .env.local.
Comments suppressed due to low confidence (3)

src/Application/Utils/Mouse.ts:24

  • if (event.clientX && event.clientY) will skip updates when the cursor is at x=0 or y=0 (valid values), leaving stale coordinates. Check for != null/!== undefined instead of truthiness, or just assign from event.clientX/event.clientY unconditionally since this handler is only for mousemove events.
        this.on('mousemove', (event: MouseEvent & { inComputer?: boolean }) => {
            if (event.clientX && event.clientY) {
                this.x = event.clientX;
                this.y = event.clientY;
            }

src/Application/UI/EventBus.ts:10

  • on() registers a wrapper listener (e) => callback(e.detail), but remove() attempts to unregister callback directly. This won't remove the actual event listener, leading to leaks and handlers firing after expected cleanup. Consider returning the wrapper function from on() (so callers can pass it to removeEventListener), or store a Map from callback -> wrapper and remove the wrapper.
    dispatch(event: string, data: unknown) {
        document.dispatchEvent(new CustomEvent(event, { detail: data }));
    },
    remove(event: string, callback: (...args: any[]) => any) {
        document.removeEventListener(event, callback);

src/Application/World/Hitboxes.ts:44

  • These lines add repeated TODOs while leaving commented-out console.log statements inline. If the goal is to remove debug output, it's cleaner to delete the logs entirely or gate them behind an existing debug flag/logger so they don't accumulate as commented code.
            // TODO: replace with proper logger // console.log(this.camera.instance.position);

            // TODO: replace with proper logger // console.log(this.mouse);
            const ray = new THREE.Raycaster();
            ray.setFromCamera(
                { x: this.mouse.x, y: this.mouse.y },
                this.camera.instance
            );
            // TODO: replace with proper logger // console.log(ray);
            const intersects = ray.intersectObjects(this.scene.children);
            // TODO: replace with proper logger // console.log(intersects);

Comment thread src/Application/World/CoffeeSteam.ts Outdated
Comment on lines 12 to 13
model: Record<string, unknown>;
application: Application;

Copilot AI Feb 28, 2026

Copy link

Choose a reason for hiding this comment

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

Changing model from any to Record<string, unknown> makes all of the subsequent this.model.color/material/mesh property accesses type-unsafe under strict TS (they become unknown), and will cause type errors (e.g., new THREE.Color(this.model.color) and this.model.material.uniforms...). Define an explicit interface/type for the model shape (color/material/mesh) or use a typed object literal so these properties are correctly typed.

Copilot uses AI. Check for mistakes.
@sonarqubecloud

Copy link
Copy Markdown

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