The packet handler funnel now holds each player's persistence lock for the
whole handler. Acquiring a second player's lock from inside a handler is
therefore a lock-ordering hazard; note the invariant on the guard method so a
future cross-player save cannot silently open an AB-BA cycle. Documentation
only, no behavioural change.
The per-player persistence lock serializes the packet handler funnel and the
save, but a few structural mutations happen off that funnel: the offline/bot
MuHelper loots and maintains its inventory on a 500ms timer, and combat
destroys depleted ammunition or a dead pet on the attacker's or a monster's
thread. Those can still run while the periodic save enumerates the change
tracker and corrupt it.
Two additions:
- Run the whole offline MuHelper tick under the player's persistence lock.
Bots are in the saved player list and loot continuously, so this was the
most likely remaining reproducer. The tick has no internal delays, so the
lock is held only briefly, and it is the bot's own lock (no cross-player
deadlock).
- Retry the save a bounded number of times on the transient exceptions a
concurrent change-tracker mutation produces. The corruption surfaces as
several types depending on where change detection was (a modified
collection, a transiently-null key, an out-of-range index), so the retry
covers that family rather than a single type. A genuinely persistent error
rethrows once the attempts are exhausted. This absorbs the rare, bursty
combat sources that no lock is held for.
A player's progress is persisted from two unrelated flows: action
handlers triggered by incoming packets (sequential per connection) and
the periodic save, which runs on an independent timer. Action handlers
mutate tracked entities with plain field and collection writes - for
example crafting toggles item.ItemOptions, and item stacking and NPC
selling delete item rows - which bypass the persistence context's own
lock. When such a mutation runs while SaveChangesAsync enumerates the
change tracker, the save throws (collection-modified, or a
DbUpdateConcurrency "affected 0 rows" since the context has no
concurrency tokens). SaveChanges is atomic, so every following save
fails too and the whole session never persists: on relog the player
rolls back, losing progress and items.
Add a per-player re-entrant persistence lock and acquire it around both
the packet-handling funnel and SaveProgressAsync, so a player's
mutations and saves can never overlap. The lock is re-entrant per
asynchronous flow (an instance AsyncLocal), so an inline save inside an
already-serialized handler does not deadlock; and it is per player, so a
trade still acquires the trading partner's lock separately.
Add regression tests covering mutual exclusion and re-entrancy of the
lock.
The reset-aware level gate in BotPartyHandler rejected nearly every party
invitation on servers with resets, because it folded reset count into the level
scale (one reset ~= 400 points against a 500 cap). A veteran player inviting a
freshly generated bot was always over the limit, so the invitation was silently
declined. Remove the gate so a bot accepts any inviter who is alive and in the
world, matching OpenMU's own party action; the situational safeguards (shopping,
revenge, mini game, pending invite, human companion) stay in place.
The switches used to be held by simply standing near them, and the crown captured
by standing near it - clicking a switch only produced the client's "not implemented
yet" message. This drives both from the original interaction instead:
- Clicking a Crown Switch starts an operation which completes after
CastleSiegeSettings.SwitchPushSeconds (15) and keeps the switch for the guild
until its operator leaves the switch's area. One player per switch; anybody else
clicking it is told another team is on it (C1 B2 14 state 2).
- While one guild holds both switches the crown's shield drops for it, and its
guild master captures the throne by CLICKING the crown and holding it for
CrownHoldTimeSeconds - seeded to 60 now, to match the countdown the client's
registration panel hardcodes. The throne stays contestable until the siege ends.
- The shield now depends on the switches alone, as in the original; the gates and
statues remain what they always were, the obstacle in the way.
The switch info packet (C1 B2 20) is broadcast before any switch-state packet
because the client's "switch released" handler reads its switch table without
checking that it exists - that table is only allocated when the info packet
arrives, so the wrong order crashes the client.
Also fixed while in here:
- The crown registration panel could never be closed: the cancel was only sent
while the master still stood on the crown, which is precisely when the hold does
NOT break. The panel is now closed for the player it was opened for.
- A contested switch was decided by enumeration order.
- Panels opened by the siege are closed when it ends.
- TryCaptureThrone was dead code carrying a second, diverged rule set.
- /csphase advertised the pre-refactor phase names to the client.
- The periodic broadcasts keyed off "UtcNow.Second % n", which silently skips when
a tick runs late; they count ticks now.
The unit tests never compiled against the refactored model - they are migrated to
the state machine and guild ids, and cover the new switch and crown rules. A new
test proves update 105 writes the configuration into an existing database.
ApplyPendingUpdatesTool applies pending configuration updates without the admin
panel; it is [Explicit], so it never runs in a normal test pass.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Moves AdaMu's working Castle Siege onto the upstream data model that the
previous commit introduced, without changing how the siege plays.
State model
- CastleSiegePhase is replaced by DataModel's CastleSiegeState, whose values are
exactly what the game client's CASTLESIEGE_STATE enum expects. The cycle now
runs Idle1(0) -> RegisterGuild(1) -> Ready(6) -> Start(7) -> End(8) ->
EndCycle(9) -> Idle1(0).
- Idle2(2), RegisterMark(3), Idle3(4) and Notify(5) keep their numbers for client
compatibility but are never entered: AdaMu registers guilds directly and has no
Mark of Lord step.
Guild identity
- Guilds are now identified by their persistent Guid instead of by name, so a
rename (or a delete and re-create under the same name) can no longer hand
castle ownership to the wrong guild. Names are carried alongside only for
display and for the packets that send a name to the client.
- Interfaces.Guild deliberately has no id and the guild server's short ids are
in-memory only, so the persistent id is resolved through the guild name once
and cached per process. This avoids adding a method to IGuildServer, which
upstream keeps changing.
Persistence
- The castle owner is stored in the CastleSiegeData row and the registrations in
CastleSiegeGuildRegistration rows, replacing the previous plugin-configuration
JSON blob. Only the current state and when it started still ride on the plugin
configuration, because they have no column in the upstream schema.
Castle NPCs
- The hard-coded gate, catapult, crown and switch coordinates are gone. They are
read from GameConfiguration.CastleSiegeConfiguration, seeded by
CastleSiegeInitializer. Definitions flagged IsPersistedToDatabase are the
breakable defenses and count towards the throne, which additionally brings in
the 4 guardian statues the previous implementation did not spawn.
- The crown hold time now comes from the seeded configuration instead of the
plugin settings.
The AdaMu operational settings (cycle durations, registration fee, designated
server id, auto-open schedule) moved to a renamed CastleSiegeSettings class, so
they no longer collide with upstream's CastleSiegeConfiguration entity.
Verified: full server build succeeds with 0 errors.
Not yet done: the 0xB2 0x00 CastleSiegeState request handler, and the docker /
local run.
The only description of a chat command was its usage string, which is
meant to be read by a human. A user interface which wants to offer the
commands to a player needs the parts separately: the command, what it
does, and one entry per parameter.
Add ChatCommandInfo and ChatCommandParameterInfo, built from the
metadata which is already there - ChatCommandHelpAttribute for the
command and its required character status, ArgumentAttribute for the
short names and whether a parameter is required, ValidValuesAttribute
for the accepted values. Name and description come from the display
attribute, so they are returned in the language of the player.
The description which is passed to ChatCommandHelpAttribute was never
stored anywhere. Keep it as a fallback for commands which have no
display attribute, instead of discarding it.
The reset info command had its texts hard coded in English. It now
refers to resources like every other chat command does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MSpK6jkyF8ZS5nYXyGwYxA
Adds the server side of the chat command list: a view plugin which
sends one ChatCommandInfo per available command, and the handler for
the request of a client which supports it.
Both are limited to the extended protocol, so classic clients don't
receive messages they can't understand. The commands come from
GetAvailableChatCommandInfos, so a player only learns about the ones
he may execute, and deactivated ones stay out.
Executing a command needs nothing new: the client sends the composed
command line as an ordinary chat message, which the chat message
processor already routes to the command without broadcasting it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01MSpK6jkyF8ZS5nYXyGwYxA
(cherry picked from commit f3a9cb132c295863942bb0f06f40cde231e57907)
Money has no owner - unlike a dropped item it can be picked up by anyone
standing on it. Deriving the drop's shares from the killer's party made a
stranger's pick up pay nothing and leave the money on the ground, destroying
zen that was previously taken and split by the picking party.
The recorded shares now apply only when the party picking the money up is the
one which earned it; otherwise there is no experience of theirs to follow and
the money is split equally between the picking party, as it was before and as
share-less money (e.g. from an item box) already is.
Covered by DroppedMoneyTest.StrangerPartySplitsPickedUpMoneyEquallyAsync and
EarningPartyReceivesMoneyByExperienceShareAsync.
(cherry picked from commit ae7cad81da6893cfbd1c28bb29811b7f0a10d540)
AwardExperienceAsync returned 0 for a character at the maximum level which has
not completed the master quest: it gains neither normal nor master experience.
Since the money of a kill is now derived from those returned values, such a
member got no zen at all in a party, while the same character still earns zen
when killing solo - AddExpAfterKillAsync returns the calculated amount there,
regardless of whether it could be applied.
The normal experience is now always calculated and returned, and only awarded
when the character can still gain it.
(cherry picked from commit 7cdb34b7c58f5924427b97ea17f26c3fe0f37719)
The money amount of a monster drop is computed as "gained experience + 7", but
in a party it was neither derived from, nor distributed like, that experience.
DistributeExperienceAfterKillAsync returned a party total without the game rate
and the personal experience rates, while AddExpAfterKillAsync returned a value
which had them applied. Both landed in the same variable in OnDeathAsync, so the
party number was orders of magnitude smaller. That mismatch was worked around by
recalculating the killer's solo experience for money purposes, which pinned the
party pool to a solo-sized amount: with the pool then split by member count, a
party of three received about a third of the solo zen each, while its experience
pool grows with the party size.
AwardExperienceAsync already computes the per member experience with all rates
applied, it just discarded it. It now returns that value, the distribution
returns the per member breakdown, and the money is split proportionally to it.
The workaround is gone, the pool follows the experience, and each member's zen
matches their own level, their own rates and their own master/normal branch. The
units lost to the integer division are handed to the shares which were cut the
most, one each, so no member is systematically favoured over many kills.
Two defects around the money rate are fixed on the way:
- MoneyAmountRate was applied for the killer when the drop was created and again
for the receiver when a party picked it up, so a rate of 3.0 paid a party 9x.
On the "money straight into the inventory" path for a solo killer it was not
applied at all. It is now applied exactly once, for the receiver.
- ClampMoneyOnPickup was only honoured for solo pick ups. A party member at
MaximumInventoryMoney silently lost their share, because the drop was consumed
as soon as any other member could take one. The clamp now runs on the shared
payout path, after the money rate, so it clamps the amount actually credited.
Money without a per player breakdown, such as the fixed amount of an item box,
keeps being split equally. Shares of players who are no longer eligible are
redistributed among the remaining ones instead of being lost.
(cherry picked from commit 53f25aca08e690255571f2bb6c796bb7c10934a2)
Two TvT (Heykel Savasi) event fixes found during mobile+PC client work:
- PeriodicTaskBasePlugIn: a forced start (GM /starths) no longer blocked while a
previous run's TaskDuration window is still open. When an event finished early
(team won / empty / before the 20-min duration) players are warped out but the
periodic state stayed Started with a future NextRunUtc, so the guard returned on
every tick and a re-start only worked after the full duration or a server
restart. Force-finish the stale run when _isStartForced so the same execution
falls through and prepares a fresh one; the NextRunUtc wait now applies to
scheduled runs only.
- HeykelSavasiTeamSelectHandlerPlugIn: IsEncryptionExpected => false. The AdaMu
client sends client-to-server packets unencrypted (like walk / animation /
talk-to-npc), so the server was dropping the team-select (JOIN) packet.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Kills now credited from OnDeathAsync (the AfterKilledPlayerAsync/PK path is skipped when
AllowPlayerKilling is true, so event kills never reached the scoreboard).
- Respawn after death now stays on the mini-game's OWN map instance (older-client path fetched the
shared world map of the same number -> respawned player saw no statues/participants).
- On game end: announce the winner + run a 60s ceremony broadcast loop sending phase-3 HUD countdown
and the final scoreboard until the Lorencia teleport.
- Statue HP overridden to 100k at spawn (MaximumHealthOverride) for the test phase - no DB change.
- Guards made fully stationary by overriding their definition's IntelligenceTypeName to
NullMonsterIntelligence at runtime (no wander, no chase); no DB change.
- Statue/guard in-game names rebranded to 'TvT Statue' / 'TvT Guard' (runtime designation override).
- Bosses: 306/309/357/459 spawn once at (55,42)/(32,42)/(31,59)/(56,58) after 3 total statues broken.
- Guards/bosses spawn in a walkable BOX (retry) instead of a fixed point; several statue-guard
offsets landed on non-walkable terrain and silently failed, leaving statues unbreakable.
- Guard-alive gate uses the ACTUAL spawned count so a statue can always reach 0 -> breakable.
- HeykelSavasiStartPlugIn.OnStartedAsync force-disposes a lingering finished context so GM /starths
restarts the event immediately (no server restart).
- Statue-break + join messages translated to English.
A forced start (e.g. /starths) was blocked for the whole TaskDuration after a
run even though the event had already ended, so re-starting needed a server
restart. ForceStart now skips the 'previous event still running' guard;
scheduled auto-starts still respect it.
SendOpenedNotificationsAsync only looped per full minute, so a <1min entrance
(e.g. a 30s test window) sent no golden 'event opened' notification. Now sends
one up front when TotalMinutes==0. >=1min windows unchanged.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Instead of a fixed lane, each statue (and its 4 guards) now spawns at a random
walkable coordinate anywhere on the map; breaking one reveals the next at a new
random spot, so players must search for it. Falls back to the fixed positions.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- HeykelSavasiContext.IsItemAllowedToEquip disallows wings/capes during
Playing only (so entry is never rejected for wearing wings); auto-unequips
any equipped wing/cape into a free inventory slot in OnGameStartAsync.
- New S2C packet HeykelSavasiHudState (C1, code FB, length 11) carrying
phase, team, counts, statue progress and remaining seconds, plus its view
plugin/interface, broadcast once per second across registration/prep/
battle and one-shot on join/statue-break/game-end.
#1: block all player-vs-player damage while State != Playing (registration/
prep), keep same-team friendly-fire off during battle. Prevents pre-battle
kills (which also caused deaths to route to Lorencia instead of a base).
#3: remove a player from their team on OnObjectRemovedFromMapAsync (real
leave to another map), not on same-map death-respawn, so team counts stay
accurate and re-joining works.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
GetTeamSpawnGate matched gates by exact X1/Y1 anchors, which threw
'Sequence contains no matching element' when the DB gate coords drifted a
tile from the code anchors. Now selects the two spawn gates by Y-order
(red=top, blue=bottom) so it works regardless of exact coordinates.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
- Embed Terrain93.att (server map No 92 -> loads Terrain93); map Number 90->92.
- Real walkable coords from the LoL arena diamond (center lane x=42):
Red base (42,10) top tip, Blue base (42,92) bottom tip; 7 statues per team along x=42.
- Client World93 + Object93 deployed to PC client separately.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A player could destroy their own team's Heykel Savasi statue: the win-condition
attribution (OnDestructibleDied) trusts geometry alone (attacker = Opponent of
defender), and the old friendly-fire guard only covered Player targets, not
Destructible statues. Block same-team damage at the NPC damage intake instead
so only the enemy team can ever land a hit on a statue.
Adds HeykelSavasiContext runtime logic for Task 4.2: per-team statue
break progress/winner tracking (extracted into a pure, testable
StatueProgressState), OnDestructibleDied wiring against the base
MiniGameContext's existing Destructible->Died auto-subscription, and
SpawnStatueAsync which spawns the next statue (561) + 4 guards (580)
via the map initializer using placeholder line-of-statues coordinates.
OnGameStartAsync now spawns each team's first statue after the warp.
