Skip to content

添加 Discord Webhook 通知功能 - #1282

Closed
jsaaa wants to merge 1 commit into
ok-oldking:masterfrom
jsaaa:pr/discord-webhook-notification
Closed

添加 Discord Webhook 通知功能#1282
jsaaa wants to merge 1 commit into
ok-oldking:masterfrom
jsaaa:pr/discord-webhook-notification

Conversation

@jsaaa

@jsaaa jsaaa commented May 16, 2026

Copy link
Copy Markdown

变更内容

  • 为部分任务添加 Discord Webhook 通知开关,默认关闭,不影响现有用户
  • 任务通知通过 ok-script 提供的通用通知服务发送,避免在本项目中重复维护 Webhook 实现
  • 支持任务通知携带当前截图
  • 补充通知相关 i18n 文案与已编译的 mo 文件

依赖

测试

  • python -m py_compile config.py src/task/BaseWWTask.py src/task/DailyTask.py src/task/FarmEchoTask.py src/task/MultiAccountDailyTask.py
  • 使用本地 ok-script 通过 PYTHONPATH 验证 Notification Config 可导入

备注

  • 本 PR 不包含 fork 同步 workflow
  • Webhook URL 等全局通知配置由 ok-script 提供
  • 通知功能默认关闭,需要用户在任务设置中启用通知并配置 Discord Webhook URL 后才会发送

@jsaaa
jsaaa force-pushed the pr/discord-webhook-notification branch from aa7629e to b2a57cf Compare May 16, 2026 07:20

@github-actions github-actions Bot 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.

🎯 Core Changes

  • Implemented Discord Webhook notification system for task alerts.
  • Refactored BaseWWTask to automatically handle notifications when notify=True is passed to log_info or log_error.
  • Integrated notification content with the project's i18n system for multilingual support.
  • Added UI configuration for Webhook URL, username, and optional screenshot attachments.

⚠️ Concerns

  • Discord API Compatibility: The implementation uses non-standard component types (9, 10, 11, 17) and internal flags (1 << 15) that are not part of the official Discord Webhook API. This will likely result in 400 Bad Request errors for standard webhooks. Using standard embeds is the reliable approach.
  • Performance: Notification delivery is synchronous. Uploading screenshots over a slow connection will block the automation task thread, potentially causing frame drops or logic timing issues in the game.

Code review performed by GOOGLE - gemini-3-flash-preview.

Comment thread src/notification/discord_webhook.py Outdated
if not has_screenshot:
return {"content": content}

return {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The component types used here (17, 9, 10, 11) and the flag 1 << 15 are internal or experimental Discord structures not supported by standard Webhooks. This payload structure is likely to be rejected by Discord with a 400 error. It is highly recommended to use the standard 'embeds' field for rich content and to display screenshots via attachment://screenshot.jpg.

Comment thread src/notification/discord_webhook.py Outdated
}

@staticmethod
def _enable_components(url: str) -> str:

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The query parameter with_components=true is used for fetching messages with components via GET requests in the REST API. It is not a standard or required parameter for POSTing to a Discord Webhook and should be removed.

Comment thread src/notification/discord_webhook.py Outdated
import requests

if image_bytes is None:
response = requests.post(self.webhook_url, json=payload, timeout=self.timeout)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This POST request is synchronous and blocking. Uploading game screenshots can take significant time depending on network conditions. This will pause the entire automation task loop. Consider executing the notification delivery in a background thread.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已处理:DiscordWebhookNotifier.send() 仍然是单次 HTTP 发送函数,但调用方 NotificationService 已经改为后台 daemon thread 发送,并在入队前复制截图帧,因此自动化任务循环不会等待网络上传完成。

Comment thread config.py Outdated
from src.notification.notification_service import NotificationService

logger = Logger.get_logger(__name__)
notification_config = dict(notification_config_option.default_config)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The test function loads configuration from the file system. If a user modifies the Webhook URL in the UI and clicks 'Send Test Notification' before clicking 'Save', the test will use the old/empty configuration from the disk, leading to confusing failures.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

