Skip to content

feat: [1097] Display設定のプレビュー機能を追加、判定キャラクタ位置の変更に対応 他#2121

Merged
cwtickle merged 10 commits into
developfrom
feature/1097-display
May 20, 2026
Merged

feat: [1097] Display設定のプレビュー機能を追加、判定キャラクタ位置の変更に対応 他#2121
cwtickle merged 10 commits into
developfrom
feature/1097-display

Conversation

@cwtickle
Copy link
Copy Markdown
Owner

@cwtickle cwtickle commented May 19, 2026

🔨 変更内容 / Details of Changes

1. Display設定のプレビュー機能を追加

  • Display設定の「StepZone」「Judgment」「LifeGauge」「Score」「MusicInfo」について、
    実際の画面での表示をプレビューできる機能を実装しました。
  • Display画面上部の「Preview」ボタンで表示できます。ショートカットは「P」です。
  • このうち、判定キャラクタ位置(矢印用、フリーズアロー用)についてはドラッグで好きな位置に移動でき、
    その内容がプレイ画面に反映されるようになります。リロードするまで有効です。
  • 内部的にページを分離しています。プレビュー中はページ移動および閉じるボタンのみ有効です。

2. Display設定の一部機能(Judgment/FastSlow, Background/Lyrics)を統合

  • Display設定のうち、FastSlowを「Judgment」に、Lyricsを「Background」に統合し、
    FastSlowとLyricsは拡張設定として機能する仕様に変更しました。
  • 「FastSlow」を選択するとJudgmentが、「Lyrics」を選択するとBackgroundが非表示になります。
  • Fast/Slowだけ、Lyricsだけを消す機能は存在しません。(用途が考えにくいため)

3. Display設定のボタンの高さや位置を変更

  • 2.の変更に伴い、Display設定のボタンの高さ・位置とAppearance以下の設定の位置を変更しました。

4. Opacity設定について、Hidden+/Sudden+のフィルターバーを対象から除外するよう変更

  • Hidden+/Sudden+のフィルターバーは連動しない方が良いという要望があり、対応しました。

5. 設定オブジェクト周りのコード整理

  • 省略できる部分や共通部分があったため、その部分のコードを整理しました。

🔖 関連Issue, 変更理由 / Related Issues, Reason for Changes

  1. Display設定でどこが変わるかを把握しやすくするため。
  2. 類似の機能をまとめることで、今後機能拡張を行った場合に使用できるスペースを増やすため。
  3. 2.と同じ理由です。
  4. Discordでの要望より。
  5. 類似のコードの整理により定義を簡単にするため。

📷 スクリーンショット / Screenshot

imageimage

imageimage

imageimage

📝 その他コメント / Other Comments

  • Display設定のプレビューはイメージを伝える目的のため、
    ステップゾーンや曲名といった情報は固定文字で、実際の設定とは一致しません。

@cwtickle cwtickle added func10:Settings 設定画面全般 code-refactoring Discord / Gitter Discord (Gitter)での議論あり labels May 19, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 2026

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

Adds a Display Preview overlay with draggable judgment-position editing, restructures display-setting categories and layout metadata, remaps display shortcuts and KeyConfig preview wiring, and updates rendering to consume the new display flags and judgment offsets.

Changes

Display Preview Feature & Display Settings Refactor

