Repository navigation
feat(gens): give the monthly rewards of the gens ranks - #1024
Conversation
The Shining, Elegant, Steel and Old Jewellery Case (14/141 to 14/144). Like other boxes, they're opened by dropping them, and give a Jewel of Chaos, Bless, Soul, Life, Creation or a Gemstone, or money (90,000 or 60,000). The update adds them to existing season 6 databases. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
In the reward period of a month (days 1 to 7 by default), a member of the ranks 1 to 8 gets the jewellery cases of its current rank once, at the npc of its gens (F8 09/0A). The claim is checked and saved in one exclusive operation, and the items are only added, when all of them fit into the inventory. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
sven-n
left a comment
There was a problem hiding this comment.
Reviewed the new commits on top of #1023 (8def3ee to 8a36ac8). The reward claim runs in one exclusive persistence operation. Items are added only when all of them fit, with a rollback on a partial add. RewardClaimedAt per month prevents double claims. The jewellery cases follow the existing item-box pattern, and ItemBoxDroppedPlugIn handles them without changes. The update plugin is idempotent.
Non-blocking notes:
- The free-slot check is
count * Width * HeightagainstFreeSlots.Count(). That works for 1×1 cases, but it doesn't guarantee placement for larger items if someone configures them. The rollback still keeps it safe. - The reward period uses UTC days. Servers in other time zones might find this surprising, so it's worth a sentence in
gens.md. - The 50% jewel chance is a guess, and it's documented as one. Fine.
- Merge conflict ahead: #1020 also adds entries to
Persistence/Initialization/Properties/PlugInResources.resx/Designer.cs, so whichever is merged second needs a small resolution.
Generated by Claude Code
|
#1019, #1021, #1022, #1023 and #1020 are merged now. GitHub refuses to merge this PR ("has merge conflicts"). @apraxico, could you merge the current master into this branch, as you planned? Then only the new changes remain and it can be merged. I can't push to your fork. #1020 added entries to the same Generated by Claude Code |
|
One more note on validation. The PR says you'll test it with MuMain. Per So please verify this in game with the original S6E3 client. The relevant parts are the reward request and response (F8 09 / F8 0A), the result codes, and whether the jewellery cases 14/141–144 are known and shown correctly. Please also mention it in the PR, the same way as for the gens chat Generated by Claude Code |
# Conflicts: # src/Persistence/Initialization/Properties/PlugInResources.Designer.cs # src/Persistence/Initialization/Properties/PlugInResources.resx
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The claim is set and saved before the items are shown to the player, so that the reward can't be claimed again when showing them fails. A failed save is logged. A jewellery case only gets its group of jewels, when at least one of them exists, so that an opened case always gives a jewel or money. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks for the review and for merging the others!
About the validation: understood. So far I tested the claim in game with MuMain only: the 30 cases of rank 1, the second request answered with "already claimed", and an opened case. I'm setting up the original Season 6 Episode 3 client to verify the F8 09/0A results and whether it knows and shows the jewellery cases 14/141–144. I'll also check the |
|
I verified it with the original Season 6 Episode 3 client (1.04d, ENG): a rank 1 member claimed the reward at the npc and got 30 Shining Jewellery Cases, which the client shows with their names and tooltips; the second request was answered with "already claimed"; and a dropped case gave its content. I updated the description. While testing, I found that the original client removes only the |
|
I think you'd need to merge master into this PR again, since a lot of code changed 😅 |
# Conflicts: # docs/Progress.md
The compiled models from master didn't know the new property of the gens member yet. The files in which only the model id changed are left as they were. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…converter Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…cases The names and descriptions come from ItemNames and ItemDropDescriptions now, like the other items since the localization of the configuration captions. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Thanks! I merged the current master again:
All four test suites pass with |
|
Merged, thanks! I re-checked the new commits:
The Azure build didn't run on this head, so I ran the tests locally on 6591a97 with Thanks also for checking with the original S6E3 client. I'm looking forward to the follow-up PR that removes the extra Generated by Claude Code |
The fifth and last part of #1011: the monthly rewards of the gens ranks.
Changes
GensRewardItems), and added to existing databases byAddGensRewardItemsUpdatePlugIn(idempotent);ItemBoxDroppedPlugIn), and give a Jewel of Chaos, Bless, Soul, Life, Creation or a Gemstone, or money: 90,000 for the Shining and Elegant cases, 60,000 for the Steel and Old ones;GensMember.RewardClaimedAt, with a migration.RewardStartDay,RewardEndDayandRewards(rank, item group and number, count) inGensConfiguration.The known server sources take the rank at the end of a month and mark the eligible members. Here, a member gets the reward of its rank at the time it claims it, which the periodic ranking keeps up to date. This needs no separate state which could get out of sync with the members which are online, while they claim their reward.
Tests
GensActionsTestcovers claiming the reward once per month, the reward period and the inventory space.GensRemoteViewTestschecks the bytes of the new results.GensRewardItemsTestchecks the jewellery cases of a new database and the update on an existing one, applied twice.-p:ci=true:MUnique.OpenMU.Tests(1759),Persistence.Initialization.Tests(628, 7 skipped),Network.Packets.Tests(641),PlugIns.Tests(49). No pending model changes.Documentation
docs/Progress.md(the gens group is complete now) and the pageserver-features/gens.mdon the documentation website.🤖 Generated with Claude Code