Repository navigation
feat(items): add the lucky coin exchange of delgado - #1464
Conversation
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 65 |
| Duplication | 13 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e96fc05de9
ℹ️ 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".
sven-n
left a comment
sven-n
left a comment
There was a problem hiding this comment.
This PR conflicts with current master, so I can't merge it. Please rebase it onto master. These files conflict:
src/GameLogic/Properties/PlugInResources.Designer.cssrc/GameLogic/Properties/PlugInResources.resxsrc/Persistence/Initialization/Properties/ItemNames.Designer.cssrc/Persistence/Initialization/Properties/ItemNames.de.resxsrc/Persistence/Initialization/Properties/ItemNames.resxsrc/Persistence/Initialization/Properties/ItemNames.zh-CN.resxsrc/Persistence/Initialization/Properties/PlugInResources.Designer.cssrc/Persistence/Initialization/Properties/PlugInResources.de.resxsrc/Persistence/Initialization/Properties/PlugInResources.resxsrc/Persistence/Initialization/Properties/PlugInResources.zh-CN.resxtests/MUnique.OpenMU.Persistence.Initialization.Tests/ItemStatProfilesDataTest.cs
Most of them are resource files (.resx / .Designer.cs) which other merged PRs also extended. When you resolve them, keep the entries of both sides, and regenerate the designer file instead of merging it by hand. I'll review it in full once it's rebased.
Generated by Claude Code
44993de to
5ff959d
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5ff959df5e
ℹ️ 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".
| && definition.Group == ItemConstants.LuckyCoin.Group | ||
| && definition.Number == ItemConstants.LuckyCoin.Number |
There was a problem hiding this comment.
Both registration and exchange use this filter, which fixes the accepted item to group 14/number 100. LuckyCoinConfiguration exposes only rewards, so operators cannot change the accepted currency without rebuilding GameLogic. Rule 3 of docs/CODING_RULES.md, made authoritative by AGENTS.md, requires gameplay values to come from configuration and explicitly states that client-side hard-coding is not an exemption. Add the accepted item group/number to the feature configuration, defaulting to 14/100, and use those settings here.
AGENTS.md reference: AGENTS.md:L16-L17
Useful? React with 👍 / 👎.
5ff959d to
d76b5c4
Compare
sven-n
left a comment
sven-n
left a comment
There was a problem hiding this comment.
Thanks for the rebase. The lucky coin flow (BF 0B/0C/0D, registering counts at the character, exchanging behind a feature plugin with configurable rewards) looks good to me.
Unfortunately, #1504 was merged in the meantime and regenerated GameLogic/Properties/PlugInResources.Designer.cs, so this PR conflicts again (GameLogic/Properties/PlugInResources.*, GameServer/Properties/PlugInResources.*). Please rebase once more: keep the entries of both sides in the .resx files, then regenerate the designers. Once it's clean and green, I'll merge it.
(Your item PRs #1458, #1461, #1464, #1470 and #1471 all add to the same resource files, so each merge makes the others conflict again. I'll take whichever is ready first.)
Generated by Claude Code
Part of MUnique#1339 Adds the Lucky Coin (14/100), opens the lucky coin dialog at Delgado and handles the client packets BF 0B/0C/0D: coins can be registered, which counts them at the character, or exchanged for a random configured item. The rewards are configured in the new lucky coin feature plugin. Rebased onto master: this is the change of the earlier commits in one. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011xNc76CH3CGWUp5r4XmXWK
d76b5c4 to
e0a015b
Compare
Part of #1339
Adds the Lucky Coin (14/100), opens the lucky coin dialog at Delgado and handles the client packets BF 0B/0C/0D: coins can be registered, which counts them at the character, or exchanged for a random configured item. The rewards are configured in the new lucky coin feature plugin.
Claude-Session: https://claude.ai/code/session_011xNc75fJTee8EPxDxQxuGi