[OviedoMarcelo]

No description provided.

[sven-n]

Thanks for sharing this work. Some of it is useful, but this PR can't be merged in its current form.

Scope doesn't match the title

The title ("Fix/announcements messages field hidden") describes a one-line change (IList<T> → ICollection<T> in ServerAnnouncementsConfiguration). The PR is actually the fork's whole master branch: 30 commits, 115 files, +9952/−329, including merge commits of the fork's own PRs #1–#11 and an upstream sync. Nobody can properly review that as one unit, and reverting any single part of it later wouldn't be practical.

Blockers

1. It removes all GitHub Actions workflows (dotnetcore.yml, publish.yml, docs-website.yml, claude.yml, discord-merge-notification.yml). The commit message says it outright: "This fork doesn't use CI, releases or notifications, so no workflow should run on it." That's fine for your fork, but it can't go upstream. It would turn off the build, the releases and the docs site for this project.
2. Fork-specific customizations that don't belong upstream:
   • FounderExperienceBonusPlugIn: its default cutoff is hardcoded to your beta launch (2026-10-12 13:00 UTC, "Argentina time"), and its docs point to your website (lib/beta.ts, BETA_OPENS_AT).
   • WebsiteController (api/web/...): a custom API for your community website. Its culture is hardcoded to "es", it can disconnect accounts, and it isn't generic.
   • The admin panel is partly localized to Spanish in a way that bypasses the resource system: SpanishHelp.*.cs, the Help.razor page (/ayuda), hardcoded Spanish title= tooltips and section headers in NavMenu.razor, the hardcoded "Impacto:" in FieldLabel.razor, and Spanish Display descriptions and default messages in the new plugin configs. Upstream localizes through .resx resources, not hardcoded strings or a language-specific class.
3. Protocol extensions for a custom client. The new C2 F5 02..06 packets (weekly quests, quest details, skill requirements, learnable item requirements, monster levels) are sent to every 106.3 Invariant client after login or map entry. They need a matching, agreed client implementation. Protocol additions like these should go in their own PR, together with the client-side change (e.g. in MuMain), so we can check that stock clients handle them safely.
4. CI is red. The Azure MUnique.OpenMU build failed on the head commit, and Codacy reports action_required.

Parts that could be upstreamed as separate, focused PRs

Some changes look useful. I'd happily review them one at a time, each with its own tests:

• Guild kick/disband security code fix (GuildKickPlayerAction): falls back to the password check like DeleteCharacterAction. It also stops logging the security code in plain text. This looks like a real bug fix.
• Slide notice (MessageType.SlideNotice, /slidenotice, and the message type selector in the admin panel's global message).
• DataTable refresh jumping back to page 1 when the current page becomes empty.
• Stacked modal dialogs (ModalService / ModalContainer).
• tzdata in the Docker image.
• Learnable-skill requirement check (LearnableSkillRequirements, the Summoning Orb refactor). Please send it separately from the new packets.
• Server Announcements plugin, with English defaults and resource-based texts.
• Weekly quests. This is a large feature with its own DB context/schema, migrations, new plugin points (ICharacterResetPlugIn, IMiniGameEndedPlugIn, IItemPickedUpPlugIn, INpcTalkStartedPlugIn) and a static service locator (WeeklyQuestProgressRepositoryRegistry.Current). Please open an issue or discussion about the design first, then send it as a dedicated PR.

Suggestion

Please close this PR. Then open separate PRs from branches based on upstream master (not on your fork's master), and leave out the workflow removal and the server-specific customizations.

---

Generated by Claude Code

[sven-n]

Closing this as discussed in the review. The useful parts are split into focused pull requests against the current master. Your commits are cherry-picked with your authorship where they applied as they were; reworked parts credit you as author.

Part	Pull request	Notes
Guild kick/disband security code fix	#1237	Cherry-picked, plus tests
tzdata in the Docker image	#1238	Cherry-picked
DataTable refresh back to page 1	#1239	Cherry-picked, plus a bUnit test
Slide notices (/slidenotice, admin panel message type)	#1241	The packet is now defined in ServerToClientPackets.xml (SlideNotice) and generated, instead of written by hand
Server announcements plugin	#1242	Texts are LocalizedString (per player language), labels in PlugInResources, per-server state in the plugin instance, English defaults
Learning a skill requires the skill's requirements	#1243	Server-side check only, reusing the existing GetLearnableSkill override of the Summoning Orb

Not split out:

• Stacked modal dialogs: already on master since fix(admin): support nested modals #1194.

• Skill requirements / monster levels packets (C2 F5 04–06) and the weekly quest packets (C2 F5 02/03): the open source client doesn't handle these sub codes yet. New client protocol should come as …Extended packets together with the client change in MuMain (see Protocol compatibility in docs/CODING_RULES.md).

• Weekly quests: a feature this large needs a design discussion before a pull request (see AGENTS.md), e.g.:

  • its own DB context and schema vs. the existing quest model
  • the static WeeklyQuestProgressRepositoryRegistry
  • localized quest texts
  • how the client shows the list

  If you'd like to get it upstream, please open an issue so we can agree on the approach first.

• Website API, Founder bonus, Spanish admin panel help, removal of the workflows: specific to your server/fork, so they stay there.

Thanks for the contribution!

---

Generated by Claude Code