Skip to content

美化 Galgame 模式切换并优化切换反馈#1192

Merged
wehos merged 6 commits into
Project-N-E-K-O:mainfrom
MomiJiSan:feat/galgame-mode-switch-beautify
May 6, 2026
Merged

美化 Galgame 模式切换并优化切换反馈#1192
wehos merged 6 commits into
Project-N-E-K-O:mainfrom
MomiJiSan:feat/galgame-mode-switch-beautify

Conversation

@MomiJiSan
Copy link
Copy Markdown
Contributor

@MomiJiSan MomiJiSan commented May 6, 2026

概要

  • 为 Galgame 模式切换控件增加滑动指示器和模式色彩区分
  • 拆分 mode-switch 与 speed-switch 样式,避免影响速度切换控件
  • 加入 hover、按下、键盘焦点等微交互
  • 加载状态未就绪时隐藏滑块,避免初始错位显示
  • 点击模式后立即更新滑块和摘要文字,避免等待下一次状态刷新
  • 增加 pending 模式保护,避免旧状态刷新把滑块拉回点击前模式
  • 将保存提示改为右上角 toast,提升点击反馈可见性

验证

  • node --check plugin\plugins\galgame_plugin\static\main.js
  • git diff --check -- plugin/plugins/galgame_plugin/static/style.css plugin/plugins/galgame_plugin/static/main.js
  • CSS 大括号数量检查通过

Summary by CodeRabbit

  • 新功能

    • 引入待定模式切换工作流:切换先进入队列并在界面反映,需通过单独“保存”操作才持久化
    • 添加待定状态管理与界面辅助,保存失败或插件不可用时会清除待定状态
  • 修复

    • 增加模式输入校验:无效输入会清除待定并阻止异常状态
  • 样式

    • 优化模式/速度切换的胶囊式样式与响应式布局
    • 新增右上角固定吐司提示样式与定位

MomiJiSan added 2 commits May 7, 2026 00:14
  - N.E.K.O/plugin/plugins/galgame_plugin/static/style.css:1923
      - 拆分 .mode-switch / .speed-switch 共享样式
      - 为模式切换新增滑动指示器 ::before
      - 增加三种模式的颜色区分、hover、active、focus-visible 微交互
      - 响应式下 mode-switch 保持横向,speed-switch 仍可纵向
  - N.E.K.O/plugin/plugins/galgame_plugin/static/main.js:3208
      - renderStatus() 更新 #modeSwitch 的 data-active
      - 根据当前 active 按钮设置 --indicator-left 和 --indicator-width

  验证:

  - node --check plugin\plugins\galgame_plugin\static\main.js 通过
  - git diff --check 通过
  - CSS {} 数量平衡检查通过:open=361, close=361
  - 文档要求的 mode-switch / speed-switch 规则结构检查通过
  - 滑块位置/高亮立即切过去
  - summaryText 里的“模式:...”立即切到新模式
  - 保存成功后仍会主动 refreshAll(... forceRefresh: true),用服务端状态校正

  具体做法:

  - 新增 updateSummaryMode(mode),如果已有 latestStatus,就复用原来的 buildStatusSummaryText({...latestStatus, mode})。
  - 如果还没有状态,就只显示现有 i18n 的:

    ui.summary.mode_part
    ui.mode_label.*

  - 没有新增硬编码提示文案,保存提示仍走现有:

    ui.flash.saving_settings
    ui.flash.settings_saved
    ui.flash.auto_saving_settings
    ui.flash.settings_auto_saved

  验证:

  - node --check plugin\plugins\galgame_plugin\static\main.js 通过
  - git diff --check 通过
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

Walkthrough 喵

为 galgame_plugin 前端引入待处理模式选择队列与对应 UI 控制与样式变更:新增 pendingModeSelectionmodeSaveRequestId,在用户点击模式时先在 UI 展示待处理状态并由单独的保存流程持久化,服务端确认或错误后清除或接受 pending 状态,禁用不可用时的交互并保持状态摘要一致喵。

Changes 喵

模式切换 UX 与状态同步 喵

Layer / File(s) Summary
数据/状态形态
plugin/plugins/galgame_plugin/static/main.js
新增全局变量 pendingModeSelectionmodeSaveRequestId,用于在最新服务端状态到达前暂存并跟踪所选模式喵。
核心实现
plugin/plugins/galgame_plugin/static/main.js
新增 updateModeSwitchControl(currentMode, { ready = true } = {})updateSummaryMode(currentMode)clearPendingModeSelection(mode);修改 renderStatus(status),若服务端状态与 pending 相同则清除 pending,渲染时优先使用 pendingModeSelection 并调用 UI 更新喵。
交互与集成
plugin/plugins/galgame_plugin/static/main.js
模式按钮点击处理器改为入队 pendingModeSelection 并先更新 UI/摘要,移除点击时的立即持久化(保存由单独 Save 路径触发并由 modeSaveRequestId 跟踪);在校验失败、插件不可用或保存出错时调用 clearPendingModeSelection(mode) 喵。
样式/视觉
plugin/plugins/galgame_plugin/static/style.css
新增分段式 pill 样式的 .mode-switch(含 ::before 指示器、模式专属 accent、按钮态与过渡)与独立的 .speed-switch(含按钮态与响应式列布局),并将 .flash-message 替换为固定位置 toast 样式喵。

