Refactor camera effects into extensible effect system#1400
Merged
Conversation
Replace hardcoded camera shake/fade state with an extensible CameraEffect system. Effects are managed via addCameraEffect/getCameraEffect/removeCameraEffect. Camera effects: - CameraEffect base class with update/draw/destroy lifecycle and isPersistent flag - ShakeEffect: extracted shake logic with intensity, duration, axis, onComplete - FadeEffect: unified fade-in/out with direction "in"/"out", supports non-opaque alpha - MaskEffect: shape-based mask transitions using Ellipse or Polygon with pooled shape (zero per-frame allocations), inverted clip masking Camera2d: - shake()/fadeIn()/fadeOut() are now backward-compatible wrappers - cameraEffects array with auto-removal of completed effects - isPersistent effects survive camera.reset() (for state transitions) Trigger: - Supports "fade" (default) and "mask" transition types via settings.transition - settings.color replaces legacy settings.fade (backward compatible) - Both hide and reveal effects handled internally via settings.onLoaded - triggerEvent() uses FadeEffect/MaskEffect based on transition type State manager: - state.transition() accepts "fade" or "mask" with optional shape parameter - state.change() uses switch/case for transition types with pre-created reveal - _onSwitchComplete cleared after execution to prevent stale callbacks - onComplete uses arrow function for correct this binding Other fixes: - Canvas: setMask(shape, true) uses evenodd clipping for proper inverted masks - Ellipse: clone() now uses ellipsePool instead of new Ellipse() - TMXObjectFactory: override warning uses console.warn() not deprecation warning() Examples: - Platformer: camera shake on player hit, LevelTrigger with star mask transition Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors melonJS camera shake/fade internals into an extensible CameraEffect system and updates state/trigger transitions to use effect instances (including a new mask-based transition), while keeping the existing shake(), fadeIn(), and fadeOut() APIs.
Changes:
- Add
CameraEffectbase class plus built-inShakeEffect,FadeEffect, andMaskEffect, and integrate an effect list intoCamera2d. - Update
state.change()andTriggerto drive hide→switch→reveal transitions via camera effects (fade or mask). - Fix Canvas inverted masking via
clip("evenodd"), poolEllipse.clone(), and update examples/tests/changelog.
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/melonjs/src/camera/effects/camera_effect.ts | Introduces base lifecycle/type for camera effects. |
| packages/melonjs/src/camera/effects/shake_effect.ts | Extracts shake behavior into an effect. |
| packages/melonjs/src/camera/effects/fade_effect.ts | Unifies fade-in/out into a single effect. |
| packages/melonjs/src/camera/effects/mask_effect.ts | Adds shape-based mask transition effect. |
| packages/melonjs/src/camera/camera2d.ts | Adds effect list + add/get/remove; refactors shake/fade wrappers and FX drawing. |
| packages/melonjs/src/state/state.ts | Adds "fade"/"mask" transitions driven by effects during state changes. |
| packages/melonjs/src/renderable/trigger.js | Updates level Trigger to use FadeEffect/MaskEffect and adds new transition settings. |
| packages/melonjs/src/video/canvas/canvas_renderer.js | Fixes inverted mask behavior using even-odd clip rule. |
| packages/melonjs/src/geometries/ellipse.ts | Makes Ellipse.clone() use ellipsePool. |
| packages/melonjs/src/level/tiled/TMXObjectFactory.js | Replaces deprecated warning helper with console.warn. |
| packages/melonjs/src/index.ts | Re-exports new camera effect classes. |
| packages/melonjs/tests/camera.spec.js | Adds tests for effect list behavior and built-in effects. |
| packages/melonjs/tests/trigger.spec.js | Adds tests for Trigger transition settings and effect creation. |
| packages/melonjs/tests/ellipse.spec.ts | Adds regression test for pooled ellipse cloning. |
| packages/melonjs/CHANGELOG.md | Documents new effect system and related fixes. |
| packages/examples/src/examples/platformer/entities/player.ts | Demonstrates camera shake on hit. |
| packages/examples/src/examples/platformer/entities/leveltrigger.ts | Adds a star-mask Trigger subclass example. |
| packages/examples/src/examples/platformer/createGame.ts | Registers custom Trigger + adjusts renderer const usage. |
| packages/examples/src/examples/platformer/assets/map/map1.tmx | Adds a Trigger instance to showcase transition behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- MaskEffect: keep drawing on hide completion (prevents flash before state switch) - state.ts: fix eslint-disable-line → eslint-disable-next-line placement - state.ts: create reveal effects in _onSwitchComplete with post-switch viewport (fixes stale camera reference when viewport is reassigned during game.reset) - Tests: deterministic Math.random stub for shake offset test Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Trigger: onLoaded wrapper preserves levelId argument and this binding - Tests: Math.random stub wrapped in try/finally for safe restoration - Tests: renamed "reset should clear all effects" → "reset should clear non-persistent effects", added companion test for persistent effects Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
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
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
Closes #1369
Replaces the hardcoded camera shake/fade state (
_shake,_fadeIn,_fadeOut) with an extensibleCameraEffectsystem. Fully backward compatible —shake(),fadeIn(),fadeOut()keep the same signatures.New camera effects
CameraEffect— base class withupdate(dt),draw(renderer, w, h),destroy()lifecycle andisPersistentflag for effects that survive state changesShakeEffect— extracted shake logic withintensity,duration,axis,onCompleteFadeEffect— unified fade-in/out withdirection: "in" | "out", supports non-opaque target alphaMaskEffect— shape-based mask transitions using Ellipse or Polygon, with pooled shape rendering (zero per-frame allocations)Camera2d API
addCameraEffect(effect)/getCameraEffect(EffectClass)/removeCameraEffect(effect)shake(),fadeIn(),fadeOut()are now thin wrappers — same signatures, fully backward compatibleisPersistenteffects survivecamera.reset()(needed for state transitions)Trigger
transitionsetting:"fade"(default) or"mask"for shape-based level transitionscolorsetting replaces legacyfade(backward compatible)State manager
state.transition()accepts"fade"or"mask"with optional shape parameterstate.change()uses switch/case for transition typesOther fixes
setMask(shape, true)usesevenoddclipping for proper inverted mask supportclone()now usesellipsePoolinstead ofnew Ellipse()console.warn()instead of deprecationwarning()Examples
LevelTriggerwith star polygon mask transitionTest plan
🤖 Generated with Claude Code