Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimize Tarot card drawing logic for daily user draws #98

Open
wants to merge 1 commit into
base: v3
Choose a base branch
from
+38 −20
Open

Optimize Tarot card drawing logic for daily user draws #98

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 38 additions & 20 deletions modules/self_contained/tarot/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,57 +1,75 @@
import json
import os
import random
from datetime import datetime
from pathlib import Path
from typing import Dict, Tuple

from graia.ariadne.app import Ariadne
from graia.ariadne.event.message import Group, GroupMessage
from graia.ariadne.event.message import Group, GroupMessage, Member
from graia.ariadne.message.chain import MessageChain
from graia.ariadne.message.element import Plain, Image, Source
from graia.ariadne.message.parser.twilight import Twilight, FullMatch, SpacePolicy
from graia.ariadne.util.saya import listen, decorate, dispatch
from graia.saya import Saya, Channel

from core.control import (
Permission,
Function,
FrequencyLimitation,
Distribute
)
from core.control import Permission, Function, FrequencyLimitation, Distribute
from core.models import saya_model

module_controller = saya_model.get_module_controller()
saya = Saya.current()
channel = Channel.current()
channel.meta["name"] = ("Tarot")
channel.meta["author"] = ("SAGIRI-kawaii")
channel.meta["description"] = ("可以抽塔罗牌的插件，在群中发送 `-塔罗牌` 即可")
channel.meta["name"] = "Tarot"
channel.meta["author"] = "SAGIRI-kawaii"
channel.meta["description"] = "可以抽塔罗牌的插件，在群中发送 `-塔罗牌` 即可"
channel.metadata = module_controller.get_metadata_from_path(Path(__file__))


@listen(GroupMessage)
@dispatch(
Twilight(
[
FullMatch("-塔罗牌").space(SpacePolicy.PRESERVE)
]
)
)
@dispatch(Twilight([FullMatch("-塔罗牌").space(SpacePolicy.PRESERVE)]))
@decorate(
Distribute.require(),
Function.require(channel.module),
FrequencyLimitation.require(channel.module),
Permission.group_require(channel.metadata.level),
Permission.user_require(Permission.User),
)
async def tarot(app: Ariadne, group: Group, source: Source):
await app.send_group_message(group, Tarot.get_tarot(), quote=source)
async def tarot(app: Ariadne, group: Group, sender: Member, source: Source):
await app.send_group_message(group, await Tarot.get_tarot(sender.id), quote=source)


class Tarot(object):
# Store user daily results: {user_id: (date, card_info, filename)}
_user_draws: Dict[int, Tuple[str, dict, str]] = {}

@staticmethod
def get_tarot() -> MessageChain:
async def get_tarot(user_id: int) -> MessageChain:
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

today = datetime.now().strftime("%Y-%m-%d")

# Check if user already drew today
if user_id in Tarot._user_draws:
last_date, card, filename = Tarot._user_draws[user_id]
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"
Copy link
Contributor

Choose a reason for hiding this comment

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

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'))

Comment on lines +52 to +61
Copy link
Contributor

Choose a reason for hiding this comment

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

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.

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_type = "正位" if card_dir == "normal" else "逆位"
content = f"{card['name']} ({card['name-en']}) {card_type}\n牌意：{card['meaning'][card_dir]}"
elements = []
Expand Down