Sequence Diagram 喵

sequenceDiagram
    participant User
    participant UI as "Mode UI"
    participant Handler as "Event Handler"
    participant State as "State Mgmt"
    participant Server

    User->>UI: 点击模式按钮
    UI->>Handler: 派发点击事件
    Handler->>State: 设置 pendingModeSelection
    Handler->>UI: 调用 updateModeSwitchControl(mode)
    Handler->>UI: 调用 updateSummaryMode(mode)
    Note right of Handler: 保存由单独 Save 操作触发(由 modeSaveRequestId 跟踪)
    Handler->>Server: (通过 Save) 发起 saveMode() 持久化请求
    Server-->>State: 返回并更新最新状态
    State->>Handler: renderStatus(status)(若与 pending 匹配则清除 pending)
    Handler->>UI: 最终调用 updateModeSwitchControl(currentMode)
Loading

Estimated Code Review Effort 喵

🎯 4 (Complex) | ⏱️ ~45 minutes喵

Poem 喵

小小模式先排队喵,
界面先亮不急着回喵,
等服务器来声确认喵,
出错轻撤候位悄悄喵,
前端更顺滑暖洋洋喵 🌟

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed PR 标题准确概括了主要改动:美化 Galgame 模式切换 UI 并优化用户反馈机制,与 JavaScript 和 CSS 变更的核心目标完全匹配。
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.


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.

@chatgpt-codex-connector
Copy link
Copy Markdown

💡 Codex Review

pendingModeSelection = mode;
updateModeSwitchControl(mode);
updateSummaryMode(mode);

P1 Badge Delay optimistic mode switch update until save passes validation

The click handler now sets pendingModeSelection and updates the mode switch/summary before saveMode() runs, but saveMode() can return early on client-side validation errors (for example an out-of-range OCR interval). In that case pendingModeSelection is never cleared, and subsequent renderStatus() calls keep forcing the UI to the unsaved mode (currentMode = pendingModeSelection || statusMode), so users see a mode that was never actually applied. This can cause persistent UI/backend mismatch until a later successful save happens to match the pending value.

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

@MomiJiSan
Copy link
Copy Markdown
Contributor Author

@CodeRabbit review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 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.

  - 新增 clearPendingModeSelection(mode),统一清理 pendingModeSelection 并用 latestStatus 重新渲染 UI。
  - saveMode() 的所有客户端校验早退路径现在都会先清理 pending mode:
      - reader mode 无效
      - OCR interval 越界
      - OCR trigger mode 无效
      - vision max image px 越界
  - 原来的保存异常 catch 分支也改为复用同一个 helper,避免两套清理逻辑分叉。

  这样 mode switch 点击后如果 saveMode() 因本地校验失败直接 return,UI 会立刻回到 backend 当前 mode,不会继续被 pendingModeSelection || statusMode 强制显示为未实际应用的模式。

  验证已跑:

  node --check N.E.K.O\plugin\plugins\galgame_plugin\static\main.js
@wehos
Copy link
Copy Markdown
Contributor

wehos commented May 6, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 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.

@MomiJiSan
Copy link
Copy Markdown
Contributor Author

三百六十五里路哟,走过春夏秋冬

@wehos
Copy link
Copy Markdown
Contributor

wehos commented May 6, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 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.

@wehos
Copy link
Copy Markdown
Contributor

wehos commented May 6, 2026

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 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.

Actionable comments posted: 2

🤖 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 `@plugin/plugins/galgame_plugin/static/main.js`:
- Around line 2957-2960: The mode switch can be clicked when the plugin is
unavailable, leaving a "fake success" UI; fix by making the control truly
disabled when not ready and guarding its click handler. Concretely: when setting
modeSwitchEl.dataset.ready = 'false' ensure updateModeSwitchControl renders a
disabled state (e.g., add a CSS disabled class/aria-disabled and remove pointer
events or set disabled attribute) and have the click handler (the function bound
where mode selection is processed) first check modeSwitchEl.dataset.ready ===
'true' and return early if not; also ensure clearPendingModeSelection uses
latestStatus to restore the correct selected/highlighted UI by resetting the
modeSwitchEl state when latestStatus === null so UI cannot remain stuck on an
incorrect selection. Reference: modeSwitchEl, updateModeSwitchControl,
clearPendingModeSelection, latestStatus and the click handler bound to
`#modeSwitch`.

In `@plugin/plugins/galgame_plugin/static/style.css`:
- Around line 1212-1216: The toast container block (the CSS rule with position:
fixed; top: 18px; right: 24px; width: min(420px, calc(100vw - 32px));) currently
uses z-index: 1000 which is the same stacking level as details.panel-fullscreen
and .modal; increase the toast z-index to a higher value (e.g. >= 2000 or 9999)
so it always sits above details.panel-fullscreen and .modal to prevent it being
covered or losing click events.
🪄 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: 93ec6389-4aad-4d9a-aea3-eabe07f4534e

📥 Commits