Layer / File(s) Summary
Constants, layout entries, CSS, and i18n
js/lib/danoni_constants.js
Adds preview/layout entries (lblDisplayPreview, btnDisplayPreview, lblDisplayPreviewMsg, previewCredit, btnKbPreview), simplifies g_settingPos, adjusts g_windowObj.displaySprite.y, adds settings_Display CSS mapping, and adds Ja/En i18n keys for preview and coordinate updates.
Display settings model and shortcut remap
js/lib/danoni_constants.js
Removes d_fastslow/d_lyrics defaults, introduces d_judgments/d_velocitys/d_backgrounds, adds arrowJdgX/frzJdgX, updates g_displays/displayNum, remaps KeyP and digit/numpad shortcuts to new display-category targets, and adds displayPreview shortcuts.
Preview overlay lifecycle and UI
js/danoni_main.js
Adds preview overlay root/state and listener tracking, open/close lifecycle, buildPreviewUI driven by Display flags, draggable judgment groups and commit logic applying offsets to g_diffObj, and ensures clearWindow() closes the overlay.
Display button geometry, sprite handling, and grid distribution
js/danoni_main.js, js/lib/danoni_constants.js
Adjusts setSpriteList() placement/size fallbacks, switches Display option builders to explicit x/y/w/h geometry, updates secondary toggle/context wiring, and reduces option grid columns from 7 to 6.
KeyConfig preview constants and toggle
js/danoni_main.js, js/lib/danoni_constants.js
Updates KeyConfig preview element IDs (kbPreviewArea, kbCanvasBase, kbCanvasMap), recreates btnKbPreview with dynamic label, and wires preview toggle to togglePreview().
Gameplay rendering & judgment-position application
js/danoni_main.js
Decouples g_workObj.judgmentDisp and fastslowDisp, simplifies g_headerObj.opacityUse, forces filterView/filterBar* opacity when Appearance ranges active, gates word-data generation on d_background === OFF, and conditionally applies judgment position diffs with a special-case for Background OFF + reset; updates Display summary composition.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related PRs

Suggested labels

func:Settings, func:Main

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title directly relates to the main changes: adding a Display settings preview feature and supporting judgment character position changes, with additional improvements mentioned.
Description check ✅ Passed The description provides detailed information about all major changes including the preview feature implementation, feature consolidation (FastSlow/Lyrics), UI adjustments, opacity settings, and code refactoring.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/1097-display

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 6

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
js/lib/danoni_constants.js (1)

2868-2936: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Fix duplicate/incorrect key names in Shift+Digit shortcuts before proceeding with consistency verification.

The ShiftRight_Digit mappings contain errors that prevent proper consistency verification:

  • Line 2871: ShiftRight_Digit5 should be ShiftRight_Digit4 (maps to lnkscoreR)
  • Line 2873: ShiftRight_Digit6 should be ShiftRight_Digit5 (maps to lnkmusicInfoR)
  • Line 2875: Duplicate ShiftRight_Digit6 should be ShiftRight_Digit6 (maps to lnkfilterLineR)

