Skip to content

feat(gens): give the monthly rewards of the gens ranks - #1024

Merged
sven-n merged 15 commits into
MUnique:masterfrom
apraxico:feature/gens-rewards
Oct 14, 2026
Merged

sven-n merged 15 commits into
MUnique:masterfrom
apraxico:feature/gens-rewards

Conversation

@apraxico

@apraxico apraxico commented Oct 1, 2026 •

Copy link
Copy Markdown
Contributor

The fifth and last part of #1011: the monthly rewards of the gens ranks.

Changes

  • Jewellery cases (14/141 to 14/144: Shining, Elegant, Steel and Old Jewellery Case), which didn't exist yet:
    • created by the season 6 initialization (GensRewardItems), and added to existing databases by AddGensRewardItemsUpdatePlugIn (idempotent);
    • like other boxes, they're opened by dropping them (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;
    • the possible contents are from the known item data (e.g. muonlinefanz), but I found no chances. I used 50 % for a jewel and documented it; it can be changed in the drop groups of the items.
  • Persistence: GensMember.RewardClaimedAt, with a migration.
  • Reward request (F8 09, answered with F8 0A):
    • in the reward period of a month (days 1 to 7 by default, UTC), a member gets the reward of its rank once per month, at the npc of its gens: 30/20 Shining (ranks 1/2), 20/10 Elegant (3/4), 10/5 Steel (5/6) and 5/3 Old (7/8) jewellery cases;
    • the results: outside of the reward period, not eligible, not enough inventory space, already claimed, a different gens and not joined, as the client knows them;
    • the checks and the claim are done in one exclusive persistence operation, so that two requests can't both get the reward. The items are only added, when all of them fit into the inventory.
  • Configuration: RewardStartDay, RewardEndDay and Rewards (rank, item group and number, count) in GensConfiguration.

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

  • New: GensActionsTest covers claiming the reward once per month, the reward period and the inventory space. GensRemoteViewTests checks the bytes of the new results. GensRewardItemsTest checks the jewellery cases of a new database and the update on an existing one, applied twice.
  • Passing, merged with the current master and built with -p:ci=true: MUnique.OpenMU.Tests (1759), Persistence.Initialization.Tests (628, 7 skipped), Network.Packets.Tests (641), PlugIns.Tests (49). No pending model changes.
  • Verified in game with MuMain: a rank 1 member got 30 Shining Jewellery Cases, the second request was answered with "already claimed", and an opened case gave its content.
  • Verified in game 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.

Documentation

docs/Progress.md (the gens group is complete now) and the page server-features/gens.md on the documentation website.

🤖 Generated with Claude Code

apraxico and others added 7 commits October 1, 2026 02:31
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 sven-n left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

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 * Height against FreeSlots.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

sven-n commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

#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 Persistence.Initialization PlugInResources.resx/Designer.cs, so you'll need to resolve a small conflict there: keep both sets of entries.


Generated by Claude Code

sven-n commented Oct 1, 2026

Copy link
Copy Markdown
Member

One more note on validation. The PR says you'll test it with MuMain. Per docs/CODING_RULES.md, the primary target is the original Season 6 Episode 3 client: original packets (no suffix) must stay compatible with it (section 3), and client-side behaviour should be cross-checked against the original client (section 9). MuMain is useful for observing behaviour and protocol facts, but it's being actively refactored and isn't the reference.

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 $ prefix handling in #1023, which was derived from MuMain's ReceiveChat.


Generated by Claude Code

apraxico and others added 3 commits October 2, 2026 01:55
# 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>
@apraxico

apraxico commented Oct 2, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the review and for merging the others!

  • I merged the current master into this branch. In the PlugInResources.resx/Designer.cs of Persistence.Initialization, I kept both sets of entries (fix(persistence): use the warp index 42 for Vulcanus, like the season 6 client #1020 and this one). GitHub shows it as mergeable now.
  • gens.md now says that the reward period is counted in UTC, and what that means on servers in other time zones.
  • A review of the claim found a small gap, fixed in fix(gens): mark the reward as claimed before the items are shown:
    • the claim is now saved before the items are shown, so showing them can't fail after the items were added but before the claim;
    • a failed save is logged;
    • a jewellery case only gets its group of jewels when at least one of them exists, so an opened case always gives something.
  • Tests: MUnique.OpenMU.Tests, Persistence.Initialization.Tests, Network.Packets.Tests and PlugIns.Tests pass with -p:ci=true, with one exception: ItemRuleFlagsTests.WearableItemIsOnlyRepairedWhenRepairableAsync(True) fails, in ItemExtensions.IsTransformationRing (called from ItemRepairAction). It fails on the current master alone, too (f32bd25, after Item Durability Logic Refactor #966), so it isn't related to this pull request.

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 $ prefix of #1023 with it, which I derived from MuMain's ReceiveChat. I'll report the results here and update the description.

@apraxico

apraxico commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

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 $ of a gens chat message, while MuMain removes two characters. The extra $ from #1023 is therefore visible in the original client. I'll open a separate pull request which removes it, together with a fix in MuMain.

@sven-n

sven-n commented Oct 8, 2026

Copy link
Copy Markdown
Member

I think you'd need to merge master into this PR again, since a lot of code changed 😅

apraxico and others added 5 commits October 8, 2026 15:14
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>
@apraxico

Copy link
Copy Markdown
Contributor Author

Thanks! I merged the current master again:

  • The jewellery cases take their names and drop descriptions from ItemNames and ItemDropDescriptions now (en, de, zh-CN), because FullConfigurationCaptionTests requires it since the localization of the captions.
  • RewardClaimedAt is in the compiled models and in the new generated BasicModelConverter.

All four test suites pass with -p:ci=true, and there are no pending model changes.

@sven-n
sven-n merged commit 3c4d131 into MUnique:master Oct 14, 2026
2 checks passed

sven-n commented Oct 14, 2026

Copy link
Copy Markdown
Member

Merged, thanks! I re-checked the new commits:

  • the claim is now saved before the items are shown;
  • the jewellery case names and drop descriptions are localized;
  • RewardClaimedAt is in the compiled models and in BasicModelConverter;
  • the earlier master merge kept the resource entries of both PRs.

The Azure build didn't run on this head, so I ran the tests locally on 6591a97 with -p:ci=true: MUnique.OpenMU.Tests (1759 passed) and Persistence.Initialization.Tests (628 passed, 7 skipped).

Thanks also for checking with the original S6E3 client. I'm looking forward to the follow-up PR that removes the extra $ from #1023.


Generated by Claude Code

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.

2 participants