Raklion: fall and push of Selupan, skill multipliers, and the closed hatchery gate

[apraxico]

Follow-up of #977.

Summary

@ze-dom confirmed in #977 that Selupan hits players when it falls from the sky at the start of the battle (video). Until now, the fall was only the animation of its appearance: it dealt no damage and pushed nobody.

This PR adds:

• the damage and the push of the fall;
• the push of the frost shock;
• the per-class limit of the push.

It also fixes two things:

• the gate to the hatchery let players enter while the hatchery was closed;
• the damage multipliers of the Selupan skills from Add the Raklion event (Battle of Selupan) #977 threw an exception on the first use of each skill, and were missing afterwards.

Behaviour of the original game

The known server sources (IGCN, MuEmu, kessiler) agree on this behaviour:

• The fall (FirstSkill) is used exactly once, at the first step of the battle. It uses skill 253 with the monster skill unit 37 ("Selupan - Fall"). The unit hits every player closer to Selupan than 4 fields (ScopeType 0, ScopeValue 4), measured from Selupan, not from its target. The damage multiplier is 2.5, as @ze-dom pointed out.
• The push (skill element type 9, gObjBackSpring2) moves each hit player away from Selupan, field by field. The fall pushes by 4 fields (element 51), the frost shock by 10 (element 50). The push stops at fields which aren't walkable, including the safezone. Stunned and frozen players aren't pushed.
• Only dark knights, magic gladiators and dark lords are pushed by the full distance. The other classes are pushed by 2 fields at most. All three sources have this in gObjBackSpring2.
• The hatchery gate doesn't move the player while the hatchery is closed. The player stays where it is and gets the message "The Hatchery Gate is closed, you cannot enter."

Two details are intentionally left out:

• In the sources, the damage of the fall goes to the main target once per player in range, while the push goes to each player in range. That looks like a bug. Here, each player in range is hit once.
• The original push bounces back from a blocked field. Here, it just stops, like the existing push of Earthshake.

Changes

• SelupanIntelligence:
  • When Selupan appears, it hits all valid targets closer to it than FallRadius. It still shows the fall animation as before. The damage uses the new skill Selupan Fall (253), so its multiplier applies, like the other attack skills of Selupan.
  • The fall and the ice strike (frost shock) push the hit players away (PushAwayAsync). The push works like the one of Earthshake: it uses Player.MoveAsync, so the clients are updated.
• RaklionEventDefinition, new settings:
  • FallRadius (4);
  • FallPushDistance (4);
  • IceStrikePushDistance (10);
  • LimitedPushDistance (2);
  • FullyPushedCharacterClassNumbers (the dark knight, magic gladiator and dark lord classes);
  • GetPushDistance(...), which applies the per-class limit.