The Digit and Numpad sections are correctly mapped and mirror each other, but Shift+Digit requires these corrections first.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/lib/danoni_constants.js` around lines 2868 - 2936, The Shift+Digit
shortcut keys in the mapping object have incorrect/duplicate names: rename the
key currently labeled ShiftRight_Digit5 to ShiftRight_Digit4 (so it pairs with
Digit4 -> lnkscoreR), rename the key currently labeled ShiftRight_Digit6 (that
maps to lnkmusicInfoR) to ShiftRight_Digit5, and ensure the subsequent
ShiftRight_Digit6 entry remains the lnkfilterLineR mapping (so the sequence
aligns: ShiftRight_Digit4 -> lnkscoreR, ShiftRight_Digit5 -> lnkmusicInfoR,
ShiftRight_Digit6 -> lnkfilterLineR); update those key names in the mapping
object around the ShiftRight_Digit* entries to match their ShiftLeft_*
counterparts (refer to ShiftLeft_Digit4, ShiftLeft_Digit5, ShiftLeft_Digit6, and
the corresponding lnk* IDs).
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@js/danoni_main.js`:
- Around line 9208-9233: The preview reverse StepZone is currently positioned
using stepY + revStepY which follows a possibly overridden stepY; change it to
use the gameplay origin by computing the reverse preview Y from C_STEP_Y +
g_posObj.reverseStepY (or use C_STEP_Y + revStepY) when creating the
previewStepR sprites so the reverse preview matches in-game placement; update
the y value passed to createEmptySprite for `previewStepR${j}` accordingly
(reference variables: stepY, revStepY, C_STEP_Y, g_posObj.reverseStepY,
previewStepR).
- Around line 9174-9179: closeDisplayPreview currently removes the overlay DOM
but never unregisters pointer listeners added via g_handler.addListener, leaving
stale handlers and DOM refs; locate where listeners are registered (calls to
g_handler.addListener that relate to the display preview) and ensure their
returned listener IDs are stored (e.g., on g_previewRoot or a module-level
variable), then in closeDisplayPreview call g_handler.removeListener(...) for
each stored ID before removing the overlay and clearing g_previewRoot; apply the
same fix pattern to the duplicate preview teardown code around the other preview
block (the code similar to lines 9402-9436).
- Around line 9151-9158: The drag handler must account for the previewFrame CSS
transform scale when converting pointer movement into left/top offsets: scale
the raw pointer deltas by the inverse of the applied transform (e.g., 1 / rate)
before applying them to the element positions and before writing to g_diffObj so
stored offsets are in unscaled screen coordinates; update the handler that reads
pointer deltas (the same logic referenced around previewFrame creation and the
drag logic that mutates left/top and g_diffObj) to divide deltas by the preview
scale (rate) and use those adjusted values for both visual movement and the
values saved into g_diffObj.
- Around line 14307-14314: The current guard around applying judgment offsets
uses only g_diffObj.arrowJdgX to decide skipping both X and Y adjustments, which
hides valid Y-only or freeze-only changes; update the condition so it only
suppresses changes when all relevant diffs are zero (e.g., arrowJdgX, frzJdgX,
arrowJdgY, frzJdgY) or alternatively apply each axis/variant independently only
when its diff is non-zero; modify the branch around jdgX/jdgY updates
(references: g_stateObj.d_background, C_FLG_OFF, g_headerObj.jdgPosReset,
g_diffObj.arrowJdgX, g_diffObj.frzJdgX, g_diffObj.arrowJdgY, g_diffObj.frzJdgY,
jdgX, jdgY) so Y adjustments and freeze adjustments are not skipped just because
arrowJdgX === 0.

