速度グラフ上にマウスオーバーで合計速度を自動計算する機能を追加 #1936
Merged
Conversation
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
Contributor
📝 WalkthroughSummary by CodeRabbitRelease Notes
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR refactors the speed graph rendering system in danoni_main.js by consolidating drawing logic into a draw() function, adding pointer interaction for graph navigation, and introducing a calculateTotalSpeed() function to compute and display total speed/boost metrics. A minor pointerEvents property is added to danoni_constants.js. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ast-grep (0.40.4)js/danoni_main.jsjs/lib/danoni_constants.jsThanks 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @js/danoni_main.js:
- Around line 6679-6731: The draw function is creating invalid canvas paths and
wrong averages: initialize drawing and averaging safely by starting each series
path at the first valid point (use context.moveTo for the first frame or set
preY = y for i===0 before any context.lineTo), compute deltaFrame as i===0 ? 0 :
frame[i] - frame[i-1] (do not subtract startFrame) so the first element doesn't
produce a negative delta, and guard divisors when normalizing averages (use
Math.max(playingFrame, 1) for avgX and keep using Math.max(avgSubFrame, 1) for
avgSubX) to avoid NaN; update references: draw, preY, frame[], deltaFrame, avgX,
avgSubX, avgSubFrame, playingFrame, startFrame, and the canvas path calls
(context.moveTo/context.lineTo) accordingly.
- Around line 6762-6793: calculateTotalSpeed currently uses fragile default
parameters that access DOM nodes (datatotal3, datatotal5) before the function
runs and assumes the score detail container (detailSpeed) exists, causing
runtime errors when score details are disabled; change calculateTotalSpeed to
accept explicit numeric args or no args, compute local speed and boost inside
the function using safe fallbacks like
Number(document.getElementById('datatotal3')?.textContent) ?? g_stateObj.speed
and Number(document.getElementById('datatotal5')?.textContent) ?? 1, and
early-return if the container/detailSpeed or required DOM root is missing; also
replace direct document.getElementById checks/update with guarded lookups
(existence checks for datatotal, datatotal3, datatotal5, datatotal7) before
creating or setting their textContent so the function is safe when the UI isn’t
mounted.
🧹 Nitpick comments (1)
js/danoni_main.js (1)
6732-6760: Harden pointer logic: avoid DOM-id globals, remove magic bounds, handle “not found” index, use radians.
graphSpeed.onmousemoverelies on theid="graphSpeed"element being available as a global (graphSpeed), which is brittle.- Hard-coded
286will desync ifg_limitObj.graphWidthchanges.findIndex(...)can return-1(e.g.,xat the end), leading toundefinedspeed and NaN drawing.context.arc(..., 0, 360)uses radians API; should useMath.PI * 2.Proposed fix
const movePointer = (offsetX) => { const x = ((offsetX - 30) * playingFrame) / (g_limitObj.graphWidth - 30); const speed = {speed: 0, boost: 0}; draw(); Object.keys(speedObj).forEach(speedType => { - const speedIndex = speedObj[speedType].frame.findIndex((frame, i) => frame <= x && speedObj[speedType].frame[i + 1] >= x); - speed[speedType] = speedObj[speedType].speed[speedIndex]; + const frames = speedObj[speedType].frame; + let speedIndex = frames.findIndex((frm, i) => frm <= x && x <= (frames[i + 1] ?? frm)); + if (speedIndex < 0) speedIndex = frames.length - 1; + speed[speedType] = speedObj[speedType].speed[Math.min(speedIndex, speedObj[speedType].speed.length - 1)]; const y = (Math.min(Math.max(speed[speedType], _min - 0.05), _max + 0.05) - 1) * _a + _b; context.beginPath(); context.fillStyle = g_graphColorObj[speedType]; - context.arc(offsetX, y, 3, 0, 360); + context.arc(offsetX, y, 3, 0, Math.PI * 2); context.closePath(); context.fill(); }); calculateTotalSpeed(speed.speed, speed.boost); }; - graphSpeed.onmousemove = (e => { - if (e.offsetX < 30 || e.offsetX > 286) { + canvas.onmousemove = (e => { + if (e.offsetX < 30 || e.offsetX > g_limitObj.graphWidth) { return; } movePointer(e.offsetX); });-->
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
js/danoni_main.jsjs/lib/danoni_constants.js
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: cwtickle
Repo: cwtickle/danoniplus PR: 0
File: :0-0
Timestamp: 2024-11-05T13:52:01.850Z
Learning: `js/danoni_main.js` の修正内容が `allArrowsPlayed` が定義前に使用されていた問題の修正のみの場合、要約にはその点を正確に反映させる。
📚 Learning: 2025-06-15T13:23:40.605Z
Learnt from: cwtickle
Repo: cwtickle/danoniplus PR: 1881
File: js/danoni_main.js:2576-2579
Timestamp: 2025-06-15T13:23:40.605Z
Learning: In js/danoni_main.js, `g_keyObj.minWidth` (500px) is used as the absolute minimum width and is set at the start of the widthList, while `g_keyObj.minWidthDefault` (600px) is used as a fallback only when a key's minimum width is undefined. These two constants have distinct roles and should not be unified.
Applied to files:
js/lib/danoni_constants.js
📚 Learning: 2026-01-12T12:23:00.668Z
Learnt from: cwtickle
Repo: cwtickle/danoniplus PR: 1934
File: js/danoni_main.js:8406-8408
Timestamp: 2026-01-12T12:23:00.668Z
Learning: In js/danoni_main.js (exSettingInit), g_msgObj.sideScrollMsg has been removed/undefined and must not be appended to lblPlayWindow.title; the entire line should be deleted rather than null-coalesced.
Applied to files:
js/lib/danoni_constants.js
📚 Learning: 2024-11-05T13:52:01.850Z
Learnt from: cwtickle
Repo: cwtickle/danoniplus PR: 0
File: :0-0
Timestamp: 2024-11-05T13:52:01.850Z
Learning: `js/danoni_main.js` の修正内容が `allArrowsPlayed` が定義前に使用されていた問題の修正のみの場合、要約にはその点を正確に反映させる。
Applied to files:
js/lib/danoni_constants.jsjs/danoni_main.js
📚 Learning: 2025-05-06T13:39:54.053Z
Learnt from: cwtickle
Repo: cwtickle/danoniplus PR: 1850
File: js/danoni_main.js:5355-5360
Timestamp: 2025-05-06T13:39:54.053Z
Learning: In `js/danoni_main.js`, the `changeMSelect` function is never called before its definition within the relevant execution flow, so its placement as a `const` arrow function is intentional and does not cause TDZ errors.
Applied to files:
js/lib/danoni_constants.js
📚 Learning: 2025-02-20T11:28:09.239Z
Learnt from: cwtickle
Repo: cwtickle/danoniplus PR: 1775
File: js/danoni_main.js:4834-4834
Timestamp: 2025-02-20T11:28:09.239Z
Learning: The `g_windowObj.dataSprite` property specifically handles size information for the Data Management layout and can be modified without affecting backward compatibility.
Applied to files:
js/lib/danoni_constants.js
📚 Learning: 2025-02-20T11:28:09.239Z
Learnt from: cwtickle
Repo: cwtickle/danoniplus PR: 1775
File: js/danoni_main.js:4834-4834
Timestamp: 2025-02-20T11:28:09.239Z
Learning: In the Data Management feature, while `g_windowObj.dataSprite` handles size information, the sprite element name `optionsprite` is intentionally kept unchanged to maintain backward compatibility with previous versions.
Applied to files:
js/lib/danoni_constants.js
📚 Learning: 2024-11-02T10:03:14.300Z
Learnt from: cwtickle
Repo: cwtickle/danoniplus PR: 1717
File: js/danoni_main.js:3796-3799
Timestamp: 2024-11-02T10:03:14.300Z
Learning: setGaugeDetailsはgetGaugeSetting関数の内部関数であり、_scoreIdは呼び出し元で範囲チェック済みのため、追加のバリデーションは不要。
Applied to files:
js/danoni_main.js
📚 Learning: 2025-08-21T12:02:28.519Z
Learnt from: cwtickle
Repo: cwtickle/danoniplus PR: 1898
File: js/danoni_main.js:0-0
Timestamp: 2025-08-21T12:02:28.519Z
Learning: getGaugeSetting の gaugeUpdateFlg（scoreLock＋分割読込時）では、fillMissingArrayElem の呼び出し順は (src=obj[key], base=g_gaugeOptionObj[`gauge${_name}s`][key])。既存値を優先し、空要素のみ補完する（同インデックスは上書きしない）。
Applied to files:
js/danoni_main.js
🧬 Code graph analysis (2)
js/lib/danoni_constants.js (1)
js/danoni_main.js (1)
C_DIS_AUTO(142-142)
js/danoni_main.js (1)
js/lib/danoni_constants.js (13)
j(1574-1574)j(1586-1586)j(1605-1605)j(1610-1610)j(1626-1626)j(1631-1631)j(1712-1712)j(1856-1856)g_limitObj(27-96)g_lblNameObj(3867-4126)g_graphColorObj(904-917)g_stateObj(1045-1125)C_ALIGN_LEFT(17-17)
🔇 Additional comments (2)
js/lib/danoni_constants.js (1)
212-212: LGTM! Pointer events enabled for speed graph interaction.The addition of
pointerEvents: C_DIS_AUTOtodetailObjis consistent with other window objects in this function (lines 207-211) and necessary to enable the mouse hover interaction for the new total speed calculation feature on the speed graph.js/danoni_main.js (1)
7420-7423:addRFunc: () => calculateTotalSpeed()is fine oncecalculateTotalSpeed()is hardened.As-is, this can crash in configurations where the score detail DOM isn’t present. After applying the guard/default handling in
calculateTotalSpeed, this hook becomes safe.-->
Owner
|
この件ですが、時間経過しているのでこちらで手直ししてみます。 |
This was referenced Jan 22, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
🔨 変更内容 / Details of Changes
🔖 関連Issue, 変更理由 / Related Issues, Reason for Changes
📷 スクリーンショット / Screenshot
📝 その他コメント / Other Comments