Reviewing files that changed from the base of the PR and between aa0e901 and 8e03d52.

📒 Files selected for processing (2)
  • plugin/plugins/galgame_plugin/static/main.js
  • plugin/plugins/galgame_plugin/static/style.css

Comment thread plugin/plugins/galgame_plugin/static/main.js Outdated
Comment thread plugin/plugins/galgame_plugin/static/style.css
… toast above modal

- updateModeSwitchControl(_, { ready: false }) 现在真正进入 disabled 视觉
  + 行为: 按钮 disabled / aria-disabled=true, 清空 active 与 indicator
  CSS 变量, 跳过 getBoundingClientRect 布局计算
- mode-btn click handler 守卫 dataset.ready !== 'true' 时直接 return,
  防止 disabled 按钮被绕过(键盘焦点 / 程序触发)
- renderPluginUnavailable 改用 updateModeSwitchControl('', { ready: false })
  统一渲染, 并清掉 pendingModeSelection 防止 stale UI
- .flash-message z-index 1000 -> 1100, 让 toast 始终盖过 .modal 和
  details.panel-fullscreen (二者都是 1000)

addresses CodeRabbit P1 (Project-N-E-K-O#1192) + Minor

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@wehos
Copy link
Copy Markdown
Contributor

wehos commented May 6, 2026

@chatgpt-codex-connector 这条已经在 8e03d52(PR 作者上一个 commit)修了:新增 clearPendingModeSelection(mode)saveMode() 所有客户端校验早退路径(invalid_reader_mode / invalid_ocr_interval / invalid_ocr_trigger / invalid_vision_max)和 catch 分支都会调用它把 pendingModeSelection 清掉并用 latestStatus 重渲染,UI 不会再停在未保存的 mode 上。

@chatgpt-codex-connector
Copy link
Copy Markdown

To use Codex here, create an environment for this repo.

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 2d371d2eaa

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +3266 to 3270
const currentMode = pendingModeSelection || statusMode;
document.getElementById('summaryText').textContent = buildStatusSummaryText({
...status,
mode: currentMode,
});
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Keep modeSelect in sync with optimistic mode while pending

When pendingModeSelection is set, renderStatus renders the UI summary/switch using currentMode but the form control is still synchronized from status.mode earlier in the function, so the visible mode and #modeSelect can diverge during stale refreshes. In that window, any later saveMode() call (for example from changing speed/push settings or autosave) will read the stale #modeSelect value and send the previous mode back to galgame_set_mode, unintentionally reverting the user's just-selected mode.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed in 85d79be — verified the race trace:

  1. 用户点 mode-btn → modeSelect.value = NEW, pendingModeSelection = NEW, settingsDirty = true, saveMode() 派发
  2. saveMode 成功后 settingsDirty = false; settingsSaveInFlight = false; await refreshAll(...)
  3. During that await, 两个 flag 都已清掉 → 任何 polling refresh 抢跑 renderStatus 用 stale status, syncSettingsValue("modeSelect", STALE) 不再被 shouldPreserveSettingsControls() 挡 → #modeSelect.value 悄悄回到 STALE
  4. UI 因 pendingModeSelection 仍显示 NEW, 但 form control 已 stale → 下次 autosave (任何控件触发) 读到 stale modeSelect → 静默 revert

修法:把 currentMode = pendingModeSelection || statusMode 的计算前移到 syncSettingsValue 之前,syncSettingsValue("modeSelect", currentMode),让 form control 永远跟可见 UI / 乐观状态一致。这样即便上面的 race window 命中,sync 进 modeSelect 的也是 NEW(pending 还在)而不是 STALE。

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)
plugin/plugins/galgame_plugin/static/main.js (1)

5676-5744: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

模式切换的乐观更新还会被“部分成功”和“并发点击”打穿喵

  • 如果 galgame_set_mode 已成功,但后面的 galgame_set_ocr_timinggalgame_set_llm_vision 失败,Line 5739 这里的 clearPendingModeSelection(mode) 会把 UI 回滚到 latestStatus 里的旧模式;这时服务端实际模式已经变了,页面会短暂显示错误状态喵。
  • 另外现在 mode 按钮在保存中仍可继续点击,Line 6823-6836 会并发发出多次 saveMode();旧请求如果后落地,仍可能把用户最后一次点击覆盖成更早的模式,变成真实写错服务端状态的竞态喵。

建议把“mode 是否已提交成功”单独记录,并给模式保存加一个递增请求序号:只有最新一次请求才能清 pending / 刷新;如果 mode 已提交而后续设置失败,应走 refreshAll(... forceRefresh: true) 做服务端校正,而不是回滚到旧 latestStatus 喵。