已修复:测试通知不再从磁盘读取 Notification Config.json,改为通过 og.global_config.get_config(notification_config_option) 使用应用当前持有的配置,避免 UI 当前配置与磁盘文件不一致。

@jsaaa
jsaaa force-pushed the pr/discord-webhook-notification branch from b2a57cf to a6a528b Compare May 16, 2026 07:26
@jsaaa

jsaaa commented May 16, 2026

Copy link
Copy Markdown
Author

已按 AI Review 的两点建议调整:

  • Discord payload 改为标准 Webhook embeds,截图使用 attachment://screenshot.jpg,移除了非标准 components / flags / with_components
  • 通知发送改为后台 daemon thread,避免截图上传或网络请求阻塞任务线程。
  • 截图只读取当前已有 frame (nullable_frame) 并复制,不再通过 task.frame 等待新帧。

验证:

  • uv run python -m py_compile config.py src\notification\discord_webhook.py src\notification\notification_service.py src\task\BaseWWTask.py src\task\AutoCombatTask.py src\task\DomainTask.py src\task\FarmMapTask.py src\char\BaseChar.py

备注:最新一次 AI Code Review workflow 失败是 Gemini API 503 Service Unavailable / high demand,不是代码检查失败。

@jsaaa
jsaaa force-pushed the pr/discord-webhook-notification branch from a6a528b to 56f47a4 Compare May 16, 2026 07:30

@github-actions github-actions Bot 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.

🎯 Core Changes

  • Discord Webhook Integration: Adds functionality to send notifications to Discord via webhooks, including support for custom usernames and user mentions.
  • Configurable Notifications: Users can now toggle notifications for INFO and ERROR levels separately and choose whether to attach a game screenshot.
  • Localization Support: Task notifications are integrated into the existing i18n framework, allowing localized alerts for various languages.
  • Test Utility: Adds a "Send Test Notification" button in the configuration GUI for quick verification.
  • Privacy Masking: Implements make_bottom_right_black as a screenshot processor to obscure sensitive on-screen information (like game UIDs) before sending to Discord.

⚠️ Concerns

  • None. The implementation uses daemon threads for network requests to avoid blocking the main automation loop, and the configuration is opt-in, ensuring no impact on existing users.

Code review performed by GOOGLE - gemini-3-flash-preview.

@2-07665

2-07665 commented May 16, 2026

Copy link
Copy Markdown
Contributor

日常一条龙这种定时自启动的任务很需要做外部通知,其它的任务我觉得加了意义不大

@github-actions github-actions Bot 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.

🎯 Core Changes

  • Discord Notification System: Fully implemented the Discord Webhook notification service with task-level granularity.
  • Asynchronous Execution: Notification delivery (including screenshot uploads) is now handled in a background daemon thread, preventing the main automation loop from blocking on network IO.
  • Configuration & Validation: Added a "Send Test Notification" button in the UI and implemented validation logic to warn users if notifications are enabled without a Webhook URL.
  • Internationalization: All notification messages are now routed through the project's i18n system, with Japanese translations updated.
  • Privacy: Screenshots sent via notifications are processed by make_bottom_right_black (likely to hide UIDs).

Fixed issues from previous reviews regarding blocking requests and incorrect configuration source for test notifications.

Verdict: Approve - The implementation is clean, follows established patterns, and addresses all previous performance and logic concerns.


Code review performed by GOOGLE - gemini-3-flash-preview.

@jsaaa

jsaaa commented May 16, 2026

Copy link
Copy Markdown
Author

已根据反馈调整通知开关粒度:

  • 移除了全局 Enable Discord Webhook 开关。
  • 改为由任务配置中的 Send Discord Notification 控制是否发送通知。
  • DailyTask / MultiAccountDailyTask 默认开启,适合定时自启动的一条龙任务。
  • FarmEchoTask 保留任务级开关但默认关闭。
  • 当任务侧开启通知但未配置 Discord Webhook URL 时,会在配置校验和运行时日志中提示用户。
  • 已更新日文 .po / .mo 翻译。