• WarpGateAction: before the warp, it calls the new plugin point IWarpGateEnteringPlugIn, which can deny the entrance by cancelling it. RaklionPlugIn implements it: RaklionContext.CanEnterThroughGateAsync denies the hatchery while it's closed and shows the message. So WarpGateAction doesn't know about any event, and other events with a closed map can use the same plugin point. Like the original game (MuEmu's ERROR_JUMP), a denied player is warped to its current position. A "map change failed" isn't enough, because the game client keeps showing its loading screen after it entered a gate until a map change is completed. Before, the player entered the closed hatchery, got the message and was moved to the start of Raklion by the next tick.
• MonsterAttributeHolder.GetOrCreateAttribute: this threw a NotImplementedException. When Selupan attacked with a skill, EnsureSkillAttributes failed while adding the relationship of the damage multiplier, because the relationship needs the SkillMultiplier of the monster. The skill attributes were already assigned to the SkillEntry at that point, so every later attack used them without the multiplier. The monster now returns its composed attribute if elements were added to it, and a constant element of its stat value otherwise.
• Data:
  • SkillNumber.SelupanFall = 253;
  • the skill with its multiplier (2.5, InputOperator.Maximum like the other three) for new databases;
  • AddSelupanFallSkillUpdatePlugIn for existing ones. It reuses AddRaklionEventUpdatePlugIn.CreateSkill, and applying it twice changes nothing.
• Static code analysis: addresses the remaining Codacy findings of the Selupan skills from Add the Raklion event (Battle of Selupan) #977. This was committed after the merge.

Update version: this uses UpdateVersion 120, because 119 is used by the Imperial Guardian event in #992. If this is merged first, I'll renumber it.

Tests

• SelupanIntelligenceTest:
  • the damage multiplier of a skill is calculated with the attributes of the monster (this failed with the NotImplementedException before);
  • only the players closer than the fall radius are hit; a player exactly at the radius or further away isn't;
  • a hit player is pushed away by the push distance;
  • the push stops in front of a field which isn't walkable.
• RaklionEventDefinitionTest: the push distance of each character class.
• RaklionHatcheryGateTest: the closed hatchery can't be entered through a gate, other maps can, and the plugin denies nothing while the event isn't running.
• RaklionDataTest now also covers the fall skill and its multiplier, for a new database and for the updates applied twice.

Built with -p:ci=true; all four test suites pass. Tested in game: the fall hits and pushes the players around Selupan. The exception of the multiplier was found in the server log of that test.

🤖 Generated with Claude Code

[sven-n]

Review

I read the whole diff and checked how it fits with the existing code. I also built it and ran the tests locally. The code looks good, and I have no blocking findings.

Verified

• The full solution builds with no errors, and the branch merges cleanly into current master. MUnique.OpenMU.Tests (1077), Network.Packets.Tests (607) and Persistence.Initialization.Tests (32, 6 skipped) pass, including both RaklionDataTest cases with the fall skill.
• MonsterAttributeHolder.GetOrCreateAttribute: good catch. #977 missed this: EnsureSkillAttributes assigns skillEntry.Attributes before it adds the relationships. So after the NotImplementedException, the cached entry had no multiplier for the rest of the battle. The fix resolves it for any monster that attacks with a skill that has relationships.
• Push classes: {4, 6, 7, 12, 13, 16, 17} matches CharacterClassNumber (DK/BK/BM, MG/DM, DL/LE).
• UpdateVersion 120 with 119 unused: fine. DataUpdateService finds pending updates as "not installed" (a set difference), not "greater than the highest installed version". So #992 can still use 119 whenever it lands, and databases already at 120 will still get it.
• Denied gate: PlayerMapTransitions.WarpToAsync does a full map change even on the same map, so the client's loading screen ends as described.
• PushAwayAsync: at map edges CalculateTargetPoint wraps the byte, but the index stays in range and those fields aren't walkable, so the push just stops.

Non-blocking suggestions

1. WarpGateAction now depends on RaklionPlugIn directly. It works, but it's a generic player action that now knows about one event. The next event with a closed gate (e.g. Imperial Guardian, Kanturu) would add another if here. A small plug-in point would keep WarpGateAction independent, e.g. an IWarpGateEnterRestrictionPlugIn with ValueTask<bool> CanEnterAsync(Player, ExitGate), which RaklionPlugIn implements. That can be a follow-up.
2. GetOrCreateAttribute returns a snapshot for monster attributes that nothing has changed yet. A relationship created from that ConstantElement won't see elements added to that attribute later, because AddElement then creates a new ComposableAttribute. It's harmless for SkillMultiplier today, since nothing changes it at runtime, but the <remarks> should say so, so nobody relies on it for dynamic stats.
3. The only change in RaklionBoss.cs is an added blank line. It's harmless, but it could be dropped to keep the diff focused.

CI: Codacy passes. Azure again "completed" the same second it started, which is the known infrastructure issue.

This looks ready to merge from my side.

---

Generated by Claude Code

[sven-n]

Please fix at least the first suggestion.

[apraxico]

Thanks for the review! Addressed in 888475c:

1. WarpGateAction no longer knows about Raklion. There's a new plugin point, IWarpGateEnteringPlugIn, and RaklionPlugIn implements it. I used ValueTask WarpGateEnteringAsync(Player, ExitGate, CancelEventArgs) instead of ValueTask<bool> CanEnterAsync(...), because the generated plugin point proxies only support void/ValueTask methods. With CancelEventArgs, the proxy stops at the first plugin which denies the entrance, like IItemMovingPlugIn and IItemDropPlugIn. When a plugin denies it, WarpGateAction warps the player to its current position, as before. I added a test that the plugin denies nothing while the event isn't running.
2. Snapshot in GetOrCreateAttribute: the <remarks> now say that the constant is a snapshot. A relationship created from it won't see elements that are added later, so it shouldn't be used for stats which change at runtime.
3. Blank line in RaklionBoss.cs: I kept it. It fixes the SA1515 finding of Codacy: a single-line comment must be preceded by a blank line. It's part of the cleanup of the remaining Add the Raklion event (Battle of Selupan) #977 findings, so removing it would bring the finding back.

MUnique.OpenMU.Tests passes (1078). I also updated the PR description.

🤖 Generated with Claude Code