🐾 可参考的最小修补方向
+let modeSaveRequestId = 0;
+
 async function saveMode({ auto = false } = {}) {
+  const requestId = ++modeSaveRequestId;
+  let modePersisted = false;
   clearSettingsAutosaveTimer();
   const mode = document.getElementById('modeSelect').value;
   ...
   try {
     if (mode && (!latestStatus || latestStatus.mode !== mode)) {
       pendingModeSelection = mode;
       updateModeSwitchControl(mode);
       updateSummaryMode(mode);
     }
     settingsSaveInFlight = true;
     ...
     await callPlugin('galgame_set_mode', {
       mode,
       push_notifications: pushNotifications,
       advance_speed: advanceSpeed,
       reader_mode: readerMode,
     });
+    modePersisted = true;
     await callPlugin('galgame_set_ocr_timing', {
       ...
     });
     await callPlugin('galgame_set_llm_vision', {
       ...
     });
-    await refreshAll({ preserveFlash: true, forceInsights: true, forceRefresh: true });
+    if (requestId === modeSaveRequestId) {
+      await refreshAll({ preserveFlash: true, forceInsights: true, forceRefresh: true });
+    }
   } catch (error) {
-    clearPendingModeSelection(mode);
+    if (requestId === modeSaveRequestId) {
+      if (modePersisted) {
+        await refreshAll({ preserveFlash: true, forceInsights: true, forceRefresh: true });
+      } else {
+        clearPendingModeSelection(mode);
+      }
+    }
     setFlash(error instanceof Error ? error.message : String(error), 'error');
   } finally {
     settingsSaveInFlight = false;
     updateSettingsDirtyHint();
   }
 }
 
 document.querySelectorAll('#modeSwitch .mode-btn').forEach((btn) => {
   btn.addEventListener('click', () => {
+    if (settingsSaveInFlight) {
+      return;
+    }
     const modeSwitchEl = document.getElementById('modeSwitch');
     if (!modeSwitchEl || modeSwitchEl.dataset.ready !== 'true') {
       return;
     }

Also applies to: 6822-6837

🤖 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 `@plugin/plugins/galgame_plugin/static/main.js` around lines 5676 - 5744, The
optimistic mode switch can be overwritten by later/partial failures because
saveMode allows concurrent requests and always calls
clearPendingModeSelection(mode) on error; fix by adding a monotonically
incrementing request id (e.g., saveModeRequestCounter) and a per-request
localRequestId in saveMode, set a flag modeCommitted when
callPlugin('galgame_set_mode') resolves successfully, and only clear or update
pendingModeSelection / call clearPendingModeSelection(mode) if localRequestId
=== current counter (so out‑of‑order responses are ignored); on error, if
modeCommitted is true then call refreshAll({ forceRefresh: true, preserveFlash:
true }) to reconcile with server instead of rolling back to latestStatus; also
prevent concurrent clicks by blocking further saveMode calls while
settingsSaveInFlight is true (or disable the mode control via
updateModeSwitchControl) so only one active saveMode request exists at a time.
🤖 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 `@plugin/plugins/galgame_plugin/static/main.js`:
- Around line 5676-5744: The optimistic mode switch can be overwritten by
later/partial failures because saveMode allows concurrent requests and always
calls clearPendingModeSelection(mode) on error; fix by adding a monotonically
incrementing request id (e.g., saveModeRequestCounter) and a per-request
localRequestId in saveMode, set a flag modeCommitted when
callPlugin('galgame_set_mode') resolves successfully, and only clear or update
pendingModeSelection / call clearPendingModeSelection(mode) if localRequestId
=== current counter (so out‑of‑order responses are ignored); on error, if
modeCommitted is true then call refreshAll({ forceRefresh: true, preserveFlash:
true }) to reconcile with server instead of rolling back to latestStatus; also
prevent concurrent clicks by blocking further saveMode calls while
settingsSaveInFlight is true (or disable the mode control via
updateModeSwitchControl) so only one active saveMode request exists at a time.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: bcef78fb-7f50-4bd3-8ff4-ab376e8efdec

📥 Commits

Reviewing files that changed from the base of the PR and between 8e03d52 and 2d371d2.

📒 Files selected for processing (2)
  • plugin/plugins/galgame_plugin/static/main.js
  • plugin/plugins/galgame_plugin/static/style.css

…rtial-success / concurrent saveMode race

Codex P1 (modeSelect divergence):
  renderStatus 之前先 sync modeSelect = status.mode 再算 currentMode,
  pending 时 modeSelect 与 UI / pendingModeSelection 错位 -> 后续任意
  saveMode (autosave / speed / push) 读到 stale modeSelect 把旧 mode
  发回 backend, 静默 revert 用户选择.
  改成先算 currentMode (pending || status), 再 syncSettingsValue 用
  currentMode, modeSelect 永远跟可见 UI 一致.

CodeRabbit outside-diff (partial-success + concurrent click):
  - galgame_set_mode 已 commit 但后续 set_ocr_timing / set_llm_vision
    抛错时, 旧 catch 走 clearPendingModeSelection(mode) 把 UI 回滚到
    latestStatus.mode (旧), 与 backend 实际状态(已是 NEW) 不一致.
  - 并发点击 mode A -> B 会 spawn 两个 saveMode, 后端响应顺序错时
    backend 可能停在 A.
  Fix: 加 modeSaveRequestId 单调序号, 每次 saveMode 取自己的 requestId
       + modeCommitted 标志.
  - 成功路径 refreshAll 用 requestId === modeSaveRequestId 门控
  - catch: stale 不动 UI; 非 stale 时 modeCommitted 走 forceRefresh
    跟 backend 调和, 否则才 clearPendingModeSelection
  - 早返回 validation 路径不动 (clearPendingModeSelection 自身已 mode-check 安全)

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@wehos
Copy link
Copy Markdown
Contributor

wehos commented May 6, 2026

@coderabbitai outside-diff 那条 partial-success + concurrent click 已在 85d79be 修了,按你 diff 的思路:

  • 加 module 级 modeSaveRequestId 单调序号;每次 saveMode() 入口取自己的 const requestId = ++modeSaveRequestId; let modeCommitted = false;
  • await callPlugin("galgame_set_mode", ...) 之后立即 modeCommitted = true
  • 成功路径的 await refreshAll(...)requestId === modeSaveRequestId 门控(stale 请求不再覆盖最新 saveMode 的 UI / backend reconcile)
  • catch 路径:
    • stale (requestId !== modeSaveRequestId) → 不动 UI(让最新的那次 saveMode 自己负责)
    • non-stale + modeCommittedrefreshAll({ forceRefresh: true }) 跟 backend 真实状态对齐,回滚到 latestStatus.mode(避免你说的"短暂显示错误状态")
    • non-stale + 未 commit → clearPendingModeSelection(mode) 走原回滚路径
  • setFlash(error...) 留在 requestId 守卫外面,所有失败路径都让用户看到错误

没加点击 block —— autosave 在飞时阻止 mode-btn 是不好 UX,序号方案天然能让 stale 出顺序错也不破坏 UI。早返回 validation 路径没加序号守卫,因为 clearPendingModeSelection(mode) 自身就有 pendingModeSelection !== mode 的 mode-check,并发场景已经安全。

另外另一条 Codex 也指的相关问题(#modeSelect 在 pending 时和 UI 错位)一并在同 commit 里修了:把 currentMode 计算前移到 syncSettingsValue("modeSelect", currentMode) 之前,form control 永远跟乐观 UI 一致。

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

Tip

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

@wehos 哼……这个方案喵~实现得挺扎实的,人家不得不承认喵……

单调序号 + modeCommitted 的三段 catch 拆分非常清晰,把「stale 静默」「partial commit 走 forceRefresh」「未 commit 走回滚」三条路径分得明明白白,不会有 stale 请求来乱搞最新 UI 喵。乐观更新和并发安全都照顾到了,设计思路是正确的喵!

有一个小细节值得稍微留意一下喵——setFlash(error...) 在 requestId 守卫外面,意味着 stale 请求失败时会弹 toast。如果场景是:旧请求网络很慢、新请求已经成功并刷新了 UI,这时候旧请求的错误 toast 突然弹出来,用户可能会困惑「咦刚才明明切成功了」喵。当然这是小问题,如果你们觉得"所有失败都告知用户"比"只告知 non-stale 失败"更重要的话,保持现状也完全说得通喵。

currentMode 前移修 #modeSelect 对齐那个,思路也对——form control 跟乐观 indicator 必须同源才不会抖喵。

总体来说,这个 commit 的修法已经把你们之前讨论的几个并发问题处理得很完整了喵。(……才、才不是在夸你们呢,只是客观评价而已喵!)

(´• ω •`) *✲゚*。⋆ 🐇

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 85d79bedd9

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

}

async function saveMode({ auto = false } = {}) {
const requestId = ++modeSaveRequestId;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Increment save request ID after validation

modeSaveRequestId is incremented before any client-side validation, so a later invalid saveMode() call can supersede an earlier valid in-flight save without sending any request. In that case, the earlier save hits requestId !== modeSaveRequestId and skips its refreshAll reconcile path, leaving mode/summaries potentially stale until periodic auto-refresh even though settings were persisted. This is reproducible when users trigger another save action while one is running and a field (like OCR interval) is temporarily invalid.

Useful? React with 👍 / 👎.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Fixed in 8317cc7++modeSaveRequestId 移到 4 个 validation 早返回之后。

之前序号在第一行就 bump 的问题 trace:

  1. saveMode_A 进入,requestId=1,validation 通过,set_mode 已 commit,正在 await set_ocr_timing
  2. 用户在某个控件输入了非法 OCR interval(比如 0.1),autosave 或 click 触发 saveMode_B
  3. saveMode_B 入口 ++modeSaveRequestId → id=2,validation 失败(OCR interval 越界)→ clearPendingModeSelection(mode); return; 没发任何 server 请求
  4. saveMode_A 后续走到 success 路径 if (requestId === modeSaveRequestId) → 1 !== 2 → 跳过 refreshAll
  5. backend 实际已经 commit A 的所有设置,但前端 UI 不 reconcile,直到下次定时 auto-refresh

修法:把 const requestId = ++modeSaveRequestId; 挪到所有 validation 之后、try 之前。无效输入的早返回不再消耗序号,不会 starve 真正在飞的 valid save。

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)
plugin/plugins/galgame_plugin/static/main.js (1)

5736-5759: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

并发保存下陈旧响应仍会覆盖最新 UI 状态喵

在 Line [5736]、Line [5755]、Line [5757],即使 requestId !== modeSaveRequestId,旧请求仍会写 setFlashsettingsDirtysettingsSaveInFlight,会把新请求结果覆盖成错误提示或提前结束“保存中”反馈喵。建议把成功/失败/finally 的 UI 写入统一收敛到“仅最新请求可写”喵。

🐾 建议最小修复喵
   try {
@@
-    setFlash(auto ? uiT('ui.flash.settings_auto_saved', '设置已自动保存') : uiT('ui.flash.settings_saved', '设置已保存'), 'success');
-    settingsDirty = false;
-    settingsSaveInFlight = false;
-    updateSettingsDirtyHint();
-    if (requestId === modeSaveRequestId) {
-      await refreshAll({ preserveFlash: true, forceInsights: true, forceRefresh: true });
-    }
+    if (requestId !== modeSaveRequestId) {
+      return;
+    }
+    setFlash(auto ? uiT('ui.flash.settings_auto_saved', '设置已自动保存') : uiT('ui.flash.settings_saved', '设置已保存'), 'success');
+    settingsDirty = false;
+    settingsSaveInFlight = false;
+    updateSettingsDirtyHint();
+    await refreshAll({ preserveFlash: true, forceInsights: true, forceRefresh: true });
   } catch (error) {
-    if (requestId === modeSaveRequestId) {
-      if (modeCommitted) {
-        try {
-          await refreshAll({ preserveFlash: true, forceRefresh: true });
-        } catch (refreshError) {
-          console.error('[galgame] mode save reconcile refresh failed', refreshError);
-        }
-      } else {
-        clearPendingModeSelection(mode);
-      }
-    }
-    setFlash(error instanceof Error ? error.message : String(error), 'error');
+    if (requestId !== modeSaveRequestId) {
+      return;
+    }
+    if (modeCommitted) {
+      try {
+        await refreshAll({ preserveFlash: true, forceRefresh: true });
+      } catch (refreshError) {
+        console.error('[galgame] mode save reconcile refresh failed', refreshError);
+      }
+    } else {
+      clearPendingModeSelection(mode);
+    }
+    setFlash(error instanceof Error ? error.message : String(error), 'error');
   } finally {
-    settingsSaveInFlight = false;
-    updateSettingsDirtyHint();
+    if (requestId === modeSaveRequestId) {
+      settingsSaveInFlight = false;
+      updateSettingsDirtyHint();
+    }
   }

Based on learnings: 在 Project-N-E-K-O/N.E.K.O 的 JavaScript 审查中,优先将问题集中在一条合并评论里按类别给出喵。

🤖 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 `@plugin/plugins/galgame_plugin/static/main.js` around lines 5736 - 5759,
Concurrent save handlers can let stale responses overwrite current UI state;
guard all UI mutations so only the latest requestId (compare requestId ===
modeSaveRequestId) may change setFlash, settingsDirty, settingsSaveInFlight, and
call updateSettingsDirtyHint/clearPendingModeSelection/refreshAll. Modify the
try/catch/finally flow around the save logic so success branch, error branch,
and finally branch each first check requestId === modeSaveRequestId (or
early-return if not) before mutating those symbols (setFlash, settingsDirty,
settingsSaveInFlight, updateSettingsDirtyHint) and only perform reconcile
actions (modeCommitted handling, clearPendingModeSelection, refreshAll) when
that check passes; keep existing logging for out-of-date responses but avoid UI
writes from stale requests.
🤖 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 `@plugin/plugins/galgame_plugin/static/main.js`:
- Around line 5736-5759: Concurrent save handlers can let stale responses
overwrite current UI state; guard all UI mutations so only the latest requestId
(compare requestId === modeSaveRequestId) may change setFlash, settingsDirty,
settingsSaveInFlight, and call
updateSettingsDirtyHint/clearPendingModeSelection/refreshAll. Modify the
try/catch/finally flow around the save logic so success branch, error branch,
and finally branch each first check requestId === modeSaveRequestId (or
early-return if not) before mutating those symbols (setFlash, settingsDirty,
settingsSaveInFlight, updateSettingsDirtyHint) and only perform reconcile
actions (modeCommitted handling, clearPendingModeSelection, refreshAll) when
that check passes; keep existing logging for out-of-date responses but avoid UI
writes from stale requests.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 3d2c43a4-2650-4252-afbb-ed996d31b40c

📥 Commits

Reviewing files that changed from the base of the PR and between 2d371d2 and 85d79be.

📒 Files selected for processing (1)
  • plugin/plugins/galgame_plugin/static/main.js

…e-request UI writes

Codex P2 (line 5677):
  原来 ++modeSaveRequestId 在 client validation 之前 bump.
  场景: saveMode_A valid 在飞 (id=1, mode 已 commit), 用户改 OCR
  interval 为非法值, autosave/click 触发 saveMode_B, B 取 id=2 然后
  validation 失败早返回 (无任何 server 请求). saveMode_A 后续 success
  路径 requestId(1) !== modeSaveRequestId(2) -> 跳过 refreshAll, 即便
  设置已持久化 UI 也不 reconcile, 直到下次定时刷新.
  Fix: ++modeSaveRequestId 移到所有 validation 早返回之后.

CodeRabbit outside-diff (line 5736-5759):
  原来 stale request 仍会写 setFlash / settingsDirty / settingsSaveInFlight
  / updateSettingsDirtyHint, 把最新 request 的 "保存中" / 成功消息
  覆盖成自己的过期内容.
  Fix: success 路径 await 全部完成后立即检查 stale, 是则 return (finally 自然走);
       catch 头部检查 stale, 是则 console.error log 一下后 return, 不写 UI;
       finally 整体 wrap 在 requestId === modeSaveRequestId 守卫里, latest
       自己负责清 settingsSaveInFlight / 刷 hint, stale 不动.
  这样并发 saveMode 无论先后完成, UI 状态都由最新一次单独负责.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@wehos
Copy link
Copy Markdown
Contributor

wehos commented May 6, 2026

@coderabbitai outside-diff(5736-5759,stale request 写 UI)已在 8317cc7 修了,按你 diff 思路完整收敛:

  • success 路径所有 await 跑完后立即 if (requestId !== modeSaveRequestId) return; —— stale 不写 setFlash/settingsDirty/settingsSaveInFlight/refreshAll
  • catch 头部同样的 stale guard,stale 时 console.error 留 log 但不 setFlash(最新那次的 saving 提示或成功不会被 stale 错误盖掉;最新如果也错,最新的 catch 会 setFlash 真正的错误给用户看)
  • finally 整体 wrap 在 if (requestId === modeSaveRequestId),stale 不清 settingsSaveInFlight / 不刷 dirty hint,让最新那次的 finally 单独负责(这样 latest 在飞期间 settingsSaveInFlight 一直 true,shouldPreserveSettingsControls() 持续保护其他控件不被 polling renderStatus 抢写)

并行另一条 Codex P2 一并修了:++modeSaveRequestId 移到所有 client validation 早返回之后,避免 invalid 早返回 starve 一个真正 commit 完了的 in-flight save。

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

Tip

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

@wehos 哼……人家不得不说,这次收敛得相当彻底喵……(才不是在夸你呢!)

几个关键点都处理对了喵:

  • success 路径末尾的 stale guard —— 把所有副作用(setFlash / dirty / inFlight / refreshAll)全部挡在外面,干净利落喵
  • catch 头部的 stale guard + console.error —— 这个取舍很妙,stale 失败留 log 可查,但绝对不会用过时的错误 toast 盖掉最新那次的 saving/success 提示喵
  • finally 整体 wrap —— 这个是人家觉得最漂亮的部分喵!stale 请求不清 settingsSaveInFlightshouldPreserveSettingsControls() 就能一直保护其他控件不被 polling renderStatus 抢写,整个并发期间的 UI 保护是连续的喵……(嗯,确实想到了喵)
  • ++modeSaveRequestId 后移 —— 这个细节非常关键喵!如果 increment 在 validation 早返回之前,用户快速乱点触发多次 invalid 就会把 modeSaveRequestId 推得很高,让真正 commit 完的 in-flight save 的 requestId 变成 stale,后移之后 invalid 路径完全不消耗序号,并发安全性更强喵。

整体设计已经相当完整了喵~这几个 commit 层层递进把竞态和乐观更新的各种边界情况都覆盖到了,人家……人家就勉为其难地认可啦!(哼!)

(`• ω •´) *✲゚*。⋆ 🐾

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)
plugin/plugins/galgame_plugin/static/main.js (1)

5676-5764: ⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

并发保存现在只防住了旧请求写 UI,没防住旧请求写后端喵

modeSaveRequestId 只包住了成功/失败后的前端回写,但 saveMode() 里这 3 次 callPlugin(...) 还是会并发落到服务端喵。快速连点两个模式时,旧请求 A 仍然可能在新请求 B 之后把 galgame_set_mode / galgame_set_ocr_timing / galgame_set_llm_vision 写回去,最终把服务端状态覆盖成旧快照喵。更糟的是,A 如果已经成功提交了 mode,B 又在提交前失败,Line 5755-5757 还会回滚到 latestStatus,而 A 因为在 Line 5736-5738 被判 stale 不会再 refreshAll(),这样 UI 会显示旧模式,但服务端其实已经是 A 了喵。

这里最好把整次保存串行化,或者改成单个原子保存接口,并在服务端按 requestId / version 丢弃旧写入喵;否则现在的 request guard 只能保证“旧请求别改 UI”,保证不了“旧请求别改配置”喵。

Also applies to: 6842-6856

🤖 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 `@plugin/plugins/galgame_plugin/static/main.js` around lines 5676 - 5764, The
saveMode handler allows concurrent backend writes because the three callPlugin
calls (galgame_set_mode, galgame_set_ocr_timing, galgame_set_llm_vision) are not
tied to modeSaveRequestId, so an older request A can still overwrite a newer
request B; fix by making the whole save atomic or serialized: either (A) create
a single atomic endpoint (e.g. galgame_set_all_settings) and call that from
saveMode so one request updates mode+ocr+vision together, or (B) serialize
client saves by guarding all callPlugin invocations with a per-save lock/request
token (use modeSaveRequestId/saveInFlightRequestId) and check the token before
each callPlugin and abort remaining calls if stale, and ensure
pendingModeSelection/latestStatus/refreshAll logic only runs for the current
token; pick one approach and update saveMode, the call sites of callPlugin, and
server handler to ignore/outdate requests by requestId if you implement
server-side protection.
🤖 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 `@plugin/plugins/galgame_plugin/static/main.js`:
- Around line 5676-5764: The saveMode handler allows concurrent backend writes
because the three callPlugin calls (galgame_set_mode, galgame_set_ocr_timing,
galgame_set_llm_vision) are not tied to modeSaveRequestId, so an older request A
can still overwrite a newer request B; fix by making the whole save atomic or
serialized: either (A) create a single atomic endpoint (e.g.
galgame_set_all_settings) and call that from saveMode so one request updates
mode+ocr+vision together, or (B) serialize client saves by guarding all
callPlugin invocations with a per-save lock/request token (use
modeSaveRequestId/saveInFlightRequestId) and check the token before each
callPlugin and abort remaining calls if stale, and ensure
pendingModeSelection/latestStatus/refreshAll logic only runs for the current
token; pick one approach and update saveMode, the call sites of callPlugin, and
server handler to ignore/outdate requests by requestId if you implement
server-side protection.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0a6d95ff-e003-4ba6-85da-8fbcc44bc366

📥 Commits

Reviewing files that changed from the base of the PR and between 85d79be and 8317cc7.

📒 Files selected for processing (1)
  • plugin/plugins/galgame_plugin/static/main.js

Copy link
Copy Markdown
Contributor

@wehos wehos left a comment

Choose a reason for hiding this comment

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

好耶!

@wehos wehos merged commit 06b87c3 into Project-N-E-K-O:main May 6, 2026
2 checks passed
@wehos
Copy link
Copy Markdown
Contributor

wehos commented May 6, 2026

@coderabbitai 这条 outside-diff(5676-5764, backend write ordering)我打算 不在这个 PR 里修,独立判断如下:

问题真实性:你的描述是对的——modeSaveRequestId 只挡住了 UI 写,三个 callPlugin 在 backend 那侧无序到达确实可能让旧 request 的写覆盖新 request。

但 scope 不匹配

  1. pre-existing:这个 backend 写顺序 race 是 saveMode 一直就有的属性。autosave / saveModeBtn / modeSelect change event 都能 fire saveMode。本 PR 只是因为 mode-btn click 直接 trigger saveMode 让暴露面稍微大一点,不是引入这个 race。
  2. 实际概率低:浏览器 fetch 同源走 HTTP/2 keep-alive 单连接,请求按发送顺序到达 backend;要触发真乱序得 HTTP/1.1 多连接 + 后端并行处理 plugin call。Electron / 本机 Pet 这条链路不太会命中。
  3. 现有 guard 已经"几乎正确":即便 backend 真乱序了,最新一次 saveMode 的 refreshAll 会拿 backend ground truth 重渲染——UI 与 backend 仍一致。只是 backend "可能" 停在用户倒数第二次点击的状态。这是 race 自带的"最终一致"上界,不是新 bug。
  4. fix 成本不匹配 PR scope:本 PR 是 mode switch 美化 + 反馈优化。要么加 backend atomic endpoint(动 plugin 后端 + 协议),要么客户端引入全局 saveMode 串行化锁(每次 save 都串行,autosave + 手动 + click 全 serialize,影响整个设置面板的 UX)。两条路都远超 PR scope。

建议作为单独 issue 跟,标 enhancement: serialize concurrent saveMode backend writes。这个 PR 在 UI 层面把"乐观更新 / stale UI 写"这一层做完已经达到目标,backend 写串行化是另一个独立 concern。

如果作者希望在这个 PR 里也修,再单独沟通。

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 6, 2026

Tip

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

@wehos 哼……人家仔细看了你的分析,不得不承认逻辑非常严密喵……(才不是被说服了呢!)

四点理由都站得住脚喵:

  1. pre-existing:这个 race 确实不是本 PR 引入的,只是暴露面略大了一点,不应该算本 PR 的 regression 喵。
  2. 实际概率低:HTTP/2 keep-alive 单连接 + 本机 Electron 链路,乱序窗口本来就很小,不值得为这个 edge case 在这里开刀喵。
  3. 最终一致兜底:最新 saveMode 的 refreshAll 会用 backend ground truth 重渲染,UI 和 backend 最终还是对齐的——不是悬空的错误状态,只是 backend 可能停在倒数第二次的写,这在"最终一致"模型下是已知上界,不是新 bug 喵。
  4. fix 成本不匹配:加 atomic endpoint 或全局串行锁的改动量远超 UI 美化 PR 的边界,硬塞进来反而不好维护喵。

这个 PR 在 UI 层的乐观更新 + stale 写保护已经做得相当完整了,backend 写串行化完全可以作为独立 concern 单独跟进喵。


要人家帮你创建一个 GitHub Issue 来追踪这个 enhancement: serialize concurrent saveMode backend writes 吗喵?说一声就帮你开喵~ (´• ω •`)✨

@MomiJiSan MomiJiSan deleted the feat/galgame-mode-switch-beautify branch May 7, 2026 01:13
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