验证:

  • .venv\Scripts\python.exe -m py_compile config.py src\notification\discord_webhook.py src\notification\notification_service.py src\task\BaseWWTask.py src\task\DailyTask.py src\task\MultiAccountDailyTask.py src\task\FarmEchoTask.py

@jsaaa
jsaaa force-pushed the pr/discord-webhook-notification branch 2 times, most recently from ab0c4c9 to ffc3da6 Compare May 16, 2026 19:18

@github-actions github-actions Bot 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.

🎯 Core Changes

  • Added internationalization (i18n) support for the Discord Webhook notification feature.
  • Provided localized strings for Spanish (es_ES), Korean (ko_KR), Simplified Chinese (zh_CN), and Traditional Chinese (zh_TW).

Verdict: Approve. The translations correctly map the new configuration options and log messages, completing the feature integration across supported locales.


Code review performed by GOOGLE - gemini-3-flash-preview.

@2-07665

2-07665 commented May 17, 2026

Copy link
Copy Markdown
Contributor

已根据反馈调整通知开关粒度:

  • 移除了全局 Enable Discord Webhook 开关。
  • 改为由任务配置中的 Send Discord Notification 控制是否发送通知。
  • DailyTask / MultiAccountDailyTask 默认开启,适合定时自启动的一条龙任务。
  • FarmEchoTask 保留任务级开关但默认关闭。
  • 当任务侧开启通知但未配置 Discord Webhook URL 时,会在配置校验和运行时日志中提示用户。
  • 已更新日文 .po / .mo 翻译。

验证:

  • .venv\Scripts\python.exe -m py_compile config.py src\notification\discord_webhook.py src\notification\notification_service.py src\task\BaseWWTask.py src\task\DailyTask.py src\task\MultiAccountDailyTask.py src\task\FarmEchoTask.py

我只是提个个人建议,虽然我是觉得通知功能很有帮助,但是具体你还是需要和 @ok-oldking 交流。比如,可能要考虑把通知集成在ok-script里,而不是在这个项目里给log打补丁。

@ok-oldking

Copy link
Copy Markdown
Owner

改动太多了, 要加也是要加在ok-script库里

@github-actions github-actions Bot 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.

🎯 核心变更

  • 功能迁移与重构:将原有的 Discord Webhook 通知逻辑从本地代码迁移到了 ok 框架内置的通知系统中。为此删除了 src/notification 目录下的服务代码,并从 config.py 中移除了本地定义的配置项。
  • 重构日志与通知接口:重构了 BaseWWTask 中的 log_infolog_error 方法。现在它们会先格式化消息,调用父类日志方法(同时显式禁用父类通知),然后根据任务配置通过框架的 self.notification 发送外部通知。这一改变使通知逻辑更加统一且易于维护。
  • 多语言支持补全:同步更新了西班牙语、日语、韩语及中文(简/繁)的语言包,增加了通知配置及外部推送相关的词条翻译。

⚠️ 注意事项

  • 依赖版本:由于本地 Notification Config 已移除,该功能的可用性完全依赖于底层 ok 框架。请确保框架版本已同步发布并包含对应的内置配置逻辑,否则 UI 中的通知设置可能会消失。

Code review performed by GOOGLE - gemini-3-flash-preview.

@jsaaa
jsaaa force-pushed the pr/discord-webhook-notification branch from b51e99d to 34fe567 Compare May 31, 2026 14:27
Comment thread config.py Fixed
@jsaaa
jsaaa force-pushed the pr/discord-webhook-notification branch from 34fe567 to 3bee6b9 Compare May 31, 2026 14:33

@github-actions github-actions Bot 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.

undefined


Code review performed by GOOGLE - gemini-3-flash-preview.

@sonarqubecloud

Copy link
Copy Markdown

@SHthemW

SHthemW commented Jun 4, 2026

Copy link
Copy Markdown

有没有考虑设置成通用的post url和post body, 实现多平台通知? (参考明日方舟MAA的方案)

@ok-oldking ok-oldking closed this Jun 5, 2026
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.

5 participants