Skip to content

サンプルの背景処理を修正 - #66

Merged
ienaga merged 1 commit into
mainfrom
develop
Jun 1, 2025
Merged

サンプルの背景処理を修正#66
ienaga merged 1 commit into
mainfrom
develop

Conversation

@ienaga

@ienaga ienaga commented Jun 1, 2025

Copy link
Copy Markdown
Member

No description provided.

@ienaga
ienaga requested a review from Copilot June 1, 2025 23:08

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 refines the background’s resize handling by always wiring up resize events in the constructor and consolidating conditional redraw/scale logic.

  • Adds an unconditional RESIZE event listener in the constructor to trigger draw and scale services.
  • Refactors the per-frame check to a single combined width/height comparison block.
  • Updates package versions in both the template’s package.json and the root package.json.

Reviewed Changes

Copilot reviewed 3 out of 4 changed files in this pull request and generated 2 comments.

File Description
template/src/model/domain/callback/Background.js Wire up resize listener in ctor; merge redraw/scale checks into one conditional
template/package.json Bump dev dependency versions (eslint, vite, vitest, etc.)
package.json Increment template version from 4.2.1 to 4.2.2
Files not reviewed (1)
  • template/package-lock.json: Language not supported
Comments suppressed due to low confidence (2)

template/src/model/domain/callback/Background.js:25

  • No tests appear to cover the new resize handling logic; consider adding unit tests or integration tests to verify that backgroundDrawService and backgroundChangeScaleService are invoked correctly when the stage is resized.
backgroundDrawService(this);

template/src/model/domain/callback/Background.js:23

  • Consider reintroducing a check like !stage.hasEventListener(Event.RESIZE) before adding the listener to prevent registering duplicate handlers and potential memory leaks.
stage.addEventListener(Event.RESIZE, () =>

Comment on lines +23 to +27
stage.addEventListener(Event.RESIZE, () =>
{
backgroundDrawService(this);
backgroundChangeScaleService(this);
});

Copilot AI Jun 1, 2025

Copy link

Choose a reason for hiding this comment

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

Add a null check or guard to ensure stage is defined before calling addEventListener, e.g. if (stage) { ... }, to avoid potential runtime errors.

Suggested change
stage.addEventListener(Event.RESIZE, () =>
{
backgroundDrawService(this);
backgroundChangeScaleService(this);
});
if (stage) {
stage.addEventListener(Event.RESIZE, () =>
{
backgroundDrawService(this);
backgroundChangeScaleService(this);
});
}

Copilot uses AI. Check for mistakes.
Comment on lines +25 to +26
backgroundDrawService(this);
backgroundChangeScaleService(this);

Copilot AI Jun 1, 2025

Copy link

Choose a reason for hiding this comment

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

Current order calls backgroundDrawService before backgroundChangeScaleService, which may draw at the wrong scale. Swap these calls so scaling happens before drawing.

Suggested change
backgroundDrawService(this);
backgroundChangeScaleService(this);
backgroundChangeScaleService(this);
backgroundDrawService(this);

Copilot uses AI. Check for mistakes.
@ienaga
ienaga merged commit 636ced8 into main Jun 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants