Optimize Tarot card drawing logic for daily user draws #98
Conversation
文件级别变更
提示和命令与 Sourcery 互动
自定义您的体验访问您的 仪表板 以：
获取帮助Original review guide in EnglishReviewer's Guide by SourceryThis pull request introduces a daily Tarot card draw feature. It stores the user's draw for the day and returns the same card if they try to draw again on the same day. The implementation involves storing user-specific card draw information, checking for existing draws, and returning the stored card if available. Updated class diagram for TarotclassDiagram
class Tarot {
- _user_draws: Dict[int, Tuple[str, dict, str]]
+ get_tarot(user_id: int) MessageChain
+ get_random_tarot() Tuple[dict, str]
}
note for Tarot "Stores user daily results: {user_id: (date, card_info, filename)}"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
嘿 @g1331 - 我已经审查了你的更改 - 这里有一些反馈：
总体评论：
- 考虑将
_user_draws持久化到文件中，以便每日抽奖可以在机器人重启后继续存在。
这是我在审查期间查看的内容
- 🟡 一般问题：发现 3 个问题
- 🟢 安全性：一切看起来都不错
- 🟢 测试：一切看起来都不错
- 🟢 复杂性：一切看起来都不错
- 🟢 文档：一切看起来都不错
帮助我更有用！ 请在每条评论上点击 👍 或 👎，我将使用反馈来改进你的评论。
Original comment in English
Hey @g1331 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider persisting
_user_drawsto a file so the daily draws persist across bot restarts.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| card_type = "正位" if card_dir == "normal" else "逆位" | ||
| content = f"[重复] {card['name']} ({card['name-en']}) {card_type}\n牌意：{card['meaning'][card_dir]}" | ||
| elements = [] | ||
| img_path = f"{os.getcwd()}/statics/tarot/{card_dir}/{filename}.jpg" |
There was a problem hiding this comment.
suggestion: 考虑使用 pathlib 构建跨平台文件路径。
在所有平台上，将 os.getcwd() 与字符串连接使用可能并不稳健。 考虑使用 Path(file).parent / 'statics' / 'tarot' / card_dir / (filename + '.jpg') 构建文件路径。
建议的实施方式：
import os
from pathlib import Path img_path = str(Path(__file__).parent / 'statics' / 'tarot' / card_dir / (filename + '.jpg'))Original comment in English
suggestion: Consider using pathlib for cross-platform file path construction.
Using os.getcwd() with string concatenation may not be robust on all platforms. Consider using Path(file).parent / 'statics' / 'tarot' / card_dir / (filename + '.jpg') to construct the file path.
Suggested implementation:
import os
from pathlib import Path img_path = str(Path(__file__).parent / 'statics' / 'tarot' / card_dir / (filename + '.jpg'))| @staticmethod | ||
| def get_tarot() -> MessageChain: | ||
| async def get_tarot(user_id: int) -> MessageChain: |
There was a problem hiding this comment.
question (bug_risk): 审查异步上下文中 _user_draws 的潜在并发问题。
由于 _user_draws 是从异步函数访问的类级别可变字典，请考虑并发访问是否可能导致竞争条件。 如果此代码并发执行，则简单的锁定机制或异步安全数据结构可能会有所帮助。
Original comment in English
question (bug_risk): Review potential concurrency issues with _user_draws in an async context.
Since _user_draws is a class-level mutable dictionary accessed from an async function, consider whether concurrent access might lead to race conditions. A simple locking mechanism or an async-safe data structure could help if this code is executed concurrently.
| if last_date == today: | ||
| card_dir = ( | ||
| "normal" | ||
| if card["meaning"]["normal"] in card["meaning"].get("current", "") | ||
| else "reverse" | ||
| ) | ||
| card_type = "正位" if card_dir == "normal" else "逆位" | ||
| content = f"[重复] {card['name']} ({card['name-en']}) {card_type}\n牌意：{card['meaning'][card_dir]}" | ||
| elements = [] | ||
| img_path = f"{os.getcwd()}/statics/tarot/{card_dir}/{filename}.jpg" |
There was a problem hiding this comment.
suggestion: 考虑将卡片方向显式存储在缓存中。
目前，重复的分支通过检查 card['meaning']['normal'] 是否在 card['meaning'].get('current', '') 中来推断卡片方向。 如果卡片文本发生更改，这种间接推断可能会容易出错。 将所选方向与卡片详细信息一起存储可以简化逻辑并提高未来的可维护性。
建议的实施方式：
_user_draws: Dict[int, Tuple[str, dict, str, str]] = {} # Check if user already drew today
if user_id in Tarot._user_draws:
last_date, card, filename, card_dir = Tarot._user_draws[user_id]
if last_date == today:
card_type = "正位" if card_dir == "normal" else "逆位"
content = f"[重复] {card['name']} ({card['name-en']}) {card_type}\n牌意：{card['meaning'][card_dir]}"
elements = []
img_path = f"{os.getcwd()}/statics/tarot/{card_dir}/{filename}.jpg"
if filename and os.path.exists(img_path):
elements.append(Image(path=img_path))
elements.append(Plain(text=content))
return MessageChain(elements) # Draw new card
card, filename = Tarot.get_random_tarot()
card_dir = random.choice(["normal", "reverse"])
card["meaning"]["current"] = card["meaning"][card_dir] # Store current meaning
Tarot._user_draws[user_id] = (today, card, filename, card_dir)确保相应地更新代码中依赖于 _user_draws 结构的任何其他部分。
Original comment in English
suggestion: Consider storing the card orientation explicitly in the cache.
Currently, the repeated branch infers the card orientation by checking if card['meaning']['normal'] is in card['meaning'].get('current', ''). This indirect inference may be error prone if card texts change. Storing the chosen orientation alongside the card details could simplify the logic and improve future maintainability.
Suggested implementation:
_user_draws: Dict[int, Tuple[str, dict, str, str]] = {} # Check if user already drew today
if user_id in Tarot._user_draws:
last_date, card, filename, card_dir = Tarot._user_draws[user_id]
if last_date == today:
card_type = "正位" if card_dir == "normal" else "逆位"
content = f"[重复] {card['name']} ({card['name-en']}) {card_type}\n牌意：{card['meaning'][card_dir]}"
elements = []
img_path = f"{os.getcwd()}/statics/tarot/{card_dir}/{filename}.jpg"
if filename and os.path.exists(img_path):
elements.append(Image(path=img_path))
elements.append(Plain(text=content))
return MessageChain(elements) # Draw new card
card, filename = Tarot.get_random_tarot()
card_dir = random.choice(["normal", "reverse"])
card["meaning"]["current"] = card["meaning"][card_dir] # Store current meaning
Tarot._user_draws[user_id] = (today, card, filename, card_dir)Ensure that any other parts of the code that rely on the structure of _user_draws are updated accordingly.
Enhance the Tarot plugin to support daily card draws for users, recording their results to prevent duplicate draws on the same day.
好的，这是翻译成中文的 pull request 总结：
Sourcery 总结
实现每日塔罗牌抽牌功能，存储结果以防止同一天重复抽牌。如果用户尝试在同一天再次抽牌，则显示之前的结果。
新功能：
Original summary in English
Summary by Sourcery
Implement daily Tarot card draws, storing results to prevent duplicate draws on the same day. Display previous results if a user attempts to draw again on the same day.
New Features: