feat: IP-bind NosMall endpoint to active game session#2091
Open
erwan-joly wants to merge 14 commits intomasterfrom
Open
feat: IP-bind NosMall endpoint to active game session#2091erwan-joly wants to merge 14 commits intomasterfrom
erwan-joly wants to merge 14 commits intomasterfrom
Conversation
Adds a second verification gate on top of the existing sas=MD5(sid+pid+user_id+m_szName+salt) check: the caller's X-Forwarded-For must match the IP of the user's live game session, so a leaked sas can't be redeemed from another machine. - AuthHub / AuthCodeService: new RegisterSessionIp / UnregisterSessionIp / GetSessionIp RPCs keyed by account name. - ClientSession: capture Channel.RemoteAddress on authenticate and push it into AuthHub; takes IAuthHub as a new ctor dep. - MallController: after the MD5 gate, resolve the session IP via authHub.GetSessionIpAsync and reject on mismatch. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds per-account session IP management via SignalR hub and AuthCodeService; integrates IP registration/unregistration into ClientSession and WebAPI validation; centralizes monster respawn scheduling into MapInstance and moves skill-cooldown resets to a clock-driven, per-map tick scheduler; adds multiple GM/mod command packets and handlers; updates tests and constructors for new dependencies. Changes
Sequence DiagramsequenceDiagram
participant Client
participant ClientSession
participant AuthHub
participant AuthCodeService
participant NosmallController
Client->>ClientSession: Connect (RemoteAddress: IP)
activate ClientSession
ClientSession->>AuthHub: RegisterSessionIpAsync(accountName, IP)
activate AuthHub
AuthHub->>AuthCodeService: RegisterSessionIp(accountName, IP)
activate AuthCodeService
AuthCodeService-->>AuthHub: ack
deactivate AuthCodeService
deactivate AuthHub
deactivate ClientSession
Client->>NosmallController: Request (X-Forwarded-For: IP)
activate NosmallController
NosmallController->>AuthHub: GetSessionIpAsync(accountName)
activate AuthHub
AuthHub->>AuthCodeService: GetSessionIp(accountName)
activate AuthCodeService
AuthCodeService-->>AuthHub: stored IP
deactivate AuthCodeService
AuthHub-->>NosmallController: stored IP
deactivate AuthHub
NosmallController->>NosmallController: compare header IP vs stored IP
alt match
NosmallController-->>Client: proceed
else mismatch
NosmallController-->>Client: reject (ArgumentException)
end
deactivate NosmallController
Client->>ClientSession: Disconnect
activate ClientSession
ClientSession->>AuthHub: UnregisterSessionIpAsync(accountName)
activate AuthHub
AuthHub->>AuthCodeService: UnregisterSessionIp(accountName)
activate AuthCodeService
AuthCodeService-->>AuthHub: ack
deactivate AuthCodeService
deactivate AuthHub
deactivate ClientSession
sequenceDiagram
participant MonsterRespawnHandler
participant Clock
participant MapInstance
participant MapTickLoop
participant BattleService
participant SessionRegistry
MonsterRespawnHandler->>Clock: compute respawnAt = now + respawnMs
activate Clock
Clock-->>MonsterRespawnHandler: respawnAt
deactivate Clock
MonsterRespawnHandler->>MapInstance: ScheduleRespawn(monster, respawnAt)
activate MapInstance
MapInstance-->>MapInstance: enqueue respawn
deactivate MapInstance
MapTickLoop->>MapInstance: life-loop tick
activate MapInstance
MapInstance->>BattleService: TickCooldownResetsAsync(this)
activate BattleService
BattleService->>SessionRegistry: enumerate sessions on map
SessionRegistry-->>BattleService: sessions
BattleService-->>BattleService: send SkillResetPacket for due entries
deactivate BattleService
MapInstance->>MapInstance: SweepPendingRespawnsAsync()
activate MapInstance
MapInstance->>MapInstance: reset monster HP/MP/alive/hitlist/pos
MapInstance->>SessionRegistry: broadcast Monster.GenerateIn()
deactivate MapInstance
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/NosCore.GameObject/Networking/ClientSession/ClientSession.cs`:
- Around line 121-125: The code is registering Channel.RemoteAddress without
normalizing so stored values like "127.0.0.1:8080" will fail IPAddress.TryParse
downstream; in ClientSession.cs before calling
authHub.RegisterSessionIpAsync(accountDto.Name, remoteAddress) extract or
normalize the IP-only component (e.g. parse Channel.RemoteAddress as an
EndPoint/IPEndPoint or split off the port and use the Address portion or use
Uri/IPAddress parsing) and pass that plain IP string to RegisterSessionIpAsync;
keep the call fire-and-forget but ensure the stored value is IP-only (mirroring
the way UnregisterSessionIpAsync is handled) so NosmallController.cs
IPAddress.TryParse(expected, ...) succeeds.
In `@src/NosCore.WebApi/Controllers/NosmallController.cs`:
- Around line 30-38: The code reads Request.Headers["X-Forwarded-For"] directly
and throws ArgumentException on mismatch; instead configure ASP.NET's forwarded
headers middleware in WebApiBootstrap by calling UseForwardedHeaders with a
properly configured ForwardedHeadersOptions and trusted proxies/networks, then
in NosmallController.cs rely on the framework-populated connection info (e.g.,
Request.HttpContext.Connection.RemoteIpAddress) rather than parsing
X-Forwarded-For manually; also replace the throw of
ArgumentException("X-Forwarded-For") with an appropriate 4xx response (e.g.,
return Unauthorized/Forbid or BadRequest) in the method that calls
authHub.GetSessionIpAsync so authentication failures produce correct client
error codes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d3ead61b-d066-4acb-84d2-8da3b038392a
📒 Files selected for processing (8)
src/NosCore.GameObject/InterChannelCommunication/Hubs/AuthHub/AuthHub.cssrc/NosCore.GameObject/InterChannelCommunication/Hubs/AuthHub/AuthHubClient.cssrc/NosCore.GameObject/InterChannelCommunication/Hubs/AuthHub/IAuthHub.cssrc/NosCore.GameObject/Networking/ClientSession/ClientSession.cssrc/NosCore.GameObject/Services/AuthService/AuthCodeService.cssrc/NosCore.GameObject/Services/AuthService/IAuthCodeService.cssrc/NosCore.WebApi/Controllers/NosmallController.cstest/NosCore.Tests.Shared/TestHelpers.cs
Comment on lines
+121
to
+125
| var remoteAddress = Channel.RemoteAddress; | ||
| if (remoteAddress != null) | ||
| { | ||
| _ = authHub.RegisterSessionIpAsync(accountDto.Name, remoteAddress); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Verify RemoteAddress contract and concrete formatting in channel implementations.
rg -n --type=cs -C3 '\bRemoteAddress\b'
# Verify authentication flow call sites for InitializeAccount.
rg -n --type=cs -C3 '\bInitializeAccount\s*\('Repository: NosCoreIO/NosCore
Length of output: 3955
🏁 Script executed:
# Find IChannel interface to understand RemoteAddress contract
rg -n --type=cs -B5 -A10 'interface IChannel'Repository: NosCoreIO/NosCore
Length of output: 4778
🏁 Script executed:
# Find IAuthHub interface to understand RegisterSessionIpAsync contract
rg -n --type=cs -B2 -A5 'RegisterSessionIpAsync'Repository: NosCoreIO/NosCore
Length of output: 3896
🏁 Script executed:
# Check OnDisconnectedAsync implementation for IP unregistration
rg -n --type=cs -B5 -A15 'OnDisconnectedAsync'Repository: NosCoreIO/NosCore
Length of output: 7798
🏁 Script executed:
# Check if RemoteAddress is normalized or parsed anywhere
rg -n --type=cs 'RemoteAddress' -A5Repository: NosCoreIO/NosCore
Length of output: 715
🏁 Script executed:
# Get full OnDisconnectedAsync method from ClientSession.cs
sed -n '164,250p' src/NosCore.GameObject/Networking/ClientSession/ClientSession.csRepository: NosCoreIO/NosCore
Length of output: 2606
🏁 Script executed:
# Find IChannel interface definition
rg -n --type=cs 'interface IChannel' -A 30Repository: NosCoreIO/NosCore
Length of output: 3434
🏁 Script executed:
# Search for UnregisterSessionIpAsync calls
rg -n --type=cs 'UnregisterSessionIpAsync' -B 2 -A 3Repository: NosCoreIO/NosCore
Length of output: 3264
🏁 Script executed:
# Find authCodeService.RegisterSessionIp implementation to understand the sync operation
rg -n --type=cs 'RegisterSessionIp\(' -B 2 -A 8Repository: NosCoreIO/NosCore
Length of output: 3026
🏁 Script executed:
# Search for IChannel interface more broadly
rg -n --type=cs 'interface IChannel[^A-Za-z]' -A 20Repository: NosCoreIO/NosCore
Length of output: 43
🏁 Script executed:
# Search for Channel class and RemoteAddress property definition
rg -n --type=cs 'RemoteAddress' -B 5 -A 2 | head -80Repository: NosCoreIO/NosCore
Length of output: 889
🏁 Script executed:
# Check DotNetty or other channel implementations for RemoteAddress
fd -e cs -x rg -l 'RemoteAddress' {} \; | head -20Repository: NosCoreIO/NosCore
Length of output: 128
🏁 Script executed:
# Find the Channel property declaration in ClientSession
rg -n --type=cs 'public.*Channel\s*{|private.*Channel\s*{' src/NosCore.GameObject/Networking/ClientSession/ClientSession.cs -A 3Repository: NosCoreIO/NosCore
Length of output: 203
🏁 Script executed:
# Look at the full ClientSession class header and constructor to understand Channel type
head -120 src/NosCore.GameObject/Networking/ClientSession/ClientSession.cs | tail -60Repository: NosCoreIO/NosCore
Length of output: 2556
🏁 Script executed:
# Search for DotNetty IChannel usage
rg -n --type=cs 'DotNetty' | head -20Repository: NosCoreIO/NosCore
Length of output: 43
🏁 Script executed:
# Search for what's imported/used for Channel in the file
head -50 src/NosCore.GameObject/Networking/ClientSession/ClientSession.csRepository: NosCoreIO/NosCore
Length of output: 1933
🏁 Script executed:
# Find NetworkClient class definition
rg -n --type=cs 'class NetworkClient' -A 30Repository: NosCoreIO/NosCore
Length of output: 43
🏁 Script executed:
# Look for Channel property in NetworkClient or related files
rg -n --type=cs 'IChannel|class.*Channel' src/NosCore.Networking/ -A 5 | head -80Repository: NosCoreIO/NosCore
Length of output: 125
🏁 Script executed:
# Search for NetworkClient class in entire codebase
rg -n --type=cs 'class NetworkClient'Repository: NosCoreIO/NosCore
Length of output: 43
🏁 Script executed:
# Find all references to NosCore.Networking namespace
rg -n 'namespace NosCore.Networking'Repository: NosCoreIO/NosCore
Length of output: 43
🏁 Script executed:
# Look for Channel property definition
rg -n --type=cs 'public.*Channel\s+\{' -A 2Repository: NosCoreIO/NosCore
Length of output: 43
🏁 Script executed:
# Check if Channel is an auto-property or field in ClientSession
rg -n --type=cs '\bChannel\b' src/NosCore.GameObject/Networking/ClientSession/ClientSession.csRepository: NosCoreIO/NosCore
Length of output: 505
🏁 Script executed:
# Check for base class definitions
rg -n --type=cs ': NetworkClient'Repository: NosCoreIO/NosCore
Length of output: 208
🏁 Script executed:
# Look for DotNetty or other networking libraries in project files
find . -name "*.csproj" -o -name "*.sln" | head -5Repository: NosCoreIO/NosCore
Length of output: 251
🏁 Script executed:
# Check dependencies
rg -n 'DotNetty|IChannel' | head -20Repository: NosCoreIO/NosCore
Length of output: 7045
🏁 Script executed:
# Find NetworkClient in NosCore.Core
rg -n --type=cs 'class NetworkClient' src/NosCore.Core/Repository: NosCoreIO/NosCore
Length of output: 43
🏁 Script executed:
# Check for NosCore.Core imports and structure
find src/NosCore.Core -name "*.cs" | grep -i network | head -10Repository: NosCoreIO/NosCore
Length of output: 43
🏁 Script executed:
# Look for uses of the IP address from RegisterSessionIp
rg -n --type=cs 'GetSessionIp|_sessionIps' -B 2 -A 5Repository: NosCoreIO/NosCore
Length of output: 6784
🏁 Script executed:
# Check if any downstream code parses or validates the IP address
rg -n --type=cs 'IPAddress|Parse.*Ip|Ip.*Parse' -B 2 -A 3 | head -50Repository: NosCoreIO/NosCore
Length of output: 954
Address potential IP parsing failure in downstream Nosmal validation.
Line 124 registers Channel.RemoteAddress as a fire-and-forget call. More critically, NosmallController.cs (line 33–34) calls IPAddress.TryParse(expected, ...) on the stored IP. If Channel.RemoteAddress includes endpoint formatting (e.g., "127.0.0.1:8080" from DotNetty), the parse will fail silently, causing legitimate Mall requests to be rejected even though the session is authenticated.
Ensure remoteAddress is normalized to a plain IP string (without port) before passing to RegisterSessionIpAsync, or extract the address component when storing. The disconnect path correctly uses error handling when calling UnregisterSessionIpAsync.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/NosCore.GameObject/Networking/ClientSession/ClientSession.cs` around
lines 121 - 125, The code is registering Channel.RemoteAddress without
normalizing so stored values like "127.0.0.1:8080" will fail IPAddress.TryParse
downstream; in ClientSession.cs before calling
authHub.RegisterSessionIpAsync(accountDto.Name, remoteAddress) extract or
normalize the IP-only component (e.g. parse Channel.RemoteAddress as an
EndPoint/IPEndPoint or split off the port and use the Address portion or use
Uri/IPAddress parsing) and pass that plain IP string to RegisterSessionIpAsync;
keep the call fire-and-forget but ensure the stored value is IP-only (mirroring
the way UnregisterSessionIpAsync is handled) so NosmallController.cs
IPAddress.TryParse(expected, ...) succeeds.
Three MMORPG-hot paths were holding one sleeping Task per event for seconds (or longer), which multiplies per-player and per-monster. All three can be driven from the map's existing 400ms life tick instead. - WorldPacketHandlingStrategy: drop the `Task.WhenAll(handler, Task.Delay(200))` wrapper. The per-session packet lock already serializes a single client's packets; the 200ms floor was a NosCore-only artifact (vanosilla/OpenNos have nothing like it) that only served to hold a timer task per packet. - MonsterRespawnHandler: swap the fire-and-forget `Task.Delay(RespawnTime)` for `MapInstance.ScheduleRespawn(monster, respawnAt)`. The map life loop sweeps the pending-respawn table on each 400ms tick and revives whichever monsters have hit their ReadyAt. - BattleService.ScheduleCooldownReset: same idea for skill-cooldown SkillResetPacket emission. BattleService now owns a `(CharacterVisualId -> CastId -> ReadyAt)` registry. MapInstance calls `IBattleService.TickCooldownResetsAsync(this)` on each tick to drain ready entries and send the packet through the character's normal outbound path. No behavior change from the player's perspective — cooldown and respawn timers still fire with ≤400ms of latency, which is under the game's client-tick granularity. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Three missing cosmetic GM commands that parallel the existing item-use handlers (ChangeGenderHandler, HairDieHandler) — same refresh sequence (GenerateEq + GenerateIn + GenerateCMode/GenerateEff for gender), same enum validation (Enum.IsDefined guard), no bypass of sanity checks beyond what a GM command is entitled to. - $ChangeGender: flips the character's gender and plays the standard visual-refresh + effect 196 swap that the in-game gender card uses. - $SetHairStyle Style: sets HairStyleType; ignores out-of-range values. - $SetHairColor Color: sets HairColorType; same guard. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/NosCore.PacketHandlers/Command/SetHairStylePacketHandler.cs`:
- Around line 23-30: The handler currently only checks Enum.IsDefined for
HairStyleType, but player-supported styles are limited up to
HairStyleType.HairStyleB; update SetHairStylePacketHandler to reject styles
greater than HairStyleType.HairStyleB (in addition to the Enum.IsDefined check)
before assigning character.HairStyle so it matches the player validation logic
in PlayerBundleExtensions; specifically, add a guard that returns if
(HairStyleType)packet.Style > HairStyleType.HairStyleB (or packet.Style >
(int)HairStyleType.HairStyleB).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: dcbebb7d-7e8a-4946-85c8-29a00c756031
📒 Files selected for processing (6)
src/NosCore.Data/CommandPackets/ChangeGenderPacket.cssrc/NosCore.Data/CommandPackets/SetHairColorPacket.cssrc/NosCore.Data/CommandPackets/SetHairStylePacket.cssrc/NosCore.PacketHandlers/Command/ChangeGenderPacketHandler.cssrc/NosCore.PacketHandlers/Command/SetHairColorPacketHandler.cssrc/NosCore.PacketHandlers/Command/SetHairStylePacketHandler.cs
Three more GM commands that reuse existing building blocks: - $ShoutHere Message — Moderator — broadcasts a yellow chat line to the caller's current map only. Complements the server-wide $Shout. - $Kill — GameMaster — sets the caller's HP to 0 and publishes an EntityDiedEvent so PlayerRevivalHandler runs its normal death + respawn flow. Self-target only for now; cross-target kill would need the cross-channel StatData pattern like $SetLevel / $SetGold. - $SetSpPoint Value — GameMaster — clamps to WorldConfiguration bounds, writes character.SpPoint, and refreshes the client via GenerateSpPoint. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
src/NosCore.PacketHandlers/Command/SetSpPointPacketHandler.cs (1)
23-27: Harden clamp against invalid negativeMaxSpPointsconfig.Current logic assumes
worldConfiguration.Value.MaxSpPoints >= 0. If config is negative, a negative SP value can still be assigned.🔧 Proposed fix
public override Task ExecuteAsync(SetSpPointPacket packet, ClientSession session) { - var clamped = packet.SpPoint < 0 ? 0 - : packet.SpPoint > worldConfiguration.Value.MaxSpPoints ? worldConfiguration.Value.MaxSpPoints + var maxSpPoints = worldConfiguration.Value.MaxSpPoints < 0 ? 0 : worldConfiguration.Value.MaxSpPoints; + var clamped = packet.SpPoint < 0 ? 0 + : packet.SpPoint > maxSpPoints ? maxSpPoints : packet.SpPoint; session.Character.SpPoint = clamped; return session.SendPacketAsync(session.Character.GenerateSpPoint(worldConfiguration)); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NosCore.PacketHandlers/Command/SetSpPointPacketHandler.cs` around lines 23 - 27, The clamp currently assumes worldConfiguration.Value.MaxSpPoints is non‑negative so a negative MaxSpPoints could allow negative SP; in SetSpPointPacketHandler (look for packet.SpPoint, worldConfiguration.Value.MaxSpPoints, session.Character.SpPoint, session.SendPacketAsync and GenerateSpPoint) compute an effectiveMax = Math.Max(0, worldConfiguration.Value.MaxSpPoints) and then clamp packet.SpPoint into [0, effectiveMax] before assigning to session.Character.SpPoint and calling session.SendPacketAsync(session.Character.GenerateSpPoint(...)).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/NosCore.PacketHandlers/Command/KillCommandPacketHandler.cs`:
- Around line 18-22: The ExecuteAsync in KillCommandPacketHandler
unconditionally sets session.Character.Hp to 0 and publishes EntityDiedEvent;
change it to first check whether the character is already dead (e.g.,
session.Character.Hp > 0 or session.Character.IsDead == false) and only set Hp
to 0 and call await messageBus.PublishAsync(new
EntityDiedEvent(...)).ConfigureAwait(false) when the character was alive,
thereby preventing duplicate EntityDiedEvent publications for already-dead
characters.
In `@src/NosCore.PacketHandlers/Command/ShoutHerePacketHandler.cs`:
- Around line 21-27: The handler currently uses
string.IsNullOrEmpty(packet.Message) which treats whitespace-only input as
valid; change the check to string.IsNullOrWhiteSpace(packet.Message) so messages
that are null, empty, or only whitespace are routed to
session.SendPacketAsync(session.Character.GenerateSay(packet.Help(),
SayColorType.Yellow)) instead of being broadcast via
session.Character.MapInstance.SendPacketAsync(session.Character.GenerateSay(packet.Message,
SayColorType.Yellow)); update the condition in the ShoutHerePacketHandler to use
IsNullOrWhiteSpace and leave the rest of the logic intact.
---
Nitpick comments:
In `@src/NosCore.PacketHandlers/Command/SetSpPointPacketHandler.cs`:
- Around line 23-27: The clamp currently assumes
worldConfiguration.Value.MaxSpPoints is non‑negative so a negative MaxSpPoints
could allow negative SP; in SetSpPointPacketHandler (look for packet.SpPoint,
worldConfiguration.Value.MaxSpPoints, session.Character.SpPoint,
session.SendPacketAsync and GenerateSpPoint) compute an effectiveMax =
Math.Max(0, worldConfiguration.Value.MaxSpPoints) and then clamp packet.SpPoint
into [0, effectiveMax] before assigning to session.Character.SpPoint and calling
session.SendPacketAsync(session.Character.GenerateSpPoint(...)).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 3c2f82ef-4480-489d-8b81-63ff0e6f9bf6
📒 Files selected for processing (6)
src/NosCore.Data/CommandPackets/KillPacket.cssrc/NosCore.Data/CommandPackets/SetSpPointPacket.cssrc/NosCore.Data/CommandPackets/ShoutHerePacket.cssrc/NosCore.PacketHandlers/Command/KillCommandPacketHandler.cssrc/NosCore.PacketHandlers/Command/SetSpPointPacketHandler.cssrc/NosCore.PacketHandlers/Command/ShoutHerePacketHandler.cs
✅ Files skipped from review due to trivial changes (1)
- src/NosCore.Data/CommandPackets/KillPacket.cs
…eroXp Four more single-field GM commands that only wire existing ICharacterEntity writable properties to a command packet. No new infrastructure. - $SetBankGold Amount — writes Character.BankGold. Bank UI refreshes on next open; no separate refresh packet needed. - $SetSpAdditionPoint Value — clamps to WorldConfiguration. MaxAdditionalSpPoints and refreshes via GenerateSpPoint, same shape as the just-added $SetSpPoint. - $SetJobLevelXp Amount — writes Character.JobLevelXp and emits GenerateLev so the XP bars sync immediately. - $SetHeroXp Amount — writes Character.HeroXp; same GenerateLev refresh. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
erwan-joly
force-pushed
the
feature-handler-fills
branch
2 times, most recently
from
d83b19e to
164cd20
Compare
Capture is a skill-side mechanic, not an item effect. Matches the
vanosilla + OpenNos implementation:
- Skill carries a `BCardType.Capture` bcard with subtype
`AdditionalTypes.Capture.CaptureAnimal` (same enum codes as both
reference servers).
- BattleService.Hit detects the bcard before damage resolution and
delegates to ICaptureService, skipping the normal damage pipeline
(no SuPacket, no stat broadcast, no EntityDamagedEvent) — matches
vanosilla BattleSystem's `if (!skill.BCards.Any(... Capture ...))`
guards around the SuPacket sends.
- Cooldown still fires so a failed roll consumes the gate.
CaptureService rules (cross-referenced with both servers):
- Caster must be a player, target must be a live monster.
- monster.Level <= player.Level.
- monster.HP% < 50 (matches vanosilla BCardCaptureHandler gate).
- Rate: 90 % if player < level 20, else 50 % (vanosilla constants).
- Mate level = max(monster.Level - 15, 1) (vanosilla offset).
- On success: insert MateDto via the auto-generated IDao<MateDto,long>,
mark monster !IsAlive + Hp = 0, broadcast `out` to despawn the sprite.
Followups (not in this PR): dignity + MaxMateCount + RaidInstance
guards, SuCapturePacket (vanosilla's dedicated capture-su), in-game
mate list refresh post-entry (sc_n / p_clear / pinit).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
erwan-joly
force-pushed
the
feature-handler-fills
branch
from
164cd20 to
4e514cb
Compare
…HSA-mr8r-92fq-pj8p) Two moderate-severity advisories surfaced by NuGet audit against the 1.15.x line we shipped in #2085: - OpenTelemetry.Api 1.15.2 -> GHSA-g94r-2vxg-569j - OpenTelemetry.Exporter.OpenTelemetryProtocol 1.15.2 -> GHSA-mr8r-92fq-pj8p Bumps: - OpenTelemetry.Exporter.OpenTelemetryProtocol 1.15.2 -> 1.15.3 - OpenTelemetry.Extensions.Hosting 1.15.2 -> 1.15.3 - OpenTelemetry.Instrumentation.AspNetCore 1.15.1 -> 1.15.2 - OpenTelemetry.Instrumentation.EntityFrameworkCore 1.15.0-beta.1 -> 1.15.1-beta.1 - OpenTelemetry.Instrumentation.Http 1.15.0 -> 1.15.1 - OpenTelemetry.Instrumentation.Runtime 1.15.0 -> 1.15.1 Build clean with 0 vulnerability warnings. Full test suite green. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Aligns the Postgres enum with ItemEffectType.MateCapture introduced during the pet-capture work. Current capture path is skill-based (BCardType.Capture / CaptureAnimal) but the enum value is kept so the label is available if we later wire item-based capture cards the way vanilla NosTale does. Runs a single AlterDatabase with the new enum label appended; no column changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Picks up the CMapPacket.MapType → IsEntering rename; updates the one call site in MapInstance.GenerateCMap accordingly. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Return early when the character is already dead so the command doesn't emit a second EntityDiedEvent and fire death side-effects twice. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Using IsNullOrWhiteSpace routes " " to the help message instead of broadcasting a blank shout to the whole map. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…s vanosilla Unset, the string property serializes to "-", producing "NsTeST - 0 …" instead of the expected "NsTeST 0 …" (literal double space) that vanosilla's LoginPacketsExtensions emits. Clients that tolerate the deserialized form still misparse the serialized-from-NosCore form. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
LeadingBlank now defaults to empty string in NsTestPacket (Packets 20.0.1); drop the redundant explicit assignment. Also picks up the AscrPacket leading-blank addition (20.0.2). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
NosCore was seeding character skills purely from WorldConfiguration.BasicSkills[class][origin]. Every character must own the universal Adventurer set — primary (200), secondary (201) and Pet Catcher (209) — but nothing enforced that: a character created from a config where those were missing ended up with no CharacterSkill row for CastId 16, so `u_s 16 3 <monster>` fell through SkillResolver.Resolve → null and BattleService replied with `cancel 2 <target> 0`, which is exactly what we hit live. Match vanosilla (Plugins.PacketHandling/Customization/BaseSkill.cs hard-codes the same three) by always inserting 200 / 201 / 209 in CharNewPacketHandler, de-duped against whatever the config-driven BasicSkills list contributes. The config-side entries that used to carry these three are dropped from world.yml since they're now guaranteed by the handler. Class-specific capture variants (235 Swordsman, 237 Mage) are still picked up by SkillService.LearnClassSkillsAsync on JobLevelUp since they're class-bound in the skill DB, not universal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a second verification gate on top of the existing
sas = MD5(sid+pid+user_id+m_szName+salt)check on the Mall entrypoint: the caller'sX-Forwarded-Formust match the IP of the user's live game session, so a leakedsastoken can't be redeemed from another machine.What changed
RegisterSessionIp/UnregisterSessionIp/GetSessionIpRPCs keyed by account name, living on MasterServer.Channel.RemoteAddressand push it into AuthHub; on disconnect, unregister it. TakesIAuthHubas a new constructor dep.authHub.GetSessionIpAsync(user_id)and reject on mismatch (or missing/malformedX-Forwarded-For).Test plan
dotnet build— green.dotnet test— 924/924 passing.Generated with Claude Code
Summary by CodeRabbit
New Features
Improvements