In `@js/lib/danoni_constants.js`:
- Around line 2872-2879: The object key for the second shift-4 shortcut is
incorrect: replace the mistaken ShiftRight_Digit5 entry with ShiftRight_Digit4
to avoid overwriting ShiftLeft_Digit5; update the key name so that
ShiftRight_Digit4 pairs with id `lnkscoreR` (parallel to ShiftLeft_Digit4 ->
`lnkscoreR`) and preserves ShiftLeft_Digit5 / ShiftRight_Digit5 ->
`lnkmusicInfoR` mappings.
- Around line 2295-2296: Add JSDoc describing the new judgment offset fields
`arrowJdgX` and `frzJdgX` near the `g_diffObj` definition: state each field's
purpose (horizontal pixel offset applied to arrow and freeze judgment visuals
for the draggable judgment positioning feature), units (pixels), data type
(integer, can be negative for left offset or positive for right), and valid
range/constraints (e.g., reasonable clamping limits or "use integers within UI
bounds; default 0"). Place the comment block directly above `g_diffObj` or
inline above the two fields and mention that defaults are 0 and that these
offsets are applied relative to the judgment origin.

---

Outside diff comments:
In `@js/lib/danoni_constants.js`:
- Around line 2868-2936: The Shift+Digit shortcut keys in the mapping object
have incorrect/duplicate names: rename the key currently labeled
ShiftRight_Digit5 to ShiftRight_Digit4 (so it pairs with Digit4 -> lnkscoreR),
rename the key currently labeled ShiftRight_Digit6 (that maps to lnkmusicInfoR)
to ShiftRight_Digit5, and ensure the subsequent ShiftRight_Digit6 entry remains
the lnkfilterLineR mapping (so the sequence aligns: ShiftRight_Digit4 ->
lnkscoreR, ShiftRight_Digit5 -> lnkmusicInfoR, ShiftRight_Digit6 ->
lnkfilterLineR); update those key names in the mapping object around the
ShiftRight_Digit* entries to match their ShiftLeft_* counterparts (refer to
ShiftLeft_Digit4, ShiftLeft_Digit5, ShiftLeft_Digit6, and the corresponding lnk*
IDs).
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 680f637a-3471-40d7-9c35-e92de24e4f75

📥 Commits

Reviewing files that changed from the base of the PR and between 486491b and 717b362.

📒 Files selected for processing (2)
  • js/danoni_main.js
  • js/lib/danoni_constants.js

Comment thread js/danoni_main.js
Comment thread js/danoni_main.js
Comment thread js/danoni_main.js
Comment thread js/danoni_main.js
Comment thread js/lib/danoni_constants.js
Comment thread js/lib/danoni_constants.js
@cwtickle cwtickle added the Layout Changes ラベルやボタンなどのレイアウト変更を伴う修正 label May 19, 2026
@cwtickle cwtickle added the func20:Main プレイ画面全般 label May 19, 2026
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
js/danoni_main.js (1)

9409-9452: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Tear down preview listeners on page transitions too.

These handlers are only released from closeDisplayPreview(). Back/KeyConfig/PLAY paths call clearWindow() directly, so the overlay can disappear without clearing g_previewLsnrKeys, leaving g_handler holding detached nodes until some later manual close.

Suggested fix
 const clearWindow = (_redrawFlg = false, _customDisplayName = ``) => {
+	closeDisplayPreview();
 	resetKeyControl();
 	resetTransform();
 	resetXY();
 const closeDisplayPreview = () => {
 	const overlay = document.getElementById(`displayPreviewOverlay`);
-	if (!overlay) return;
-	deleteChildspriteAll(`displayPreviewOverlay`);
+	if (overlay) {
+		deleteChildspriteAll(`displayPreviewOverlay`);
+		overlay.remove();
+	}
 	// プレビュー専用に登録した残りのハンドラを明示解除
 	if (g_previewLsnrKeys?.size) {
 		g_previewLsnrKeys.forEach(k => g_handler.removeListener(k));
 		g_previewLsnrKeys.clear();
 	}
-	overlay.remove();
 	g_previewRoot = null;
 };
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/danoni_main.js` around lines 9409 - 9452, The preview pointer listeners
added via addPreviewListener (and stored via the lsnrkey/lsnrkeyTS/lsnrkeyTE
attributes) are only torn down by closeDisplayPreview(), but clearWindow() and
navigation paths (Back/KeyConfig/PLAY) can remove the overlay without calling
that, leaving entries in g_previewLsnrKeys and dangling references in g_handler;
update the teardown so page transitions also remove these listeners by either
calling closeDisplayPreview() from clearWindow() or adding logic in
clearWindow() to iterate g_previewLsnrKeys (and any g_handler references),
remove the associated pointer listeners from the stored elements, clear those
registry entries, and release pointer captures, ensuring to reference
addPreviewListener, closeDisplayPreview, clearWindow, g_previewLsnrKeys and
g_handler so the cleanup runs on all transition paths.
♻️ Duplicate comments (1)
js/danoni_main.js (1)

9409-9426: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Convert drag deltas back into playfield coordinates.

These deltas are taken in viewport pixels, but the preview frame is rendered at scale(0.8). The dragged judgment group will lag behind the pointer, and the saved arrowJdg* / frzJdg* offsets won't match the position the user actually dropped.

Suggested fix
  const keyDown = addPreviewListener(group, `pointerdown`, _evt => {
 		dragging = true;
 		dragStartX = _evt.clientX;
 		dragStartY = _evt.clientY;
 		elemStartX = parseInt(group.style.left, 10);
 		elemStartY = parseInt(group.style.top, 10);
 		group.style.cursor = `grabbing`;
 		group.setPointerCapture(_evt.pointerId);
 		_evt.stopPropagation();
 	});
  const keyMove = addPreviewListener(group, `pointermove`, _evt => {
 		if (!dragging) return;
-		const dx = _evt.clientX - dragStartX;
-		const dy = _evt.clientY - dragStartY;
+		const scale = _parent.getBoundingClientRect().width / _playW || 1;
+		const dx = (_evt.clientX - dragStartX) / scale;
+		const dy = (_evt.clientY - dragStartY) / scale;
 		const newX = Math.max(0, Math.min(_playW - 370, elemStartX + dx));
 		const newY = Math.max(0, Math.min(_playH - 50, elemStartY + dy));
 		group.style.left = wUnit(newX);
 		group.style.top = wUnit(newY);
 		_evt.stopPropagation();
 	});
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/danoni_main.js` around lines 9409 - 9426, The drag deltas (dx/dy) are in
viewport pixels while the preview is rendered at scale(0.8), so in the keyMove
handler convert those deltas back into playfield coordinates before applying
them: compute scaledDx = dx / PREVIEW_SCALE and scaledDy = dy / PREVIEW_SCALE
(use the same scale used to render the preview, e.g., 0.8) and use elemStartX +
scaledDx and elemStartY + scaledDy when computing newX/newY and clamping against
_playW/_playH; update the references in the pointermove listener (variables:
keyMove, dragStartX/dragStartY, elemStartX/elemStartY, newX/newY, wUnit) so the
saved arrowJdg*/frzJdg* offsets match the visual drop position.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@js/danoni_main.js`:
- Around line 9409-9452: The preview pointer listeners added via
addPreviewListener (and stored via the lsnrkey/lsnrkeyTS/lsnrkeyTE attributes)
are only torn down by closeDisplayPreview(), but clearWindow() and navigation
paths (Back/KeyConfig/PLAY) can remove the overlay without calling that, leaving
entries in g_previewLsnrKeys and dangling references in g_handler; update the
teardown so page transitions also remove these listeners by either calling
closeDisplayPreview() from clearWindow() or adding logic in clearWindow() to
iterate g_previewLsnrKeys (and any g_handler references), remove the associated
pointer listeners from the stored elements, clear those registry entries, and
release pointer captures, ensuring to reference addPreviewListener,
closeDisplayPreview, clearWindow, g_previewLsnrKeys and g_handler so the cleanup
runs on all transition paths.

---

Duplicate comments:
In `@js/danoni_main.js`:
- Around line 9409-9426: The drag deltas (dx/dy) are in viewport pixels while
the preview is rendered at scale(0.8), so in the keyMove handler convert those
deltas back into playfield coordinates before applying them: compute scaledDx =
dx / PREVIEW_SCALE and scaledDy = dy / PREVIEW_SCALE (use the same scale used to
render the preview, e.g., 0.8) and use elemStartX + scaledDx and elemStartY +
scaledDy when computing newX/newY and clamping against _playW/_playH; update the
references in the pointermove listener (variables: keyMove,
dragStartX/dragStartY, elemStartX/elemStartY, newX/newY, wUnit) so the saved
arrowJdg*/frzJdg* offsets match the visual drop position.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6b0d23db-6807-4184-b6cf-4d493596ae7a

📥 Commits

Reviewing files that changed from the base of the PR and between 5395ddf and ca4ba22.

📒 Files selected for processing (1)
  • js/danoni_main.js

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@js/danoni_main.js`:
- Around line 9188-9197: closeDisplayPreview currently calls
g_handler.removeListener for each key in g_previewLsnrKeys but removeListener
does not delete the underlying events entries, leaving stale references; update
closeDisplayPreview so after calling g_handler.removeListener(k) you also remove
the corresponding registry entry (delete g_handler.events[k] or equivalent) for
each key in g_previewLsnrKeys, then clear g_previewLsnrKeys, ensuring both the
listener is removed and the events[key] entry is purged to avoid retained
references.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 8ed7ce33-37df-4127-8d0f-5ffbef87da6d

📥 Commits

Reviewing files that changed from the base of the PR and between ca4ba22 and ef2f4c0.

📒 Files selected for processing (2)
  • js/danoni_main.js
  • js/lib/danoni_constants.js

Comment thread js/danoni_main.js
@cwtickle
Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)
js/danoni_main.js (3)

9266-9274: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Seed the preview from the same judgment baseline as gameplay.

These initial coordinates do not match mainInit()'s source of truth: runtime judgment placement is derived from the centered base plus g_diffObj, but the preview starts from g_lblPosObj/base-only values. On the first open, existing arrowJdgY / frzJdgY adjustments are not reflected, so the preview can already be out of sync before the user drags anything.

Suggested fix
-		const jX0 = g_lblPosObj?.jdgJ?.x ?? Math.round(_playW / 2 - 220);
-		const jY0 = g_lblPosObj?.jdgJ?.y ?? Math.round((_playH + (g_posObj?.stepYR ?? 0)) / 2 - 60);
-		const fX0 = g_lblPosObj?.jdgFJ?.x ?? Math.round(_playW / 2 - 120);
-		const fY0 = g_lblPosObj?.jdgFJ?.y ?? Math.round((_playH + (g_posObj?.stepYR ?? 0)) / 2 + 10);
+		const jX0 = Math.round(_playW / 2 - 220) + (g_diffObj.arrowJdgX ?? 0);
+		const jY0 = Math.round((_playH + (g_posObj?.stepYR ?? 0)) / 2 - 60) + (g_diffObj.arrowJdgY ?? 0);
+		const fX0 = Math.round(_playW / 2 - 120) + (g_diffObj.frzJdgX ?? 0);
+		const fY0 = Math.round((_playH + (g_posObj?.stepYR ?? 0)) / 2 + 10) + (g_diffObj.frzJdgY ?? 0);
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/danoni_main.js` around lines 9266 - 9274, The preview initial positions
are computed from g_lblPosObj/base-only (jX0/jY0/fX0/fY0) instead of using the
same centered baseline plus runtime judgment offsets used by mainInit(); update
jdgInitX/jdgInitY/jdgFInitX/jdgFInitY to seed from the same source as mainInit()
by applying the centered base positions plus g_diffObj (and the runtime
arrowJdgY/frzJdgY offsets) rather than g_lblPosObj alone—i.e., compute the
centered baseline (as mainInit does), add g_diffObj or arrowJdgY/frzJdgY where
appropriate, and use those values for jdgInitX/jdgInitY/jdgFInitX/jdgFInitY so
the preview matches gameplay on first open.

14328-14335: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't drop vertical-only judgment moves when jdgPosReset is enabled.

This reset guard only checks arrowJdgX === 0. If the user drags only on Y, arrowJdgY / frzJdgY can be non-zero and still get ignored whenever Background is OFF and jdgPosReset is enabled, which breaks the new preview workflow.

Suggested fix
-	if (g_stateObj.d_background === C_FLG_OFF && g_headerObj.jdgPosReset && g_diffObj.arrowJdgX === 0) {
-		// jdgPosResetオプションにより判定位置変更が阻害されることを防ぐため、
-		// X座標が変わった場合は判定位置変更が行われたと判断
-	} else {
+	const shouldResetJdgPos =
+		g_stateObj.d_background === C_FLG_OFF &&
+		g_headerObj.jdgPosReset &&
+		g_diffObj.arrowJdgX === 0 &&
+		g_diffObj.arrowJdgY === 0 &&
+		g_diffObj.frzJdgX === 0 &&
+		g_diffObj.frzJdgY === 0;
+	if (!shouldResetJdgPos) {
 		jdgX[0] += g_diffObj.arrowJdgX;
 		jdgX[1] += g_diffObj.frzJdgX;
 		jdgY[0] += g_diffObj.arrowJdgY;
 		jdgY[1] += g_diffObj.frzJdgY;
 	}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/danoni_main.js` around lines 14328 - 14335, The current guard skips
applying judgment deltas whenever jdgPosReset is enabled and arrowJdgX === 0,
which also drops vertical-only moves; change the condition so it only skips when
all judgment deltas are zero. In the block that checks g_stateObj.d_background,
g_headerObj.jdgPosReset and g_diffObj.arrowJdgX, update the predicate to require
arrowJdgX, arrowJdgY, frzJdgX and frzJdgY all be zero (e.g., check
g_diffObj.arrowJdgX === 0 && g_diffObj.arrowJdgY === 0 && g_diffObj.frzJdgX ===
0 && g_diffObj.frzJdgY === 0) before skipping; otherwise execute the existing
jdgX/jdgY adjustments (jdgX[0] += g_diffObj.arrowJdgX, jdgX[1] +=
g_diffObj.frzJdgX, jdgY[0] += g_diffObj.arrowJdgY, jdgY[1] +=
g_diffObj.frzJdgY).

9165-9178: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Divide drag deltas by the previewFrame's scale factor to fix cursor lag.

previewFrame is rendered at scale(0.8), but the drag handler computes pointer deltas from viewport coordinates without accounting for this CSS transform. As a result, when the user drags the judgment group, it moves only ~80% as far as the visual cursor movement, causing noticeable lag between cursor and element.

To fix this, add a _scale parameter to buildDraggableJudgGroup and divide the dx/dy deltas by that factor:

Suggested fix
-const buildDraggableJudgGroup = (_parent, _groupId, _initX, _initY, _playW, _playH, _opts) => {
+const buildDraggableJudgGroup = (_parent, _groupId, _initX, _initY, _playW, _playH, _opts, _scale = 1) => {

At both call sites (lines 9277 and 9285), pass the rate constant:

-		buildDraggableJudgGroup(_frame, `jdgJ`, jdgInitX, jdgInitY, _playW, _playH, {
+		buildDraggableJudgGroup(_frame, `jdgJ`, jdgInitX, jdgInitY, _playW, _playH, {
 			charaText: d.judgment === C_FLG_ON ? g_lblNameObj.j_ii : ``,
 			comboText: d.judgment === C_FLG_ON ? `5 Combo!!` : ``,
 			diffText: `Fast 3 Frames`,
 			charaColor: `#66ffff`,
-		});
+		}, rate);

-		buildDraggableJudgGroup(_frame, `jdgFJ`, jdgFInitX, jdgFInitY, _playW, _playH, {
+		buildDraggableJudgGroup(_frame, `jdgFJ`, jdgFInitX, jdgFInitY, _playW, _playH, {
 			charaText: d.judgment === C_FLG_ON ? g_lblNameObj.j_kita : ``,
 			comboText: d.judgment === C_FLG_ON ? `5 Combo!!` : ``,
 			diffText: `Fast 2 Frames`,
 			charaColor: `#ffff99`,
-		});
+		}, rate);

Inside the drag handler (around line 9426):

-		const dx = _evt.clientX - dragStartX;
-		const dy = _evt.clientY - dragStartY;
+		const dx = (_evt.clientX - dragStartX) / _scale;
+		const dy = (_evt.clientY - dragStartY) / _scale;
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@js/danoni_main.js` around lines 9165 - 9178, The previewFrame is rendered
with transform: scale(rate) but the drag handler in buildDraggableJudgGroup uses
viewport pointer deltas directly, causing the moved element to lag; add a new
_scale parameter to buildDraggableJudgGroup, pass the local rate when calling
it, and in the drag handler divide computed dx and dy by _scale (i.e., use dx /=
_scale and dy /= _scale before applying translation) so pointer movement maps
correctly to the scaled element; update both call sites that construct the
draggable judgment group to supply rate.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@js/danoni_main.js`:
- Around line 9266-9274: The preview initial positions are computed from
g_lblPosObj/base-only (jX0/jY0/fX0/fY0) instead of using the same centered
baseline plus runtime judgment offsets used by mainInit(); update
jdgInitX/jdgInitY/jdgFInitX/jdgFInitY to seed from the same source as mainInit()
by applying the centered base positions plus g_diffObj (and the runtime
arrowJdgY/frzJdgY offsets) rather than g_lblPosObj alone—i.e., compute the
centered baseline (as mainInit does), add g_diffObj or arrowJdgY/frzJdgY where
appropriate, and use those values for jdgInitX/jdgInitY/jdgFInitX/jdgFInitY so
the preview matches gameplay on first open.
- Around line 14328-14335: The current guard skips applying judgment deltas
whenever jdgPosReset is enabled and arrowJdgX === 0, which also drops
vertical-only moves; change the condition so it only skips when all judgment
deltas are zero. In the block that checks g_stateObj.d_background,
g_headerObj.jdgPosReset and g_diffObj.arrowJdgX, update the predicate to require
arrowJdgX, arrowJdgY, frzJdgX and frzJdgY all be zero (e.g., check
g_diffObj.arrowJdgX === 0 && g_diffObj.arrowJdgY === 0 && g_diffObj.frzJdgX ===
0 && g_diffObj.frzJdgY === 0) before skipping; otherwise execute the existing
jdgX/jdgY adjustments (jdgX[0] += g_diffObj.arrowJdgX, jdgX[1] +=
g_diffObj.frzJdgX, jdgY[0] += g_diffObj.arrowJdgY, jdgY[1] +=
g_diffObj.frzJdgY).
- Around line 9165-9178: The previewFrame is rendered with transform:
scale(rate) but the drag handler in buildDraggableJudgGroup uses viewport
pointer deltas directly, causing the moved element to lag; add a new _scale
parameter to buildDraggableJudgGroup, pass the local rate when calling it, and
in the drag handler divide computed dx and dy by _scale (i.e., use dx /= _scale
and dy /= _scale before applying translation) so pointer movement maps correctly
to the scaled element; update both call sites that construct the draggable
judgment group to supply rate.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3303d91d-fe2b-4601-b15e-2df3c3c05e54

📥 Commits

Reviewing files that changed from the base of the PR and between ef2f4c0 and 314e722.

📒 Files selected for processing (1)
  • js/danoni_main.js

@cwtickle
Copy link
Copy Markdown
Owner Author

@coderabbitai
Outside diff range commentsについて、コメントします。

Seed the preview from the same judgment baseline as gameplay.
⇒この件についてはその通りなので、修正します。

Don't drop vertical-only judgment moves when jdgPosReset is enabled.
⇒この件は下記で議論済みです。理由があってこの仕様にしています。
#2121 (comment)

Divide drag deltas by the previewFrame's scale factor to fix cursor lag.
⇒この件は下記で議論済みです。修正対象外です。カーソル速度を変える必要もありません。
#2121 (comment)

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 20, 2026

Tip

For best results, initiate chat on the files or code changes.

The PR description images show the Display Preview feature in action — the first image displays the judgment coordinate update toast ("矢印判定の座標を更新: dX=187.5, dY=90") with draggable judgment positions. The subsequent images show the full preview layout, the updated DISPLAY configuration menu (with Background replacing Judgment/FastSlow split, and layout adjustments), disabled-element placeholders with red X icons, and the legacy menu with FastSlow/Lyrics highlighted in gold.


@cwtickle、各点についてご説明ありがとうございます。

  1. Seed the preview from the same judgment baseline as gameplay — 修正いただけるとのこと、ありがとうございます。

  2. Don't drop vertical-only judgment moves when jdgPosReset is enabled — 設計上の意図があるとのこと、承知しました。当該コメントは取り下げます。

  3. Divide drag deltas by the previewFrame's scale factor to fix cursor lag — カーソル速度の変更は不要という設計判断、承知しました。当該コメントも取り下げます。

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code-refactoring Discord / Gitter Discord (Gitter)での議論あり func10:Settings 設定画面全般 func20:Main プレイ画面全般 Layout Changes ラベルやボタンなどのレイアウト変更を伴う修正

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant