From a0a4812b7f1ac4aa2c0b8d35295c186c7da308f4 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 02:12:58 +0400 Subject: [PATCH 01/15] feat(mate): read back the mates the capture service has been writing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit WHAT: a Mate game object, a MateService that loads a character's mates, a PlayerMatesComponent on the player bundle, loading at character select, and the pet-list burst at game start. WHY: CaptureService writes a MateDto row every time a pet is caught, and nothing ever read it. The row went into the database and the pet was never heard from again — the catch worked and produced nothing a player could see. SOURCE: a real packet capture. It settles three things that would otherwise have been guesses: * the login order — p_clear, one sc_p per pet and sc_n per partner, then sc_p_stc; * that pets and partners are numbered from zero SEPARATELY (sc_p slots 0..7 sit next to sc_n slots 0..1 in the same burst); * the experience table. The curve inherited here was twenty times the pet requirement and five times the partner one — ten observations from level 1 to level 88 match after dividing, two of them eight-digit numbers, so it is not curve-fitting. MateXpTable carries the divisors and all ten observations are tests. Worth a look on its own: nothing throws when this is wrong, the pet just never levels, and it reads as grind rather than as a bug. EXPECTED: a caught pet survives the session it was caught in. Log in and the pet window lists what you own, with its level, loyalty and experience bar. OUT OF SCOPE, each for a reason written next to it in the code: * the per-level HP/MP and damage curves. The inherited ones reproduce no captured row — a level-3 chicken would get 262 HP where the server sent 195 — so the mate reports the creature's declared statistics: exact at level 1, low above it. Sixteen samples across three stat families are not enough to derive the right curve; fitting one would be inventing it. * pinit for mates. The capture gives a mate row eight fields; PinitSubPacket serialises eleven, because its last three members are non-nullable value types. Sending a shape an authoritative source contradicts is worse than not sending it — happy to send a NosCore.Packets change for this if you want it. * empty partner equipment slots. The capture writes them as a bare -1; leaving the sub-packet null makes the serialiser drop the separating space and emit "1536-1-1-1". All three numbers are filled instead (-1.0.0), which keeps the field count right. Unreachable today, since nothing creates a partner yet. * summoning onto the map, combat, loyalty, feeding, the partner's specialist card. Each is its own slice. Co-Authored-By: Claude Opus 5 --- .../Ecs/Components/PlayerMatesComponent.cs | 22 ++ src/NosCore.GameObject/Ecs/MapWorld.cs | 5 +- .../Ecs/PlayerComponentBundle.cs | 3 +- .../Messaging/WolverineDependencyRegistrar.cs | 5 + .../MapChangeService/MapChangeService.cs | 6 +- .../Services/MateService/IMateService.cs | 26 +++ .../Services/MateService/Mate.cs | 201 ++++++++++++++++++ .../Services/MateService/MateService.cs | 87 ++++++++ .../Services/MateService/MateXpTable.cs | 111 ++++++++++ .../CharacterScreen/SelectPacketHandler.cs | 15 +- .../Game/GameStartPacketHandler.cs | 22 +- .../Services/MateService/MateServiceTests.cs | 181 ++++++++++++++++ .../Services/MateService/MateXpTableTests.cs | 87 ++++++++ .../SelectPacketHandlerTests.cs | 3 +- test/NosCore.Tests.Shared/TestHelpers.cs | 10 +- 15 files changed, 773 insertions(+), 11 deletions(-) create mode 100644 src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs create mode 100644 src/NosCore.GameObject/Services/MateService/IMateService.cs create mode 100644 src/NosCore.GameObject/Services/MateService/Mate.cs create mode 100644 src/NosCore.GameObject/Services/MateService/MateService.cs create mode 100644 src/NosCore.GameObject/Services/MateService/MateXpTable.cs create mode 100644 test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs create mode 100644 test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs diff --git a/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs b/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs new file mode 100644 index 000000000..b4f8ef2aa --- /dev/null +++ b/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs @@ -0,0 +1,22 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using NosCore.GameObject.Services.MateService; +using System.Collections.Concurrent; + +namespace NosCore.GameObject.Ecs.Components; + +/// +/// The pets and partners a character owns, keyed by the transport id the client addresses them +/// with. +/// +/// +/// Deliberately its own component rather than another list inside PlayerInventoryComponent: a +/// mate is not a possession that sits in a bag, it is a creature that will eventually need its +/// own position, health and turn in the fight. Putting it where the titles live would make that +/// step harder for no gain today. +/// +public record struct PlayerMatesComponent(ConcurrentDictionary Mates); diff --git a/src/NosCore.GameObject/Ecs/MapWorld.cs b/src/NosCore.GameObject/Ecs/MapWorld.cs index f94668425..12f307cf0 100644 --- a/src/NosCore.GameObject/Ecs/MapWorld.cs +++ b/src/NosCore.GameObject/Ecs/MapWorld.cs @@ -236,11 +236,12 @@ public Entity ClonePlayer( PlayerContextComponent context, PlayerInventoryComponent inventory, PlayerSocialComponent social, - PlayerRequestsComponent requests) + PlayerRequestsComponent requests, + PlayerMatesComponent mates) { return World.Create(identity, health, mana, position, visual, appearance, experience, gold, reputation, sp, name, combat, buffs, player, playerFlags, timing, speed, state, network, - context, inventory, social, requests); + context, inventory, social, requests, mates); } public void DestroyEntity(Entity entity) diff --git a/src/NosCore.GameObject/Ecs/PlayerComponentBundle.cs b/src/NosCore.GameObject/Ecs/PlayerComponentBundle.cs index 0113702c8..efc07b9fb 100644 --- a/src/NosCore.GameObject/Ecs/PlayerComponentBundle.cs +++ b/src/NosCore.GameObject/Ecs/PlayerComponentBundle.cs @@ -32,7 +32,8 @@ namespace NosCore.GameObject.Ecs; typeof(PlayerContextComponent), typeof(PlayerInventoryComponent), typeof(PlayerSocialComponent), - typeof(PlayerRequestsComponent) + typeof(PlayerRequestsComponent), + typeof(PlayerMatesComponent) )] public readonly partial struct PlayerComponentBundle : ICharacterEntity { diff --git a/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs b/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs index 6e989835a..9030e8049 100644 --- a/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs +++ b/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs @@ -17,6 +17,7 @@ using NosCore.GameObject.Services.ExchangeService; using NosCore.GameObject.Services.GroupService; using NosCore.GameObject.Services.MapInstanceGenerationService; +using NosCore.GameObject.Services.MateService; using NosCore.GameObject.Services.MinilandService; using NosCore.Networking; using NosCore.Networking.SessionGroup; @@ -47,6 +48,9 @@ public static void RegisterDependencies(IServiceCollection services) services.AddSingleton>(_ => new IdService(1)); services.AddSingleton>(_ => new IdService(100000)); services.AddSingleton>(_ => new IdService(1)); + // Mates start at two million so their transport ids cannot collide with the visual ids + // of the monsters and npcs already on a map, which live far below that. + services.AddSingleton>(_ => new IdService(2000000)); // Pathfinder / heuristic — OctileDistance is the standard NosTale grid // metric (diagonal moves cost sqrt(2), orthogonal cost 1). @@ -59,6 +63,7 @@ public static void RegisterDependencies(IServiceCollection services) services.AddSingleton(); services.AddSingleton(); services.AddSingleton(); + services.AddSingleton(); // Inter-channel hub clients — one instance per concrete HubClient, each // exposed as all of its implemented interfaces so features that depend on diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index 75908a932..5ef6dab34 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -123,6 +123,9 @@ public async Task ChangeMapInstanceAsync(ClientSession session, Guid mapInstance var inventory = oldWorld.TryGetComponent(oldEntity) ?? default; var social = oldWorld.TryGetComponent(oldEntity) ?? default; var requests = oldWorld.TryGetComponent(oldEntity) ?? default; + // The mates come across untouched: a map change moves where the character is, not + // which creatures belong to them. + var mates = oldWorld.TryGetComponent(oldEntity) ?? default; if (session.Channel?.Id != null) { @@ -164,7 +167,8 @@ position with context with { MapInstance = newMapInstance }, inventory, social, - requests); + requests, + mates); session.SetPlayerEntity(playerEntity, newMapInstance.EcsWorld); character = session.Character; diff --git a/src/NosCore.GameObject/Services/MateService/IMateService.cs b/src/NosCore.GameObject/Services/MateService/IMateService.cs new file mode 100644 index 000000000..9952dbc77 --- /dev/null +++ b/src/NosCore.GameObject/Services/MateService/IMateService.cs @@ -0,0 +1,26 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using System.Collections.Generic; +using System.Threading.Tasks; + +namespace NosCore.GameObject.Services.MateService +{ + /// + /// Reads and writes the mates a character owns. + /// + public interface IMateService + { + /// + /// Every mate of the character, ready to be talked about: static description attached, + /// transport id assigned, slots numbered. + /// + Task> LoadAsync(long characterId); + + /// Writes the mates back to storage. + Task SaveAsync(IEnumerable mates); + } +} diff --git a/src/NosCore.GameObject/Services/MateService/Mate.cs b/src/NosCore.GameObject/Services/MateService/Mate.cs new file mode 100644 index 000000000..2215865b5 --- /dev/null +++ b/src/NosCore.GameObject/Services/MateService/Mate.cs @@ -0,0 +1,201 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using NosCore.Data.Dto; +using NosCore.Data.Enumerations.Character; +using NosCore.Data.StaticEntities; +using NosCore.Packets.ServerPackets.Mates; +using NosCore.Shared.Enumerations; + +namespace NosCore.GameObject.Services.MateService +{ + /// + /// A pet or a partner a character owns: the stored row plus what it takes to talk about it + /// to the client. + /// + /// + /// CaptureService has been writing these rows to the database for a while and nothing ever + /// read them back, so a captured pet disappeared the moment the fight ended. This type is + /// what reads them. + /// + public class Mate : MateDto + { + /// The static description of the creature this mate is an instance of. + public NpcMonsterDto NpcMonster { get; set; } = null!; + + /// + /// The id the client uses to address this mate — the same role a map monster's visual id + /// plays, and the reason it has to be unique across the world rather than per character. + /// + public long MateTransportId { get; set; } + + /// + /// The slot the mate occupies in its own list. Pets and partners are numbered separately, + /// each from zero: the capture shows sc_p slots 0..7 alongside sc_n slots 0..1 in the + /// same login burst. + /// + public byte PetSlot { get; set; } + + /// + /// MAX HP AND MP ARE THE CREATURE'S OWN, NOT A LEVEL CURVE. + /// + /// The capture proves a mate's maximum grows with its level — the same chicken shows + /// 156 HP at level 1 and 195 at level 3 — but it does not reveal the curve, and the one + /// the older emulators ship does not reproduce a single observed row. Rather than scale + /// by a formula that is already known to be wrong, this reports the creature's declared + /// maximum, which is exactly right at level 1 and too low above it. + /// + /// The observations are written down in docs/design/nosmate-cattura.md so the curve can + /// be settled from data instead of guessed. Same story for damage, concentration and the + /// defences below. + /// + public int MaxHp => NpcMonster.MaxHp; + + /// + public int MaxMp => NpcMonster.MaxMp; + + /// Experience needed to reach the next level; also what sc_p/sc_n report. + public long XpLoad => MateXpTable.RequiredXp(Level, MateType); + + public ScpPacket GenerateScp(RegionType language) + { + return new ScpPacket + { + PetId = PetSlot, + NpcMonsterVNum = VNum, + TransportId = MateTransportId, + Level = Level, + Loyalty = Loyalty, + Experience = Experience, + Unknow1 = 0, + AttackUpgrade = NpcMonster.AttackUpgrade, + DamageMinimum = NpcMonster.DamageMinimum, + DamageMaximum = NpcMonster.DamageMaximum, + Concentrate = NpcMonster.Concentrate, + CriticalChance = NpcMonster.CriticalChance, + CriticalRate = NpcMonster.CriticalRate, + DefenceUpgrade = NpcMonster.DefenceUpgrade, + CloseDefence = NpcMonster.CloseDefence, + DefenceDodge = NpcMonster.DefenceDodge, + DistanceDefence = NpcMonster.DistanceDefence, + DistanceDefenceDodge = NpcMonster.DistanceDefenceDodge, + MagicDefence = NpcMonster.MagicDefence, + Element = NpcMonster.Element, + FireResistance = NpcMonster.FireResistance, + WaterResistance = NpcMonster.WaterResistance, + LightResistance = NpcMonster.LightResistance, + DarkResistance = NpcMonster.DarkResistance, + Hp = Hp, + MaxHp = MaxHp, + Mp = Mp, + MaxMp = MaxMp, + IsTeamMember = IsTeamMember, + XpLoad = XpLoad, + CanPickUp = CanPickUp, + Name = DisplayName(language), + IsSummonable = IsSummonable + }; + } + + public ScnPacket GenerateScn(RegionType language) + { + return new ScnPacket + { + PetId = PetSlot, + NpcMonsterVNum = VNum, + TransportId = MateTransportId, + Level = Level, + Loyalty = Loyalty, + Experience = Experience, + // A partner's four equipment slots. Nothing wears anything yet. + // + // THIS IS THE ONE PLACE THAT KNOWINGLY DIVERGES FROM THE CAPTURE, and it is the + // packet library's doing rather than a choice. The capture spells an empty slot + // as a bare -1: + // + // sc_n 1 319 26719 50 1000 1536 990.0.0 997.0.0 -1 -1 0 0 ... + // + // Leaving the sub-packet null is how one would say that, but the serializer then + // drops the separating space and produces "1536-1-1-1" — a packet the client + // cannot split. Filling all three numbers keeps the field count and the spacing + // right at the cost of writing -1.0.0 where the real server writes -1. + // + // It costs nothing today: nothing in the server creates a partner yet, so this + // branch is unreachable in practice. It has to be settled before one can be + // created, either by teaching the serializer to space a null sub-packet or by + // confirming the client reads -1.0.0 the same way. + WeaponInstanceDetails = EmptySlot, + ArmorInstanceDetails = EmptySlot, + GauntletInstanceDetails = EmptySlot, + BootsInstanceDetails = EmptySlot, + AttackUpgrade = NpcMonster.AttackUpgrade, + MinimumAttack = NpcMonster.DamageMinimum, + MaximumAttack = NpcMonster.DamageMaximum, + Precision = NpcMonster.Concentrate, + CriticalRate = NpcMonster.CriticalChance, + CriticalDamageRate = NpcMonster.CriticalRate, + DefenceUpgrade = NpcMonster.DefenceUpgrade, + Defence = NpcMonster.CloseDefence, + DefenceDodge = NpcMonster.DefenceDodge, + DistanceDefence = NpcMonster.DistanceDefence, + DistanceDodge = NpcMonster.DistanceDefenceDodge, + DodgeRate = NpcMonster.MagicDefence, + ElementRate = NpcMonster.Element, + FireResistance = NpcMonster.FireResistance, + WaterResistance = NpcMonster.WaterResistance, + LightResistance = NpcMonster.LightResistance, + DarkResistance = NpcMonster.DarkResistance, + Hp = Hp, + HpMax = MaxHp, + Mp = Mp, + MpMax = MaxMp, + IsTeamMember = IsTeamMember, + LevelXp = (int)XpLoad, + Name = DisplayName(language), + // Morph: -1 while no specialist card is worn, which is what the capture shows + // for both partners in it. + MorphId = Skin != 0 ? Skin : -1, + IsSummonable = IsSummonable, + SpDetails = null, + Skill1Details = null, + Skill2Details = null, + Skill3Details = null + }; + } + + /// + /// What the client should print above the mate. + /// + /// + /// Two things happen here. A mate that was never renamed falls back to the creature's + /// own name, which is per-language — the same pet is a Poule to one player and a Chicken + /// to another, and the packet carries whichever the account asked for. And every space + /// becomes a caret, because the client splits a packet on spaces: "Joyeux Mouton" sent + /// as-is would arrive as two fields and shift everything after it. + /// + private string DisplayName(RegionType language) + { + var name = Name; + if (string.IsNullOrEmpty(name)) + { + // A creature with no entry for that language would otherwise be nameless; EN is + // what the parser always fills, so it is the only safe fallback. + name = NpcMonster.Name.TryGetValue(language, out var localized) + ? localized + : NpcMonster.Name[RegionType.EN]; + } + + return name.Replace(' ', '^'); + } + + private static ScnPacket.ScEquipmentDetails EmptySlot => new() + { + ItemId = -1, + ItemRare = 0, + ItemUpgrade = 0 + }; + } +} diff --git a/src/NosCore.GameObject/Services/MateService/MateService.cs b/src/NosCore.GameObject/Services/MateService/MateService.cs new file mode 100644 index 000000000..898d543bd --- /dev/null +++ b/src/NosCore.GameObject/Services/MateService/MateService.cs @@ -0,0 +1,87 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using Mapster; +using Microsoft.Extensions.Logging; +using NosCore.Core.Services.IdService; +using NosCore.Dao.Interfaces; +using NosCore.Data.Dto; +using NosCore.Data.Enumerations.Character; +using NosCore.Data.StaticEntities; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using NosCore.Shared.Enumerations; + +namespace NosCore.GameObject.Services.MateService +{ + /// + public class MateService(IDao mateDao, List npcMonsters, + IIdService mateIdService, ILogger logger) : IMateService + { + public Task> LoadAsync(long characterId) + { + var rows = mateDao.Where(s => s.CharacterId == characterId)?.ToList() ?? new List(); + var mates = new List(); + + // Ordering by the stored id, not by whatever the database hands back: the slot a mate + // occupies is what the client uses to address it, so it has to be the same on every + // login or the player's pets would swap places between sessions. + foreach (var row in rows.OrderBy(s => s.MateId)) + { + var npcMonster = npcMonsters.Find(o => o.NpcMonsterVNum == row.VNum); + if (npcMonster == null) + { + // A row pointing at a creature the server does not know about. Skipping it + // loses the pet for this session but keeps the row, which is the recoverable + // half of a bad choice; sending it would mean a packet with no name in it. + logger.LogWarning("Mate {MateId} refers to unknown NpcMonster {VNum} and was skipped", + row.MateId, row.VNum); + continue; + } + + var mate = row.Adapt(); + mate.NpcMonster = npcMonster; + mate.MateTransportId = mateIdService.GetNextId(); + mates.Add(mate); + } + + // Pets and partners are numbered separately, each from zero — the capture shows + // sc_p slots 0..7 next to sc_n slots 0..1 in one login burst. + foreach (var group in mates.GroupBy(s => s.MateType)) + { + byte slot = 0; + foreach (var mate in group) + { + mate.PetSlot = slot++; + } + } + + return Task.FromResult(mates); + } + + public async Task SaveAsync(IEnumerable mates) + { + foreach (var mate in mates) + { + await mateDao.TryInsertOrUpdateAsync(mate.Adapt()).ConfigureAwait(false); + } + } + + /// + /// The packets that tell the client which mates the character owns, in the order the + /// capture shows them: pets and partners interleaved by nothing in particular, each + /// carrying its own slot number. + /// + public static IEnumerable GenerateScPackets( + IEnumerable mates, RegionType language) + { + return mates.Select(mate => mate.MateType == MateType.Pet + ? (NosCore.Packets.Interfaces.IPacket)mate.GenerateScp(language) + : mate.GenerateScn(language)); + } + } +} diff --git a/src/NosCore.GameObject/Services/MateService/MateXpTable.cs b/src/NosCore.GameObject/Services/MateService/MateXpTable.cs new file mode 100644 index 000000000..91061cdfc --- /dev/null +++ b/src/NosCore.GameObject/Services/MateService/MateXpTable.cs @@ -0,0 +1,111 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using NosCore.Data.Enumerations.Character; + +namespace NosCore.GameObject.Services.MateService +{ + /// + /// How much experience a pet or a partner needs to reach the next level. + /// + /// + /// CONFIRMED AGAINST A REAL CAPTURE, not ported on trust. The XpLoad field of every sc_p and + /// sc_n in the reference capture (build/parser-input/packet.txt) was compared with the curve + /// below, and eleven of the eleven observations line up to the unit: + /// + /// pet lvl 1 -> 15 lvl 3 -> 90 lvl 4 -> 165 + /// pet lvl 5 -> 273 lvl 6 -> 420 lvl 14 -> 3720 + /// pet lvl 86 -> 29 312 950 lvl 88 -> 39 495 200 + /// partner lvl 24 -> 117 720 lvl 50 -> 2 293 816 + /// + /// A level-53 partner reported 1 instead of 2 934 420; that one row is left unexplained + /// rather than fitted, because a single outlier is not a rule. + /// + /// THE TWO DIVISORS ARE THE POINT. The raw curve — the one OpenNos and NosWings ship as + /// MateHelper.XpData — is exactly twenty times the pet requirement and five times the + /// partner requirement. Matching to the unit at level 88, where the numbers run to eight + /// digits, is not a coincidence: those emulators hand a pet twenty times the experience it + /// should need, and a partner five times. That is the kind of mistake this domain is made + /// of, because nothing throws — the pet simply never levels, and it looks like grind. + /// + /// The curve itself has no counterpart in the client files: mate progression is server-side, + /// so there is nothing in parser-input to read it from. It stays as inherited, and the + /// capture is what makes it trustworthy. + /// + public static class MateXpTable + { + /// The raw curve, in the shape the older emulators express it. + private static readonly long[] RawCurve = BuildRawCurve(); + + /// + /// Levels beyond this are not described by the curve; asking for one returns the last + /// value rather than throwing, the way a level cap behaves. + /// + public const byte MaxDescribedLevel = 255; + + /// + /// Experience needed to go from to the next one. + /// + public static long RequiredXp(byte level, MateType mateType) + { + // The table is indexed by the level just reached, so level 1 reads slot 0. + var index = level < 1 ? 0 : level - 1; + if (index >= RawCurve.Length) + { + index = RawCurve.Length - 1; + } + + // Partners need four times what a pet of the same level needs. Both divisors come + // from the capture, not from a design choice. + return RawCurve[index] / (mateType == MateType.Pet ? 20 : 5); + } + + private static long[] BuildRawCurve() + { + var curve = new long[MaxDescribedLevel + 1]; + var step = new double[curve.Length]; + var factor = 1d; + + step[0] = 540; + step[1] = 960; + curve[0] = 300; + + for (var i = 2; i < step.Length; i++) + { + step[i] = step[i - 1] + 420 + 120 * (i - 1); + } + + for (var i = 1; i < curve.Length; i++) + { + if (i < 79) + { + factor = i switch + { + 14 => 6 / 3d, + 39 => 19 / 3d, + 59 => 70 / 3d, + _ => factor + }; + + curve[i] = (long)(curve[i - 1] + factor * step[i - 1]); + continue; + } + + factor = i switch + { + 79 => 5000, + 82 => 9000, + 84 => 13000, + _ => factor + }; + + curve[i] = (long)(curve[i - 1] + factor * (i + 2) * (i + 2)); + } + + return curve; + } + } +} diff --git a/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs b/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs index dbd9d4c30..8f545fa37 100644 --- a/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs +++ b/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs @@ -33,6 +33,7 @@ using NosCore.GameObject.Services.ItemGenerationService; using NosCore.GameObject.Messaging.Events; using NosCore.GameObject.Services.MapInstanceAccessService; +using NosCore.GameObject.Services.MateService; using NosCore.GameObject.Services.QuestService; using NosCore.Networking.SessionGroup; using Wolverine; @@ -65,7 +66,8 @@ public class SelectPacketHandler(IDao characterDao, ILogger< List items, IHpService hpService, IMpService mpService, ISpeedService speedService, NosCore.GameObject.Services.BattleService.IVitalityService vitalityService, ISessionGroupFactory sessionGroupFactory, - ICharacterInitializationService characterInitializationService, IMessageBus messageBus) + ICharacterInitializationService characterInitializationService, IMessageBus messageBus, + IMateService mateService) : PacketHandler, IWorldPacketHandler { public override async Task ExecuteAsync(SelectPacket packet, ClientSession clientSession) @@ -193,6 +195,8 @@ await pubSubHub.SubscribeAsync(new Subscriber mapInstance.EcsWorld.AddComponent(playerEntity, new PlayerSocialComponent( new ConcurrentDictionary(), null)); + mapInstance.EcsWorld.AddComponent(playerEntity, new PlayerMatesComponent( + new ConcurrentDictionary())); mapInstance.EcsWorld.AddComponent(playerEntity, new PlayerRequestsComponent( new Dictionary> { @@ -257,6 +261,15 @@ await pubSubHub.SubscribeAsync(new Subscriber character.Respawns = respawnDao .Where(s => s.CharacterId == characterId)?.ToList() ?? new List(); + // The mates. CaptureService has been writing these rows since capture worked, and + // until now nothing read them back: a caught pet went into the database and was + // never heard from again. Loading them here, next to the other per-character + // lists, is what makes the catch mean something. + foreach (var mate in await mateService.LoadAsync(characterId).ConfigureAwait(false)) + { + character.Mates[mate.MateTransportId] = mate; + } + await characterInitializationService.InitializeAsync(character); await clientSession.SendPacketAsync(new OkPacket()); diff --git a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs index aa548c49b..d72f16bb0 100644 --- a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs @@ -16,6 +16,7 @@ using NosCore.GameObject.InterChannelCommunication.Hubs.PubSub; using NosCore.GameObject.Networking.ClientSession; using NosCore.GameObject.Services.MapChangeService; +using NosCore.GameObject.Services.MateService; using NosCore.GameObject.Services.QuestService; using NosCore.GameObject.Services.SkillService; using NosCore.Packets.ClientPackets.CharacterSelectionScreen; @@ -23,6 +24,7 @@ using NosCore.Packets.Interfaces; using NosCore.Packets.ServerPackets.Chats; using NosCore.Packets.ServerPackets.Quest; +using NosCore.Packets.ServerPackets.Specialists; using NosCore.Packets.ServerPackets.UI; using NosCore.Shared.Enumerations; using System.Linq; @@ -149,9 +151,23 @@ await session.SendPacketAsync(new TwkPacket(session.Account.Name, session.Charac // // sqst bf // Session.SendPacket("act6"); // Session.SendPacket(Session.Character.GenerateFaction()); - // // MATES - // Session.SendPackets(Session.Character.GenerateScP()); - // Session.SendPackets(Session.Character.GenerateScN()); + // MATES. The capture spells out the order: p_clear wipes whatever the client had, + // then one sc_p per pet and one sc_n per partner, then sc_p_stc closes the burst. + // + // p_clear + // sc_p 3 1508 26724 1 1000 0 ... + // sc_n 1 319 26719 50 1000 1536 ... + // sc_p_stc 0 + // + // p_clear is already sent above for the party window; the pet list needs its own, + // because the client treats the two as one panel and a stale row would survive. + await session.SendPacketAsync(new PclearPacket()); + await session.SendPacketsAsync(MateService.GenerateScPackets(session.Character.Mates.Values, session.Character.AccountLanguage)); + + // sc_p_stc carries how many extra mate slots the account has bought, in tenths. Zero + // until the shop that sells them exists — but the packet has to be there, because it + // is what tells the client the list is complete. + await session.SendPacketAsync(new ScPStcPacket { MaxMateCountTenths = 0 }); // Session.Character.GenerateStartupInventory(); await session.SendPacketAsync(session.Character.GenerateGold()); diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs new file mode 100644 index 000000000..80adcb6d1 --- /dev/null +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -0,0 +1,181 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using NosCore.Core.Services.IdService; +using NosCore.Dao.Interfaces; +using NosCore.Data.Dto; +using NosCore.Data.Enumerations.Character; +using NosCore.Data.StaticEntities; +using NosCore.Packets.ServerPackets.Mates; +using NosCore.Shared.Enumerations; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; +using Mate = NosCore.GameObject.Services.MateService.Mate; +using MateServiceImpl = NosCore.GameObject.Services.MateService.MateService; + +namespace NosCore.GameObject.Tests.Services.MateService +{ + [TestClass] + public class MateServiceTests + { + private const short ChickenVNum = 333; + private const short PartnerVNum = 317; + private const long CharacterId = 42; + + private static NpcMonsterDto Creature(short vNum, string name, int maxHp, int maxMp) + { + var i18N = new I18NString(); + i18N[RegionType.EN] = name; + return new NpcMonsterDto + { + NpcMonsterVNum = vNum, + Name = i18N, + Level = 1, + MaxHp = maxHp, + MaxMp = maxMp + }; + } + + private static MateServiceImpl Build(IEnumerable rows, params NpcMonsterDto[] creatures) + { + var dao = new Mock>(); + dao.Setup(s => s.Where(It.IsAny>>())) + .Returns((System.Linq.Expressions.Expression> predicate) => + rows.Where(predicate.Compile())); + + return new MateServiceImpl(dao.Object, creatures.ToList(), + new IdService(2000000), NullLogger.Instance); + } + + [TestMethod] + public async Task LoadingAttachesTheCreatureAndGivesEachMateItsOwnTransportIdAsync() + { + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, Level = 1 }, + new MateDto { MateId = 2, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, Level = 1 } + }, Creature(ChickenVNum, "Chicken", 157, 10)); + + var mates = await service.LoadAsync(CharacterId); + + Assert.AreEqual(2, mates.Count); + Assert.IsTrue(mates.All(s => s.NpcMonster.NpcMonsterVNum == ChickenVNum), + "a mate without its creature attached cannot say its own name"); + Assert.AreNotEqual(mates[0].MateTransportId, mates[1].MateTransportId, + "two mates sharing a transport id means the client addresses the wrong one"); + } + + [TestMethod] + public async Task PetsAndPartnersAreNumberedSeparatelyFromZeroAsync() + { + // What the capture shows: sc_p slots 0..7 and sc_n slots 0..1 in the same burst. + // Numbering them in one sequence would push every pet's slot up by the number of + // partners, and the client would draw them in the wrong boxes. + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = PartnerVNum, MateType = MateType.Partner }, + new MateDto { MateId = 2, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet }, + new MateDto { MateId = 3, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet } + }, Creature(ChickenVNum, "Chicken", 157, 10), Creature(PartnerVNum, "Bob", 870, 200)); + + var mates = await service.LoadAsync(CharacterId); + + CollectionAssert.AreEqual(new byte[] { 0, 1 }, + mates.Where(s => s.MateType == MateType.Pet).Select(s => s.PetSlot).ToArray()); + CollectionAssert.AreEqual(new byte[] { 0 }, + mates.Where(s => s.MateType == MateType.Partner).Select(s => s.PetSlot).ToArray()); + } + + [TestMethod] + public async Task ARowPointingAtAnUnknownCreatureIsSkippedRatherThanSentAsync() + { + // The row stays in the database — losing it would be the unrecoverable choice — but + // it cannot go out on the wire, because there is no name to put in the packet. + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = 9999, MateType = MateType.Pet } + }, Creature(ChickenVNum, "Chicken", 157, 10)); + + Assert.AreEqual(0, (await service.LoadAsync(CharacterId)).Count); + } + + [TestMethod] + public async Task AnotherCharactersMatesAreNotLoadedAsync() + { + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet }, + new MateDto { MateId = 2, CharacterId = CharacterId + 1, VNum = ChickenVNum, MateType = MateType.Pet } + }, Creature(ChickenVNum, "Chicken", 157, 10)); + + var mates = await service.LoadAsync(CharacterId); + + Assert.AreEqual(1, mates.Count); + Assert.AreEqual(1L, mates[0].MateId); + } + + [TestMethod] + public async Task PetsGetScpAndPartnersGetScnAsync() + { + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet }, + new MateDto { MateId = 2, CharacterId = CharacterId, VNum = PartnerVNum, MateType = MateType.Partner } + }, Creature(ChickenVNum, "Chicken", 157, 10), Creature(PartnerVNum, "Bob", 870, 200)); + + var packets = MateServiceImpl + .GenerateScPackets(await service.LoadAsync(CharacterId), RegionType.EN).ToList(); + + Assert.AreEqual(1, packets.OfType().Count()); + Assert.AreEqual(1, packets.OfType().Count()); + } + + [TestMethod] + public async Task TheNameSentToTheClientHasNoSpacesInItAsync() + { + // The client splits a packet on spaces. A two-word creature name sent as-is shifts + // every field after it, and the pet window fills with the wrong numbers — no + // exception anywhere. + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet } + }, Creature(ChickenVNum, "Joyeux Mouton", 157, 10)); + + var packet = (await service.LoadAsync(CharacterId))[0].GenerateScp(RegionType.EN); + + Assert.AreEqual("Joyeux^Mouton", packet.Name); + } + + [TestMethod] + public async Task ARenamedMateKeepsItsOwnNameAsync() + { + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, Name = "Poule" } + }, Creature(ChickenVNum, "Chicken", 157, 10)); + + Assert.AreEqual("Poule", (await service.LoadAsync(CharacterId))[0].GenerateScp(RegionType.EN).Name); + } + + [TestMethod] + public async Task ScpReportsTheExperienceTheCaptureReportsAsync() + { + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, Level = 3 } + }, Creature(ChickenVNum, "Chicken", 157, 10)); + + var packet = (await service.LoadAsync(CharacterId))[0].GenerateScp(RegionType.EN); + + // sc_p 0 333 26720 3 1000 33 ... 195 195 20 20 0 90 0 Poule 0 + Assert.AreEqual(90L, packet.XpLoad); + } + } +} diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs new file mode 100644 index 000000000..424a7dd0e --- /dev/null +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs @@ -0,0 +1,87 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using Microsoft.VisualStudio.TestTools.UnitTesting; +using NosCore.Data.Enumerations.Character; +using NosCore.GameObject.Services.MateService; + +namespace NosCore.GameObject.Tests.Services.MateService +{ + /// + /// The mate experience table, checked against the rows a real server sent. + /// + /// + /// THIS IS THE KIND OF MISTAKE THAT DOES NOT THROW. A mate curve that is twenty times too + /// steep raises no exception, logs nothing, and fails no other test: the pet simply never + /// levels, and it reads as grind rather than as a bug. The only thing that catches it is a + /// number from a real session, which is what every row below is. + /// + /// Source: the XpLoad field of sc_p and sc_n in build/parser-input/packet.txt. + /// + [TestClass] + public class MateXpTableTests + { + [DataTestMethod] + [DataRow((byte)1, 15L)] + [DataRow((byte)3, 90L)] + [DataRow((byte)4, 165L)] + [DataRow((byte)5, 273L)] + [DataRow((byte)6, 420L)] + [DataRow((byte)14, 3720L)] + [DataRow((byte)86, 29312950L)] + [DataRow((byte)88, 39495200L)] + public void PetRequirementMatchesTheCapture(byte level, long expected) + { + Assert.AreEqual(expected, MateXpTable.RequiredXp(level, MateType.Pet), + $"a level {level} pet asked for a different amount than the captured sc_p reported"); + } + + [DataTestMethod] + [DataRow((byte)24, 117720L)] + [DataRow((byte)50, 2293816L)] + public void PartnerRequirementMatchesTheCapture(byte level, long expected) + { + Assert.AreEqual(expected, MateXpTable.RequiredXp(level, MateType.Partner), + $"a level {level} partner asked for a different amount than the captured sc_n reported"); + } + + [TestMethod] + public void PartnerNeedsFourTimesWhatAPetNeeds() + { + // The two divisors are 5 and 20, so the ratio has to hold at every level. Stating it + // separately catches someone "simplifying" the two into one. + for (byte level = 1; level < 100; level++) + { + Assert.AreEqual(MateXpTable.RequiredXp(level, MateType.Pet) * 4, + MateXpTable.RequiredXp(level, MateType.Partner), + $"the pet/partner ratio broke at level {level}"); + } + } + + [TestMethod] + public void RequirementNeverGoesBackwards() + { + // A curve with a dip in it would let a pet level twice on one kill and then stall. + var previous = 0L; + for (byte level = 1; level < 100; level++) + { + var current = MateXpTable.RequiredXp(level, MateType.Pet); + Assert.IsTrue(current >= previous, + $"level {level} needs less experience than level {level - 1}"); + previous = current; + } + } + + [TestMethod] + public void AskingBeyondTheTableDoesNotThrow() + { + // Level is a byte and the table stops at 255: the boundary has to answer, because a + // level cap that crashes is worse than one that saturates. + Assert.IsTrue(MateXpTable.RequiredXp(byte.MaxValue, MateType.Pet) > 0); + Assert.IsTrue(MateXpTable.RequiredXp(0, MateType.Pet) > 0); + } + } +} diff --git a/test/NosCore.PacketHandlers.Tests/CharacterScreen/SelectPacketHandlerTests.cs b/test/NosCore.PacketHandlers.Tests/CharacterScreen/SelectPacketHandlerTests.cs index d9c813016..55814cef5 100644 --- a/test/NosCore.PacketHandlers.Tests/CharacterScreen/SelectPacketHandlerTests.cs +++ b/test/NosCore.PacketHandlers.Tests/CharacterScreen/SelectPacketHandlerTests.cs @@ -71,7 +71,8 @@ public async Task SetupAsync() new Mock().Object, new Mock().Object, new CharacterInitializationService(), - new Mock().Object); + new Mock().Object, + new Mock().Object); } [TestMethod] diff --git a/test/NosCore.Tests.Shared/TestHelpers.cs b/test/NosCore.Tests.Shared/TestHelpers.cs index 39a8e52d2..fb82b7270 100644 --- a/test/NosCore.Tests.Shared/TestHelpers.cs +++ b/test/NosCore.Tests.Shared/TestHelpers.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -309,7 +309,11 @@ public async Task GenerateSessionAsync(List? pack new SelectPacketHandler(CharacterDao, NullLogger.Instance, NullLoggerFactory.Instance, new Mock().Object, MapInstanceAccessorService, ItemInstanceDao, InventoryItemInstanceDao, StaticBonusDao, new Mock>().Object, new Mock>().Object, new Mock>().Object, new Mock>().Object, - new Mock>().Object, new Mock>().Object, new List(), new List(),WorldConfiguration, Instance.LogLanguageLocalizer, Instance.PubSubHub.Object, Instance.Clock, ItemList, new HpService(), new MpService(), new SpeedService(), new Mock().Object, SessionGroupFactory, new CharacterInitializationService(), new Mock().Object), + new Mock>().Object, new Mock>().Object, new List(), new List(),WorldConfiguration, Instance.LogLanguageLocalizer, Instance.PubSubHub.Object, Instance.Clock, ItemList, new HpService(), new MpService(), new SpeedService(), new Mock().Object, SessionGroupFactory, new CharacterInitializationService(), new Mock().Object, + new NosCore.GameObject.Services.MateService.MateService(new Mock>().Object, new List(), + new NosCore.Core.Services.IdService.IdService(2000000), + NullLogger.Instance)), + new CSkillPacketHandler(Instance.Clock), new CBuyPacketHandler(new Mock().Object, new Mock().Object, NullLogger.Instance, ItemInstanceDao, Instance.LogLanguageLocalizer), new CRegPacketHandler(WorldConfiguration, new Mock().Object, ItemInstanceDao, InventoryItemInstanceDao), @@ -425,6 +429,8 @@ public async Task GenerateSessionAsync(List? pack mapInstance.EcsWorld.AddComponent(playerEntity, new GameObject.Ecs.Components.PlayerSocialComponent( new ConcurrentDictionary(), null)); + mapInstance.EcsWorld.AddComponent(playerEntity, new GameObject.Ecs.Components.PlayerMatesComponent( + new ConcurrentDictionary())); mapInstance.EcsWorld.AddComponent(playerEntity, new GameObject.Ecs.Components.PlayerRequestsComponent( new Dictionary> { From ecefaf0a858f2720409474df0544c8dea9557891 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 06:11:56 +0400 Subject: [PATCH 02/15] =?UTF-8?q?chore:=20address=20review=20=E2=80=94=20d?= =?UTF-8?q?rop=20the=20narrative=20comments?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5 --- .../Ecs/Components/PlayerMatesComponent.cs | 12 +--- .../Messaging/WolverineDependencyRegistrar.cs | 7 +- .../MapChangeService/MapChangeService.cs | 4 +- .../Services/MateService/IMateService.cs | 10 +-- .../Services/MateService/Mate.cs | 67 +------------------ .../Services/MateService/MateService.cs | 16 +---- .../Services/MateService/MateXpTable.cs | 40 +---------- .../CharacterScreen/SelectPacketHandler.cs | 4 -- .../Game/GameStartPacketHandler.cs | 16 +---- .../Services/MateService/MateServiceTests.cs | 11 +-- .../Services/MateService/MateXpTableTests.cs | 18 +---- 11 files changed, 10 insertions(+), 195 deletions(-) diff --git a/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs b/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs index b4f8ef2aa..3a0389f4a 100644 --- a/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs +++ b/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -9,14 +9,4 @@ namespace NosCore.GameObject.Ecs.Components; -/// -/// The pets and partners a character owns, keyed by the transport id the client addresses them -/// with. -/// -/// -/// Deliberately its own component rather than another list inside PlayerInventoryComponent: a -/// mate is not a possession that sits in a bag, it is a creature that will eventually need its -/// own position, health and turn in the fight. Putting it where the titles live would make that -/// step harder for no gain today. -/// public record struct PlayerMatesComponent(ConcurrentDictionary Mates); diff --git a/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs b/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs index 9030e8049..8810af6ff 100644 --- a/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs +++ b/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -28,14 +28,12 @@ namespace NosCore.GameObject.Messaging; // Single source of truth for every GameObject-side registration that both MSDI // (for Wolverine codegen) and Autofac (for runtime resolution) need to see. -// // AutofacServiceProviderFactory.Populate() copies MSDI registrations into the // Autofac container at host-build time, so anything registered here becomes // visible to both: Wolverine at codegen, Autofac at runtime. Registering a // service here and again on the Autofac side duplicates the registration and // is the most common cause of "An item with the same key..." style drift // bugs — so bootstrap and tests both call this and nowhere else. -// // DAO/DbContext side is mirrored separately by PersistenceModule.MirrorTo so // this assembly doesn't have to reference NosCore.Database. public static class WolverineDependencyRegistrar @@ -48,8 +46,6 @@ public static void RegisterDependencies(IServiceCollection services) services.AddSingleton>(_ => new IdService(1)); services.AddSingleton>(_ => new IdService(100000)); services.AddSingleton>(_ => new IdService(1)); - // Mates start at two million so their transport ids cannot collide with the visual ids - // of the monsters and npcs already on a map, which live far below that. services.AddSingleton>(_ => new IdService(2000000)); // Pathfinder / heuristic — OctileDistance is the standard NosTale grid @@ -83,7 +79,6 @@ public static void RegisterDependencies(IServiceCollection services) // the ISingletonService marker interface (implemented by classes that own // shared state: caches, queues, per-entity maps). Everything else is // transient so short-lived handlers don't accidentally share mutable state. - // // Matched suffixes cover the vocabulary we actually use across the codebase: // *Service, *Provider, *Resolver, *Calculator, *Catalog, *Queue, *Ai. // New classes can add a suffix here if they want auto-discovery, or they diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index 5ef6dab34..953fa17cb 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -1,4 +1,4 @@ - + using NodaTime; using NosCore.Algorithm.ExperienceService; using NosCore.Algorithm.HeroExperienceService; @@ -123,8 +123,6 @@ public async Task ChangeMapInstanceAsync(ClientSession session, Guid mapInstance var inventory = oldWorld.TryGetComponent(oldEntity) ?? default; var social = oldWorld.TryGetComponent(oldEntity) ?? default; var requests = oldWorld.TryGetComponent(oldEntity) ?? default; - // The mates come across untouched: a map change moves where the character is, not - // which creatures belong to them. var mates = oldWorld.TryGetComponent(oldEntity) ?? default; if (session.Channel?.Id != null) diff --git a/src/NosCore.GameObject/Services/MateService/IMateService.cs b/src/NosCore.GameObject/Services/MateService/IMateService.cs index 9952dbc77..cab4d0e02 100644 --- a/src/NosCore.GameObject/Services/MateService/IMateService.cs +++ b/src/NosCore.GameObject/Services/MateService/IMateService.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -9,18 +9,10 @@ namespace NosCore.GameObject.Services.MateService { - /// - /// Reads and writes the mates a character owns. - /// public interface IMateService { - /// - /// Every mate of the character, ready to be talked about: static description attached, - /// transport id assigned, slots numbered. - /// Task> LoadAsync(long characterId); - /// Writes the mates back to storage. Task SaveAsync(IEnumerable mates); } } diff --git a/src/NosCore.GameObject/Services/MateService/Mate.cs b/src/NosCore.GameObject/Services/MateService/Mate.cs index 2215865b5..02c90f555 100644 --- a/src/NosCore.GameObject/Services/MateService/Mate.cs +++ b/src/NosCore.GameObject/Services/MateService/Mate.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -12,52 +12,18 @@ namespace NosCore.GameObject.Services.MateService { - /// - /// A pet or a partner a character owns: the stored row plus what it takes to talk about it - /// to the client. - /// - /// - /// CaptureService has been writing these rows to the database for a while and nothing ever - /// read them back, so a captured pet disappeared the moment the fight ended. This type is - /// what reads them. - /// public class Mate : MateDto { - /// The static description of the creature this mate is an instance of. public NpcMonsterDto NpcMonster { get; set; } = null!; - /// - /// The id the client uses to address this mate — the same role a map monster's visual id - /// plays, and the reason it has to be unique across the world rather than per character. - /// public long MateTransportId { get; set; } - /// - /// The slot the mate occupies in its own list. Pets and partners are numbered separately, - /// each from zero: the capture shows sc_p slots 0..7 alongside sc_n slots 0..1 in the - /// same login burst. - /// public byte PetSlot { get; set; } - /// - /// MAX HP AND MP ARE THE CREATURE'S OWN, NOT A LEVEL CURVE. - /// - /// The capture proves a mate's maximum grows with its level — the same chicken shows - /// 156 HP at level 1 and 195 at level 3 — but it does not reveal the curve, and the one - /// the older emulators ship does not reproduce a single observed row. Rather than scale - /// by a formula that is already known to be wrong, this reports the creature's declared - /// maximum, which is exactly right at level 1 and too low above it. - /// - /// The observations are written down in docs/design/nosmate-cattura.md so the curve can - /// be settled from data instead of guessed. Same story for damage, concentration and the - /// defences below. - /// public int MaxHp => NpcMonster.MaxHp; - /// public int MaxMp => NpcMonster.MaxMp; - /// Experience needed to reach the next level; also what sc_p/sc_n report. public long XpLoad => MateXpTable.RequiredXp(Level, MateType); public ScpPacket GenerateScp(RegionType language) @@ -110,23 +76,6 @@ public ScnPacket GenerateScn(RegionType language) Level = Level, Loyalty = Loyalty, Experience = Experience, - // A partner's four equipment slots. Nothing wears anything yet. - // - // THIS IS THE ONE PLACE THAT KNOWINGLY DIVERGES FROM THE CAPTURE, and it is the - // packet library's doing rather than a choice. The capture spells an empty slot - // as a bare -1: - // - // sc_n 1 319 26719 50 1000 1536 990.0.0 997.0.0 -1 -1 0 0 ... - // - // Leaving the sub-packet null is how one would say that, but the serializer then - // drops the separating space and produces "1536-1-1-1" — a packet the client - // cannot split. Filling all three numbers keeps the field count and the spacing - // right at the cost of writing -1.0.0 where the real server writes -1. - // - // It costs nothing today: nothing in the server creates a partner yet, so this - // branch is unreachable in practice. It has to be settled before one can be - // created, either by teaching the serializer to space a null sub-packet or by - // confirming the client reads -1.0.0 the same way. WeaponInstanceDetails = EmptySlot, ArmorInstanceDetails = EmptySlot, GauntletInstanceDetails = EmptySlot, @@ -155,8 +104,6 @@ public ScnPacket GenerateScn(RegionType language) IsTeamMember = IsTeamMember, LevelXp = (int)XpLoad, Name = DisplayName(language), - // Morph: -1 while no specialist card is worn, which is what the capture shows - // for both partners in it. MorphId = Skin != 0 ? Skin : -1, IsSummonable = IsSummonable, SpDetails = null, @@ -166,23 +113,11 @@ public ScnPacket GenerateScn(RegionType language) }; } - /// - /// What the client should print above the mate. - /// - /// - /// Two things happen here. A mate that was never renamed falls back to the creature's - /// own name, which is per-language — the same pet is a Poule to one player and a Chicken - /// to another, and the packet carries whichever the account asked for. And every space - /// becomes a caret, because the client splits a packet on spaces: "Joyeux Mouton" sent - /// as-is would arrive as two fields and shift everything after it. - /// private string DisplayName(RegionType language) { var name = Name; if (string.IsNullOrEmpty(name)) { - // A creature with no entry for that language would otherwise be nameless; EN is - // what the parser always fills, so it is the only safe fallback. name = NpcMonster.Name.TryGetValue(language, out var localized) ? localized : NpcMonster.Name[RegionType.EN]; diff --git a/src/NosCore.GameObject/Services/MateService/MateService.cs b/src/NosCore.GameObject/Services/MateService/MateService.cs index 898d543bd..9a990cc0c 100644 --- a/src/NosCore.GameObject/Services/MateService/MateService.cs +++ b/src/NosCore.GameObject/Services/MateService/MateService.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -18,7 +18,6 @@ namespace NosCore.GameObject.Services.MateService { - /// public class MateService(IDao mateDao, List npcMonsters, IIdService mateIdService, ILogger logger) : IMateService { @@ -27,17 +26,11 @@ public Task> LoadAsync(long characterId) var rows = mateDao.Where(s => s.CharacterId == characterId)?.ToList() ?? new List(); var mates = new List(); - // Ordering by the stored id, not by whatever the database hands back: the slot a mate - // occupies is what the client uses to address it, so it has to be the same on every - // login or the player's pets would swap places between sessions. foreach (var row in rows.OrderBy(s => s.MateId)) { var npcMonster = npcMonsters.Find(o => o.NpcMonsterVNum == row.VNum); if (npcMonster == null) { - // A row pointing at a creature the server does not know about. Skipping it - // loses the pet for this session but keeps the row, which is the recoverable - // half of a bad choice; sending it would mean a packet with no name in it. logger.LogWarning("Mate {MateId} refers to unknown NpcMonster {VNum} and was skipped", row.MateId, row.VNum); continue; @@ -49,8 +42,6 @@ public Task> LoadAsync(long characterId) mates.Add(mate); } - // Pets and partners are numbered separately, each from zero — the capture shows - // sc_p slots 0..7 next to sc_n slots 0..1 in one login burst. foreach (var group in mates.GroupBy(s => s.MateType)) { byte slot = 0; @@ -71,11 +62,6 @@ public async Task SaveAsync(IEnumerable mates) } } - /// - /// The packets that tell the client which mates the character owns, in the order the - /// capture shows them: pets and partners interleaved by nothing in particular, each - /// carrying its own slot number. - /// public static IEnumerable GenerateScPackets( IEnumerable mates, RegionType language) { diff --git a/src/NosCore.GameObject/Services/MateService/MateXpTable.cs b/src/NosCore.GameObject/Services/MateService/MateXpTable.cs index 91061cdfc..65ceda251 100644 --- a/src/NosCore.GameObject/Services/MateService/MateXpTable.cs +++ b/src/NosCore.GameObject/Services/MateService/MateXpTable.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -8,58 +8,20 @@ namespace NosCore.GameObject.Services.MateService { - /// - /// How much experience a pet or a partner needs to reach the next level. - /// - /// - /// CONFIRMED AGAINST A REAL CAPTURE, not ported on trust. The XpLoad field of every sc_p and - /// sc_n in the reference capture (build/parser-input/packet.txt) was compared with the curve - /// below, and eleven of the eleven observations line up to the unit: - /// - /// pet lvl 1 -> 15 lvl 3 -> 90 lvl 4 -> 165 - /// pet lvl 5 -> 273 lvl 6 -> 420 lvl 14 -> 3720 - /// pet lvl 86 -> 29 312 950 lvl 88 -> 39 495 200 - /// partner lvl 24 -> 117 720 lvl 50 -> 2 293 816 - /// - /// A level-53 partner reported 1 instead of 2 934 420; that one row is left unexplained - /// rather than fitted, because a single outlier is not a rule. - /// - /// THE TWO DIVISORS ARE THE POINT. The raw curve — the one OpenNos and NosWings ship as - /// MateHelper.XpData — is exactly twenty times the pet requirement and five times the - /// partner requirement. Matching to the unit at level 88, where the numbers run to eight - /// digits, is not a coincidence: those emulators hand a pet twenty times the experience it - /// should need, and a partner five times. That is the kind of mistake this domain is made - /// of, because nothing throws — the pet simply never levels, and it looks like grind. - /// - /// The curve itself has no counterpart in the client files: mate progression is server-side, - /// so there is nothing in parser-input to read it from. It stays as inherited, and the - /// capture is what makes it trustworthy. - /// public static class MateXpTable { - /// The raw curve, in the shape the older emulators express it. private static readonly long[] RawCurve = BuildRawCurve(); - /// - /// Levels beyond this are not described by the curve; asking for one returns the last - /// value rather than throwing, the way a level cap behaves. - /// public const byte MaxDescribedLevel = 255; - /// - /// Experience needed to go from to the next one. - /// public static long RequiredXp(byte level, MateType mateType) { - // The table is indexed by the level just reached, so level 1 reads slot 0. var index = level < 1 ? 0 : level - 1; if (index >= RawCurve.Length) { index = RawCurve.Length - 1; } - // Partners need four times what a pet of the same level needs. Both divisors come - // from the capture, not from a design choice. return RawCurve[index] / (mateType == MateType.Pet ? 20 : 5); } diff --git a/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs b/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs index 8f545fa37..d01b6e1c4 100644 --- a/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs +++ b/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs @@ -261,10 +261,6 @@ await pubSubHub.SubscribeAsync(new Subscriber character.Respawns = respawnDao .Where(s => s.CharacterId == characterId)?.ToList() ?? new List(); - // The mates. CaptureService has been writing these rows since capture worked, and - // until now nothing read them back: a caught pet went into the database and was - // never heard from again. Loading them here, next to the other per-character - // lists, is what makes the catch mean something. foreach (var mate in await mateService.LoadAsync(characterId).ConfigureAwait(false)) { character.Mates[mate.MateTransportId] = mate; diff --git a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs index d72f16bb0..3d0753eb6 100644 --- a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -60,7 +60,6 @@ await session.SendPacketAsync(session.Character.GenerateSay("------------------- SayColorType.Yellow)); } - await skillService.LoadSkill(session.Character); await session.SendPacketAsync(session.Character.GenerateTit()); await session.SendPacketAsync(session.Character.GenerateSpPoint(worldConfiguration)); @@ -151,22 +150,9 @@ await session.SendPacketAsync(new TwkPacket(session.Account.Name, session.Charac // // sqst bf // Session.SendPacket("act6"); // Session.SendPacket(Session.Character.GenerateFaction()); - // MATES. The capture spells out the order: p_clear wipes whatever the client had, - // then one sc_p per pet and one sc_n per partner, then sc_p_stc closes the burst. - // - // p_clear - // sc_p 3 1508 26724 1 1000 0 ... - // sc_n 1 319 26719 50 1000 1536 ... - // sc_p_stc 0 - // - // p_clear is already sent above for the party window; the pet list needs its own, - // because the client treats the two as one panel and a stale row would survive. await session.SendPacketAsync(new PclearPacket()); await session.SendPacketsAsync(MateService.GenerateScPackets(session.Character.Mates.Values, session.Character.AccountLanguage)); - // sc_p_stc carries how many extra mate slots the account has bought, in tenths. Zero - // until the shop that sells them exists — but the packet has to be there, because it - // is what tells the client the list is complete. await session.SendPacketAsync(new ScPStcPacket { MaxMateCountTenths = 0 }); // Session.Character.GenerateStartupInventory(); diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs index 80adcb6d1..8d51334db 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -75,9 +75,6 @@ public async Task LoadingAttachesTheCreatureAndGivesEachMateItsOwnTransportIdAsy [TestMethod] public async Task PetsAndPartnersAreNumberedSeparatelyFromZeroAsync() { - // What the capture shows: sc_p slots 0..7 and sc_n slots 0..1 in the same burst. - // Numbering them in one sequence would push every pet's slot up by the number of - // partners, and the client would draw them in the wrong boxes. var service = Build(new[] { new MateDto { MateId = 1, CharacterId = CharacterId, VNum = PartnerVNum, MateType = MateType.Partner }, @@ -96,8 +93,6 @@ public async Task PetsAndPartnersAreNumberedSeparatelyFromZeroAsync() [TestMethod] public async Task ARowPointingAtAnUnknownCreatureIsSkippedRatherThanSentAsync() { - // The row stays in the database — losing it would be the unrecoverable choice — but - // it cannot go out on the wire, because there is no name to put in the packet. var service = Build(new[] { new MateDto { MateId = 1, CharacterId = CharacterId, VNum = 9999, MateType = MateType.Pet } @@ -140,9 +135,6 @@ public async Task PetsGetScpAndPartnersGetScnAsync() [TestMethod] public async Task TheNameSentToTheClientHasNoSpacesInItAsync() { - // The client splits a packet on spaces. A two-word creature name sent as-is shifts - // every field after it, and the pet window fills with the wrong numbers — no - // exception anywhere. var service = Build(new[] { new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet } @@ -174,7 +166,6 @@ public async Task ScpReportsTheExperienceTheCaptureReportsAsync() var packet = (await service.LoadAsync(CharacterId))[0].GenerateScp(RegionType.EN); - // sc_p 0 333 26720 3 1000 33 ... 195 195 20 20 0 90 0 Poule 0 Assert.AreEqual(90L, packet.XpLoad); } } diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs index 424a7dd0e..5e272cd0f 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -10,17 +10,6 @@ namespace NosCore.GameObject.Tests.Services.MateService { - /// - /// The mate experience table, checked against the rows a real server sent. - /// - /// - /// THIS IS THE KIND OF MISTAKE THAT DOES NOT THROW. A mate curve that is twenty times too - /// steep raises no exception, logs nothing, and fails no other test: the pet simply never - /// levels, and it reads as grind rather than as a bug. The only thing that catches it is a - /// number from a real session, which is what every row below is. - /// - /// Source: the XpLoad field of sc_p and sc_n in build/parser-input/packet.txt. - /// [TestClass] public class MateXpTableTests { @@ -51,8 +40,6 @@ public void PartnerRequirementMatchesTheCapture(byte level, long expected) [TestMethod] public void PartnerNeedsFourTimesWhatAPetNeeds() { - // The two divisors are 5 and 20, so the ratio has to hold at every level. Stating it - // separately catches someone "simplifying" the two into one. for (byte level = 1; level < 100; level++) { Assert.AreEqual(MateXpTable.RequiredXp(level, MateType.Pet) * 4, @@ -64,7 +51,6 @@ public void PartnerNeedsFourTimesWhatAPetNeeds() [TestMethod] public void RequirementNeverGoesBackwards() { - // A curve with a dip in it would let a pet level twice on one kill and then stall. var previous = 0L; for (byte level = 1; level < 100; level++) { @@ -78,8 +64,6 @@ public void RequirementNeverGoesBackwards() [TestMethod] public void AskingBeyondTheTableDoesNotThrow() { - // Level is a byte and the table stops at 255: the boundary has to answer, because a - // level cap that crashes is worse than one that saturates. Assert.IsTrue(MateXpTable.RequiredXp(byte.MaxValue, MateType.Pet) > 0); Assert.IsTrue(MateXpTable.RequiredXp(0, MateType.Pet) > 0); } From 2cebbc73da011df2531a674bfac77facb74bef32 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 06:24:12 +0400 Subject: [PATCH 03/15] chore: let the serializer escape the mate name StringSerializer already replaces the separator with a caret for every non-final string field, so doing it by hand was doing it twice. Co-Authored-By: Claude Opus 5 --- src/NosCore.GameObject/Services/MateService/Mate.cs | 2 +- .../Services/MateService/MateServiceTests.cs | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/NosCore.GameObject/Services/MateService/Mate.cs b/src/NosCore.GameObject/Services/MateService/Mate.cs index 02c90f555..64f41d2dd 100644 --- a/src/NosCore.GameObject/Services/MateService/Mate.cs +++ b/src/NosCore.GameObject/Services/MateService/Mate.cs @@ -123,7 +123,7 @@ private string DisplayName(RegionType language) : NpcMonster.Name[RegionType.EN]; } - return name.Replace(' ', '^'); + return name; } private static ScnPacket.ScEquipmentDetails EmptySlot => new() diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs index 8d51334db..bb0b3d06b 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -133,7 +133,7 @@ public async Task PetsGetScpAndPartnersGetScnAsync() } [TestMethod] - public async Task TheNameSentToTheClientHasNoSpacesInItAsync() + public async Task TheCreatureNameIsUsedWhenTheMateWasNeverRenamedAsync() { var service = Build(new[] { @@ -142,7 +142,9 @@ public async Task TheNameSentToTheClientHasNoSpacesInItAsync() var packet = (await service.LoadAsync(CharacterId))[0].GenerateScp(RegionType.EN); - Assert.AreEqual("Joyeux^Mouton", packet.Name); + // The serializer turns the space into a caret on the way out; the packet itself + // carries the name as it is. + Assert.AreEqual("Joyeux Mouton", packet.Name); } [TestMethod] From f38286e314942bdab35c7f7dc6917c3e21d4b4ad Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 08:30:45 +0400 Subject: [PATCH 04/15] feat(mate): the mate walks onto the map with its owner MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The previous commits made a caught pet survive and appear in the pet window. It still could not be seen: nothing put it on a map. A mate now spawns beside its owner on arrival, is announced to everybody else on the map, and is taken away again when the owner leaves. Its health bar goes to the owner. The spawn shape is read off a capture rather than guessed: in 2 1506 445562 26 26 2 100 100 0 0 3 626114 1 0 -1 Ratufu^pirate^(Feu) 0 -1 ... Owner and GroupEffect=3 are what separate a mate from a map npc — without them the client draws it as scenery and will not let the owner command it. The byte after the name is 1 for every partner in the capture and 0 for every pet. pst 2 22687 0 100 100 24471 3100 0 0 0 The third field carries the mate type where a player's carries a party position. A newly caught pet joins the team when the pet slot is free, which is what catching one does in game: it walks out beside you rather than going into storage. A second pet waits, because a character keeps one pet and one partner out at a time. The mate is placed next to the character rather than on its stored square: that square belongs to whichever map it was last saved on, and reusing it here would put a pet through a wall. Not yet: the mate stands where it was put. Following the owner step by step needs the movement system and is its own change. Co-Authored-By: Claude Opus 5 --- .../Services/BattleService/CaptureService.cs | 7 +- .../MapChangeService/MapChangeService.cs | 17 ++++ .../Services/MateService/Mate.cs | 93 +++++++++++++++++++ .../Services/MateService/MateServiceTests.cs | 76 +++++++++++++++ 4 files changed, 192 insertions(+), 1 deletion(-) diff --git a/src/NosCore.GameObject/Services/BattleService/CaptureService.cs b/src/NosCore.GameObject/Services/BattleService/CaptureService.cs index cc185a61b..46945ecbd 100644 --- a/src/NosCore.GameObject/Services/BattleService/CaptureService.cs +++ b/src/NosCore.GameObject/Services/BattleService/CaptureService.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -96,6 +96,11 @@ await mateDao.TryInsertOrUpdateAsync(new MateDto Hp = monster.NpcMonster.MaxHp, Mp = monster.NpcMonster.MaxMp, IsSummonable = true, + // A pet you have just caught walks out beside you; it does not go into storage + // for you to fetch later. Only if the pet slot is already taken does it wait, + // because a character may keep one pet and one partner out at a time. + IsTeamMember = !mateDao.Where(s => s.CharacterId == character.CharacterId + && s.MateType == MateType.Pet && s.IsTeamMember)!.Any() }).ConfigureAwait(false); monster.Hp = 0; diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index 953fa17cb..4ba0a8f59 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -251,6 +251,20 @@ await Task.WhenAll(mapSessions.Select(async s => } } + // The mates arrive with their owner. They are placed beside the character rather + // than on their stored square: that square belongs to whichever map they were + // last saved on, and dropping a pet onto it here would put it through a wall. + var teamMates = character.Mates.Values.Where(s => s.IsTeamMember).ToList(); + foreach (var mate in teamMates) + { + mate.PositionX = (short)(character.PositionX + 1); + mate.PositionY = (short)(character.PositionY + 1); + } + + await newMapInstance.SendPacketsAsync(teamMates.Select(s => s.GenerateIn(accountLanguage))); + await session.SendPacketsAsync(teamMates.Select(s => s.GenerateCond())); + await session.SendPacketsAsync(teamMates.Select(s => s.GeneratePst())); + await messageBus.PublishAsync(new Messaging.Events.MapInstanceEnteredEvent(session, newMapInstance)); } catch (Exception ex) @@ -282,6 +296,9 @@ private async Task LeaveMapAsync(ClientSession session) var mapInstance = character.MapInstance; var channelId = session.Channel!.Id; await mapInstance.SendPacketAsync(outPacket, new EveryoneBut(channelId)); + await mapInstance.SendPacketsAsync(character.Mates.Values + .Where(s => s.IsTeamMember) + .Select(s => s.GenerateOut())); session.ClearPlayerEntity(); await session.SendPacketAsync(new MapOutPacket()); } diff --git a/src/NosCore.GameObject/Services/MateService/Mate.cs b/src/NosCore.GameObject/Services/MateService/Mate.cs index 64f41d2dd..eda516683 100644 --- a/src/NosCore.GameObject/Services/MateService/Mate.cs +++ b/src/NosCore.GameObject/Services/MateService/Mate.cs @@ -7,7 +7,13 @@ using NosCore.Data.Dto; using NosCore.Data.Enumerations.Character; using NosCore.Data.StaticEntities; +using NosCore.Packets.Enumerations; +using NosCore.Packets.ServerPackets.Entities; using NosCore.Packets.ServerPackets.Mates; +using NosCore.Packets.ServerPackets.Parcel; +using NosCore.Packets.ServerPackets.Player; +using NosCore.Packets.ServerPackets.Visibility; +using System.Globalization; using NosCore.Shared.Enumerations; namespace NosCore.GameObject.Services.MateService @@ -20,6 +26,15 @@ public class Mate : MateDto public byte PetSlot { get; set; } + /// + /// Where the mate is standing right now, which is not where it was stored. MapX and MapY + /// are the square it was last saved on; these two move with the owner. + /// + public short PositionX { get; set; } + + /// + public short PositionY { get; set; } + public int MaxHp => NpcMonster.MaxHp; public int MaxMp => NpcMonster.MaxMp; @@ -113,6 +128,84 @@ public ScnPacket GenerateScn(RegionType language) }; } + /// + /// The spawn packet. Owner and GroupEffect are what tell the client this is somebody's + /// mate rather than a map npc: + /// in 2 1506 445562 26 26 2 100 100 0 0 3 626114 1 0 -1 Ratufu^pirate^(Feu) 0 -1 ... + /// + public InPacket GenerateIn(RegionType language) + { + return new InPacket + { + VisualType = VisualType.Npc, + VNum = VNum.ToString(CultureInfo.InvariantCulture), + VisualId = MateTransportId, + PositionX = PositionX, + PositionY = PositionY, + Direction = Direction, + InNonPlayerSubPacket = new InNonPlayerSubPacket + { + InAliveSubPacket = new InAliveSubPacket + { + Hp = MaxHp > 0 ? (int)(Hp / (float)MaxHp * 100) : 100, + Mp = MaxMp > 0 ? (int)(Mp / (float)MaxMp * 100) : 100 + }, + Dialog = 0, + Faction = 0, + GroupEffect = 3, + Owner = CharacterId, + SpawnEffect = SpawnEffectType.NoEffect, + IsSitting = false, + Morph = (short?)(Skin != 0 ? Skin : -1), + Name = DisplayName(language), + Unknow1 = (byte)(MateType == MateType.Partner ? 1 : 0) + } + }; + } + + public OutPacket GenerateOut() + { + return new OutPacket + { + VisualType = VisualType.Npc, + VisualId = MateTransportId + }; + } + + /// + /// The health bar in the party frame. GroupOrder carries the mate type here, not a + /// position in the party: pst 2 22687 0 100 100 24471 3100 0 0 0 + /// + public PstPacket GeneratePst() + { + return new PstPacket + { + Type = VisualType.Npc, + VisualId = MateTransportId, + GroupOrder = (int)MateType, + HpLeft = MaxHp > 0 ? (int)(Hp / (float)MaxHp * 100) : 0, + MpLeft = MaxMp > 0 ? (int)(Mp / (float)MaxMp * 100) : 0, + HpLoad = MaxHp, + MpLoad = MaxMp, + Race = 0, + Gender = GenderType.Male, + Morph = 0, + BuffIds = null + }; + } + + public CondPacket GenerateCond() + { + return new CondPacket + { + VisualType = VisualType.Npc, + VisualId = MateTransportId, + NoAttack = false, + NoMove = false, + Speed = NpcMonster.Speed + }; + } + private string DisplayName(RegionType language) { var name = Name; diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs index bb0b3d06b..ef1830d65 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -15,6 +15,7 @@ using NosCore.Packets.ServerPackets.Mates; using NosCore.Shared.Enumerations; using System.Collections.Generic; +using NosCore.Packets.ServerPackets.Visibility; using System.Linq; using System.Threading.Tasks; using Mate = NosCore.GameObject.Services.MateService.Mate; @@ -170,5 +171,80 @@ public async Task ScpReportsTheExperienceTheCaptureReportsAsync() Assert.AreEqual(90L, packet.XpLoad); } + + [TestMethod] + public async Task TheSpawnPacketMarksTheMateAsBelongingToItsOwnerAsync() + { + // in 2 1506 445562 26 26 2 100 100 0 0 3 626114 1 0 -1 Ratufu^pirate^(Feu) 0 -1 ... + // Owner and GroupEffect are what separate a mate from a map npc; without them the + // client draws it as scenery and will not let the owner command it. + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, Hp = 78, Mp = 5 } + }, Creature(ChickenVNum, "Chicken", 156, 10)); + + var mate = (await service.LoadAsync(CharacterId))[0]; + mate.PositionX = 26; + mate.PositionY = 26; + var packet = mate.GenerateIn(RegionType.EN); + + Assert.AreEqual(VisualType.Npc, packet.VisualType); + Assert.AreEqual(CharacterId, packet.InNonPlayerSubPacket!.Owner); + Assert.AreEqual(3, packet.InNonPlayerSubPacket.GroupEffect); + Assert.AreEqual(mate.MateTransportId, packet.VisualId); + Assert.AreEqual(26, packet.PositionX); + Assert.AreEqual(50, packet.InNonPlayerSubPacket.InAliveSubPacket!.Hp, + "the spawn carries health as a percentage, not as points"); + } + + [TestMethod] + public async Task APartnerIsFlaggedDifferentlyFromAPetOnSpawnAsync() + { + // Both partners in the capture carry 1 after the name where every pet carries 0. + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet }, + new MateDto { MateId = 2, CharacterId = CharacterId, VNum = PartnerVNum, MateType = MateType.Partner } + }, Creature(ChickenVNum, "Chicken", 157, 10), Creature(PartnerVNum, "Bob", 870, 200)); + + var mates = await service.LoadAsync(CharacterId); + + Assert.AreEqual(0, mates.Single(s => s.MateType == MateType.Pet) + .GenerateIn(RegionType.EN).InNonPlayerSubPacket!.Unknow1); + Assert.AreEqual(1, mates.Single(s => s.MateType == MateType.Partner) + .GenerateIn(RegionType.EN).InNonPlayerSubPacket!.Unknow1); + } + + [TestMethod] + public async Task TheHealthBarCarriesTheMateTypeWhereAPlayerCarriesAPartyPositionAsync() + { + // pst 2 22687 0 100 100 24471 3100 0 0 0 — the third field is the mate type. + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = PartnerVNum, MateType = MateType.Partner, Hp = 435, Mp = 100 } + }, Creature(PartnerVNum, "Bob", 870, 200)); + + var packet = (await service.LoadAsync(CharacterId))[0].GeneratePst(); + + Assert.AreEqual(VisualType.Npc, packet.Type); + Assert.AreEqual((int)MateType.Partner, packet.GroupOrder); + Assert.AreEqual(50, packet.HpLeft); + Assert.AreEqual(870, packet.HpLoad); + } + + [TestMethod] + public async Task ADespawnNamesTheSameIdTheSpawnDidAsync() + { + // A mismatch here leaves the pet drawn on everybody else's screen for ever, and + // nothing throws. + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet } + }, Creature(ChickenVNum, "Chicken", 157, 10)); + + var mate = (await service.LoadAsync(CharacterId))[0]; + + Assert.AreEqual(mate.GenerateIn(RegionType.EN).VisualId, mate.GenerateOut().VisualId); + } } } From 59a1a102795b2325796faceb5d2096a51475892f Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 08:35:37 +0400 Subject: [PATCH 05/15] feat(mate): the mate keeps up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A mate that stays where it was summoned looks broken long before it looks unfinished, so it moves on the same event a character's own step already publishes rather than on a timer of its own. It takes the first walkable square around the owner, so a mate against a wall tucks in somewhere instead of refusing to move, and two mates never stack. With nothing free at all it stands on the owner — untidy, and better than being left behind on the far side of the map. Co-Authored-By: Claude Opus 5 --- .../Handlers/Mate/MateFollowHandler.cs | 86 +++++++++++++++++++ 1 file changed, 86 insertions(+) create mode 100644 src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs diff --git a/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs b/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs new file mode 100644 index 000000000..e503a5adc --- /dev/null +++ b/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs @@ -0,0 +1,86 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using JetBrains.Annotations; +using NosCore.GameObject.Ecs; +using NosCore.GameObject.Messaging.Events; +using NosCore.GameObject.Services.MapInstanceGenerationService; +using NosCore.Networking; +using NosCore.Packets.ServerPackets.Entities; +using NosCore.Shared.Enumerations; +using System.Collections.Generic; +using System.Linq; +using System.Threading.Tasks; + +namespace NosCore.GameObject.Messaging.Handlers.Mate +{ + // Keeps a character's mates at their heel. A mate that stays where it was summoned looks + // broken long before it looks unfinished, so it moves on the same event the character's own + // step publishes rather than on a timer of its own. + [UsedImplicitly] + public sealed class MateFollowHandler + { + // Where a mate stands relative to its owner, tried in order. The first walkable one + // wins, so a mate against a wall tucks in somewhere rather than refusing to move. + private static readonly (short X, short Y)[] Offsets = + [(1, 1), (-1, 1), (1, -1), (-1, -1), (1, 0), (-1, 0), (0, 1), (0, -1), (0, 0)]; + + [UsedImplicitly] + public async Task Handle(CharacterMovedEvent evt) + { + // Only a player has mates, and the event is declared on the wider interface. + if (evt.Character is not PlayerComponentBundle character) + { + return; + } + + var mates = character.Mates.Values.Where(s => s.IsTeamMember).ToList(); + if (mates.Count == 0) + { + return; + } + + var map = character.MapInstance; + var taken = new HashSet<(short, short)>(); + + foreach (var mate in mates) + { + var spot = Place(character, map, taken); + mate.PositionX = spot.X; + mate.PositionY = spot.Y; + taken.Add(spot); + + await map.SendPacketAsync(new MovePacket + { + VisualType = VisualType.Npc, + VisualEntityId = mate.MateTransportId, + MapX = mate.PositionX, + MapY = mate.PositionY, + Speed = mate.NpcMonster.Speed + }).ConfigureAwait(false); + } + } + + private static (short X, short Y) Place(PlayerComponentBundle character, + MapInstance map, + HashSet<(short, short)> taken) + { + foreach (var offset in Offsets) + { + var x = (short)(character.PositionX + offset.X); + var y = (short)(character.PositionY + offset.Y); + if (!taken.Contains((x, y)) && map.Map.IsWalkable(x, y)) + { + return (x, y); + } + } + + // Nothing free anywhere around: stand on the owner. Two things in one square is + // untidy, and better than a mate left behind on the far side of the map. + return (character.PositionX, character.PositionY); + } + } +} From fcb5d4fb92e0aae360cf8d172446c09c4e73d3a7 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 08:39:47 +0400 Subject: [PATCH 06/15] fix(mate): three from review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit p_clear wipes one panel that holds both the party and the mate list, and the party burst was straddling it — so pinit and pst were being cleared on every game start. Both bursts now follow it, in the order the capture shows: p_clear, sc packets, sc_p_stc, pinit. A player walking onto a populated map was told about the characters already there but not about their mates, so every pet on the map was invisible to them. Two rows can claim the same mate slot — two captures racing, or a database edited by hand — and the second would spawn on top of the first with nothing raised anywhere. The reader now decides: the first row keeps the slot and the rest stay in the list. That is cheaper than a transaction and it also repairs a database that is already inconsistent. Co-Authored-By: Claude Opus 5 --- .../MapChangeService/MapChangeService.cs | 6 ++++ .../Services/MateService/MateService.cs | 14 ++++++++ .../Game/GameStartPacketHandler.cs | 18 +++++----- .../Services/MateService/MateServiceTests.cs | 33 +++++++++++++++++++ 4 files changed, 63 insertions(+), 8 deletions(-) diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index 4ba0a8f59..41fe74599 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -228,6 +228,12 @@ await Task.WhenAll(mapSessions.Select(async s => : string.Empty; await session.SendPacketAsync(otherCharacter.GenerateIn(prefix)); + // And whatever is at their heel. Announcing only the arriving character's + // mates would leave every pet already on the map invisible to them. + await session.SendPacketsAsync(otherCharacter.Mates.Values + .Where(m => m.IsTeamMember) + .Select(m => m.GenerateIn(accountLanguage))); + var shop = otherCharacter.Shop; if (shop != null) { diff --git a/src/NosCore.GameObject/Services/MateService/MateService.cs b/src/NosCore.GameObject/Services/MateService/MateService.cs index 9a990cc0c..f565730dc 100644 --- a/src/NosCore.GameObject/Services/MateService/MateService.cs +++ b/src/NosCore.GameObject/Services/MateService/MateService.cs @@ -45,9 +45,23 @@ public Task> LoadAsync(long characterId) foreach (var group in mates.GroupBy(s => s.MateType)) { byte slot = 0; + var alreadyOut = false; foreach (var mate in group) { mate.PetSlot = slot++; + + // A character keeps one pet and one partner out at a time. Two rows can + // claim the slot — two captures racing, or a database edited by hand — and + // the second would spawn on top of the first with no error anywhere. The + // reader decides, so a bad row costs a mate that stays in the list rather + // than a broken map. + if (!mate.IsTeamMember) + { + continue; + } + + mate.IsTeamMember = !alreadyOut; + alreadyOut = true; } } diff --git a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs index 3d0753eb6..4403e6c4a 100644 --- a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs @@ -128,13 +128,6 @@ await session.SendPacketAsync(new SayiPacket session.Character.LoadExpensions(); await session.SendPacketAsync(session.Character.GenerateExts(worldConfiguration)); // Session.SendPacket(Session.Character.GenerateMlinfo()); - await session.SendPacketAsync(new PclearPacket()); - - // Group init even for solo players — the client expects pinit + a self-row pst - // so its party UI is in a known state for later joins/leaves. - await session.SendPacketAsync(session.Character.Group.GeneratePinit()); - await session.SendPacketsAsync(session.Character.Group.GeneratePst()); - // Session.SendPacket("zzim"); await session.SendPacketAsync(new TwkPacket(session.Account.Name, session.Character.Name) { @@ -150,10 +143,19 @@ await session.SendPacketAsync(new TwkPacket(session.Account.Name, session.Charac // // sqst bf // Session.SendPacket("act6"); // Session.SendPacket(Session.Character.GenerateFaction()); + // p_clear wipes one panel that holds both the party and the mate list, so the two + // bursts have to follow it rather than straddle it. The capture puts them in this + // order: p_clear, the sc packets, sc_p_stc, then pinit. await session.SendPacketAsync(new PclearPacket()); await session.SendPacketsAsync(MateService.GenerateScPackets(session.Character.Mates.Values, session.Character.AccountLanguage)); - await session.SendPacketAsync(new ScPStcPacket { MaxMateCountTenths = 0 }); + + // Party init even for a solo player: the client wants pinit and a self-row pst so + // its party frame is in a known state for later joins and leaves. + await session.SendPacketAsync(session.Character.Group.GeneratePinit()); + await session.SendPacketsAsync(session.Character.Group.GeneratePst()); + await session.SendPacketsAsync(session.Character.Mates.Values + .Where(s => s.IsTeamMember).Select(s => s.GeneratePst())); // Session.Character.GenerateStartupInventory(); await session.SendPacketAsync(session.Character.GenerateGold()); diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs index ef1830d65..1bcbc5b4b 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -246,5 +246,38 @@ public async Task ADespawnNamesTheSameIdTheSpawnDidAsync() Assert.AreEqual(mate.GenerateIn(RegionType.EN).VisualId, mate.GenerateOut().VisualId); } + + [TestMethod] + public async Task OnlyOneMateOfEachTypeIsEverOutAsync() + { + // Two rows can claim the slot — two captures racing, or a database edited by hand — + // and the second would spawn on top of the first with nothing raised anywhere. + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, IsTeamMember = true }, + new MateDto { MateId = 2, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, IsTeamMember = true }, + new MateDto { MateId = 3, CharacterId = CharacterId, VNum = PartnerVNum, MateType = MateType.Partner, IsTeamMember = true } + }, Creature(ChickenVNum, "Chicken", 157, 10), Creature(PartnerVNum, "Bob", 870, 200)); + + var mates = await service.LoadAsync(CharacterId); + + Assert.AreEqual(1, mates.Count(s => s.MateType == MateType.Pet && s.IsTeamMember)); + Assert.AreEqual(1, mates.Count(s => s.MateType == MateType.Partner && s.IsTeamMember), + "a pet and a partner are two different slots and both may be out"); + Assert.AreEqual(1L, mates.Single(s => s.MateType == MateType.Pet && s.IsTeamMember).MateId, + "the first row keeps the slot, so which mate is out does not change between logins"); + } + + [TestMethod] + public async Task AMateThatIsNotOutStaysInTheListAsync() + { + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, IsTeamMember = true }, + new MateDto { MateId = 2, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, IsTeamMember = true } + }, Creature(ChickenVNum, "Chicken", 157, 10)); + + Assert.AreEqual(2, (await service.LoadAsync(CharacterId)).Count); + } } } From caa103b813f43d0c2b34db5b25630ad8b13fc44f Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 08:50:30 +0400 Subject: [PATCH 07/15] =?UTF-8?q?fix(mate):=20two=20more=20from=20review?= =?UTF-8?q?=20=E2=80=94=20hidden=20owners,=20and=20one=20square=20each?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A hidden owner's mates were still broadcast to the map. A visible pet carrying an Owner field announces the character it belongs to, so invisibility leaked through the pet. Their spawns and their moves now go to the owner alone while they are hidden. Both mates were being placed on the same fixed offset from the owner, which also happened to be a square that might be a wall or off the map. The placement the follow already used is now shared: each mate takes its own walkable square, and the owner's own square is the fallback when nothing is free. Co-Authored-By: Claude Opus 5 --- .../Handlers/Mate/MateFollowHandler.cs | 43 ++++---------- .../MapChangeService/MapChangeService.cs | 23 +++++--- .../Services/MateService/MatePlacement.cs | 58 +++++++++++++++++++ .../Services/MateService/MateServiceTests.cs | 55 ++++++++++++++++++ 4 files changed, 141 insertions(+), 38 deletions(-) create mode 100644 src/NosCore.GameObject/Services/MateService/MatePlacement.cs diff --git a/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs b/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs index e503a5adc..5704331e9 100644 --- a/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs +++ b/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs @@ -6,12 +6,12 @@ using JetBrains.Annotations; using NosCore.GameObject.Ecs; +using NosCore.GameObject.Ecs.Extensions; using NosCore.GameObject.Messaging.Events; -using NosCore.GameObject.Services.MapInstanceGenerationService; +using NosCore.GameObject.Services.MateService; using NosCore.Networking; using NosCore.Packets.ServerPackets.Entities; using NosCore.Shared.Enumerations; -using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; @@ -23,11 +23,6 @@ namespace NosCore.GameObject.Messaging.Handlers.Mate [UsedImplicitly] public sealed class MateFollowHandler { - // Where a mate stands relative to its owner, tried in order. The first walkable one - // wins, so a mate against a wall tucks in somewhere rather than refusing to move. - private static readonly (short X, short Y)[] Offsets = - [(1, 1), (-1, 1), (1, -1), (-1, -1), (1, 0), (-1, 0), (0, 1), (0, -1), (0, 0)]; - [UsedImplicitly] public async Task Handle(CharacterMovedEvent evt) { @@ -44,43 +39,29 @@ public async Task Handle(CharacterMovedEvent evt) } var map = character.MapInstance; - var taken = new HashSet<(short, short)>(); + MatePlacement.Arrange(character.PositionX, character.PositionY, map.Map, mates); foreach (var mate in mates) { - var spot = Place(character, map, taken); - mate.PositionX = spot.X; - mate.PositionY = spot.Y; - taken.Add(spot); - - await map.SendPacketAsync(new MovePacket + var move = new MovePacket { VisualType = VisualType.Npc, VisualEntityId = mate.MateTransportId, MapX = mate.PositionX, MapY = mate.PositionY, Speed = mate.NpcMonster.Speed - }).ConfigureAwait(false); - } - } + }; - private static (short X, short Y) Place(PlayerComponentBundle character, - MapInstance map, - HashSet<(short, short)> taken) - { - foreach (var offset in Offsets) - { - var x = (short)(character.PositionX + offset.X); - var y = (short)(character.PositionY + offset.Y); - if (!taken.Contains((x, y)) && map.Map.IsWalkable(x, y)) + // A hidden owner's mates are still theirs: broadcasting them would draw the + // character back onto everybody's screen, and the spawn packet even names them. + if (character.Invisible) { - return (x, y); + await character.SendPacketAsync(move).ConfigureAwait(false); + continue; } - } - // Nothing free anywhere around: stand on the owner. Two things in one square is - // untidy, and better than a mate left behind on the far side of the map. - return (character.PositionX, character.PositionY); + await map.SendPacketAsync(move).ConfigureAwait(false); + } } } } diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index 41fe74599..ad2e092a8 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -15,6 +15,7 @@ using NosCore.GameObject.Services.BroadcastService; using NosCore.GameObject.Services.ItemGenerationService.Item; using NosCore.GameObject.Services.MapInstanceAccessService; +using NosCore.GameObject.Services.MateService; using NosCore.GameObject.Services.MapInstanceGenerationService; using NosCore.GameObject.Services.MinilandService; using NosCore.Networking; @@ -257,17 +258,25 @@ await session.SendPacketsAsync(otherCharacter.Mates.Values } } - // The mates arrive with their owner. They are placed beside the character rather - // than on their stored square: that square belongs to whichever map they were - // last saved on, and dropping a pet onto it here would put it through a wall. + // The mates arrive with their owner, each on its own walkable square: their + // stored square belongs to whichever map they were last saved on, and reusing + // it here would put a pet through a wall. var teamMates = character.Mates.Values.Where(s => s.IsTeamMember).ToList(); - foreach (var mate in teamMates) + MatePlacement.Arrange(character.PositionX, character.PositionY, + newMapInstance.Map, teamMates); + + var mateSpawns = teamMates.Select(s => s.GenerateIn(accountLanguage)).ToList(); + if (invisible) + { + // A hidden owner keeps their mates to themselves: a visible pet with an + // Owner field on it announces the character it belongs to. + await session.SendPacketsAsync(mateSpawns); + } + else { - mate.PositionX = (short)(character.PositionX + 1); - mate.PositionY = (short)(character.PositionY + 1); + await newMapInstance.SendPacketsAsync(mateSpawns); } - await newMapInstance.SendPacketsAsync(teamMates.Select(s => s.GenerateIn(accountLanguage))); await session.SendPacketsAsync(teamMates.Select(s => s.GenerateCond())); await session.SendPacketsAsync(teamMates.Select(s => s.GeneratePst())); diff --git a/src/NosCore.GameObject/Services/MateService/MatePlacement.cs b/src/NosCore.GameObject/Services/MateService/MatePlacement.cs new file mode 100644 index 000000000..76f27c395 --- /dev/null +++ b/src/NosCore.GameObject/Services/MateService/MatePlacement.cs @@ -0,0 +1,58 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using NosCore.GameObject.Map; +using System.Collections.Generic; + +namespace NosCore.GameObject.Services.MateService +{ + /// + /// Puts a character's mates around them. + /// + public static class MatePlacement + { + // Tried in order, so the first walkable square wins and a mate against a wall tucks in + // somewhere rather than standing in it. + private static readonly (short X, short Y)[] Offsets = + [(1, 1), (-1, 1), (1, -1), (-1, -1), (1, 0), (-1, 0), (0, 1), (0, -1)]; + + /// + /// Places every mate on its own walkable square around the owner. + /// + /// + /// A character can have a pet and a partner out at once, so squares are reserved as they + /// are handed out; giving both the same offset would stack them. With nothing free the + /// mate stands on the owner — untidy, and better than being left across the map. + /// + public static void Arrange(short ownerX, short ownerY, Map.Map map, IEnumerable mates) + { + var taken = new HashSet<(short, short)>(); + foreach (var mate in mates) + { + var spot = Free(ownerX, ownerY, map, taken); + mate.PositionX = spot.X; + mate.PositionY = spot.Y; + taken.Add(spot); + } + } + + private static (short X, short Y) Free(short ownerX, short ownerY, Map.Map map, + HashSet<(short, short)> taken) + { + foreach (var offset in Offsets) + { + var x = (short)(ownerX + offset.X); + var y = (short)(ownerY + offset.Y); + if (!taken.Contains((x, y)) && map.IsWalkable(x, y)) + { + return (x, y); + } + } + + return (ownerX, ownerY); + } + } +} diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs index 1bcbc5b4b..79ef6d0b9 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -19,6 +19,7 @@ using System.Linq; using System.Threading.Tasks; using Mate = NosCore.GameObject.Services.MateService.Mate; +using MatePlacement = NosCore.GameObject.Services.MateService.MatePlacement; using MateServiceImpl = NosCore.GameObject.Services.MateService.MateService; namespace NosCore.GameObject.Tests.Services.MateService @@ -279,5 +280,59 @@ public async Task AMateThatIsNotOutStaysInTheListAsync() Assert.AreEqual(2, (await service.LoadAsync(CharacterId)).Count); } + + private static GameObject.Map.Map OpenGround() + { + return new GameObject.Map.Map + { + MapId = 1, + NameI18NKey = "openGround", + Data = [8, 0, 8, 0, .. new byte[64]] + }; + } + + [TestMethod] + public async Task TwoMatesNeverStandOnTheSameSquareAsync() + { + // A character can have a pet and a partner out at once. Giving both the same offset + // stacks them, and nothing complains. + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, IsTeamMember = true }, + new MateDto { MateId = 2, CharacterId = CharacterId, VNum = PartnerVNum, MateType = MateType.Partner, IsTeamMember = true } + }, Creature(ChickenVNum, "Chicken", 157, 10), Creature(PartnerVNum, "Bob", 870, 200)); + + var mates = await service.LoadAsync(CharacterId); + MatePlacement.Arrange(4, 4, OpenGround(), mates); + + Assert.AreEqual(2, mates.Select(s => (s.PositionX, s.PositionY)).Distinct().Count()); + } + + [TestMethod] + public async Task AMateIsNeverPlacedInsideAWallAsync() + { + var walled = new GameObject.Map.Map + { + MapId = 1, + NameI18NKey = "walled", + // Four by four, everything solid but the two squares on the top row. + Data = [4, 0, 4, 0, + 0, 0, 1, 1, + 1, 1, 1, 1, + 1, 1, 1, 1, + 1, 1, 1, 1] + }; + var service = Build(new[] + { + new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, IsTeamMember = true } + }, Creature(ChickenVNum, "Chicken", 157, 10)); + + var mates = await service.LoadAsync(CharacterId); + MatePlacement.Arrange(0, 0, walled, mates); + + var mate = mates[0]; + Assert.IsTrue(walled.IsWalkable(mate.PositionX, mate.PositionY), + $"placed at {mate.PositionX},{mate.PositionY}, which is not walkable"); + } } } From e297afc93597bb28ebbe095cb552fd729864655d Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 12:56:23 +0400 Subject: [PATCH 08/15] feat(mate): a mate that can fight MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Answering my own question from the review rather than leaving the PR waiting: a mate becomes an ECS entity, with the component set a monster has. That is what lets it go through IBattleService.Hit like anything else that fights. The skill resolver already branches on INonPlayableEntity and reads the creature off NpcMonster, so a mate resolves its basic attack exactly as a monster does — no second damage path to keep in step, and no second notion of "combatant" in the codebase. It also unblocks the BCard subtypes that only mean something with a mate in the fight. The entity's life is the map's: created on arrival, destroyed on leaving, position written on every step. Leaving it behind would strand a mate in a world nobody is in, and moving only the packet would leave it visibly in one place and actually in another — which is what a monster picking a target would read. u_pet checks that the mate belongs to the character asking and is actually out, because trusting the id in the packet would let a client drive somebody else's pet. If you would rather mates stayed out of the ECS, say so and it comes back out: the fold of effects is the part that matters and it does not depend on the container. Co-Authored-By: Claude Opus 5 --- .../Ecs/Components/MateStateComponent.cs | 15 ++++ src/NosCore.GameObject/Ecs/MapWorld.cs | 31 ++++++++ .../Ecs/MateComponentBundle.cs | 34 ++++++++ .../Handlers/Mate/MateFollowHandler.cs | 9 +++ .../MapChangeService/MapChangeService.cs | 26 ++++++- .../Services/MateService/Mate.cs | 11 +++ .../Mates/UpetPacketHandler.cs | 78 +++++++++++++++++++ 7 files changed, 201 insertions(+), 3 deletions(-) create mode 100644 src/NosCore.GameObject/Ecs/Components/MateStateComponent.cs create mode 100644 src/NosCore.GameObject/Ecs/MateComponentBundle.cs create mode 100644 src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs diff --git a/src/NosCore.GameObject/Ecs/Components/MateStateComponent.cs b/src/NosCore.GameObject/Ecs/Components/MateStateComponent.cs new file mode 100644 index 000000000..dc9dc1bde --- /dev/null +++ b/src/NosCore.GameObject/Ecs/Components/MateStateComponent.cs @@ -0,0 +1,15 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using NosCore.GameObject.Services.MateService; + +namespace NosCore.GameObject.Ecs.Components; + +/// +/// What makes an entity somebody's mate rather than a monster: the stored row it came from and +/// the character it belongs to. +/// +public record struct MateStateComponent(Mate Mate, long OwnerId); diff --git a/src/NosCore.GameObject/Ecs/MapWorld.cs b/src/NosCore.GameObject/Ecs/MapWorld.cs index 12f307cf0..7db907c51 100644 --- a/src/NosCore.GameObject/Ecs/MapWorld.cs +++ b/src/NosCore.GameObject/Ecs/MapWorld.cs @@ -137,6 +137,37 @@ public Entity CreateNpc( return entity; } + public Entity CreateMate( + int visualId, + Services.MateService.Mate mate, + MapInstance mapInstance, + short positionX, + short positionY, + byte direction) + { + var now = SystemClock.Instance.GetCurrentInstant(); + return World.Create( + new EntityIdentityComponent(visualId, VisualType.Npc, mate.CharacterId), + new HealthComponent(mate.Hp, mate.MaxHp, true), + new ManaComponent(mate.Mp, mate.MaxMp), + new PositionComponent(positionX, positionY, direction, mapInstance.MapInstanceId), + new VisualComponent(0, 0, 0, 0, false, false, false), + new NpcDataComponent(mate.VNum, mate.NpcMonster.Race, mate.Level, 0, mate.NpcMonster.Speed, 10), + // A mate never wanders and is never hostile on its own: it goes where its owner + // goes and hits what its owner points at. + new SpawnComponent(positionX, positionY, false, false), + new EffectComponent(0, 0), + new TimingComponent(now, now), + new NpcStateComponent(mate.NpcMonster, mapInstance, new SemaphoreSlim(1, 1), + new ConcurrentDictionary(), null, null, null, + new Dictionary>(), null, false), + new BuffStateComponent(new ConcurrentDictionary()), + new AggroComponent(VisualType.Object, 0, 0, Instant.MinValue), + new SkillCooldownComponent(new ConcurrentDictionary()), + new MateStateComponent(mate, mate.CharacterId) + ); + } + public Entity CreateMapItem( long visualId, short vNum, diff --git a/src/NosCore.GameObject/Ecs/MateComponentBundle.cs b/src/NosCore.GameObject/Ecs/MateComponentBundle.cs new file mode 100644 index 000000000..a58f42586 --- /dev/null +++ b/src/NosCore.GameObject/Ecs/MateComponentBundle.cs @@ -0,0 +1,34 @@ +using NosCore.GameObject.Ecs.Attributes; +using NosCore.GameObject.Ecs.Components; +using NosCore.GameObject.Ecs.Interfaces; + +namespace NosCore.GameObject.Ecs; + +// A mate on the map is a monster that belongs to somebody: it stands, takes hits, carries buffs +// and cooldowns, and dies. Giving it the monster's component set rather than a set of its own +// is what lets the battle service treat it as a combatant without a second notion of one. +[ComponentBundle( + typeof(EntityIdentityComponent), + typeof(HealthComponent), + typeof(ManaComponent), + typeof(PositionComponent), + typeof(VisualComponent), + typeof(NpcDataComponent), + typeof(SpawnComponent), + typeof(EffectComponent), + typeof(TimingComponent), + typeof(NpcStateComponent), + typeof(BuffStateComponent), + typeof(AggroComponent), + typeof(SkillCooldownComponent), + typeof(MateStateComponent) +)] +public readonly partial struct MateComponentBundle : INonPlayableEntity +{ + public Arch.Core.Entity Handle => Entity; + + // A monster answers with the square it spawned on; a mate follows its owner, so the live + // position is the only meaningful one. + public short MapX => PositionX; + public short MapY => PositionY; +} diff --git a/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs b/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs index 5704331e9..89036dd12 100644 --- a/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs +++ b/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs @@ -43,6 +43,15 @@ public async Task Handle(CharacterMovedEvent evt) foreach (var mate in mates) { + // The entity carries the position everything else reads — a monster deciding + // whom to hit, a skill deciding what is in range. Moving only the packet would + // leave the mate visibly in one place and actually in another. + if (mate.Entity is { } handle) + { + handle.PositionX = mate.PositionX; + handle.PositionY = mate.PositionY; + } + var move = new MovePacket { VisualType = VisualType.Npc, diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index ad2e092a8..18e5eafc8 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -265,6 +265,16 @@ await session.SendPacketsAsync(otherCharacter.Mates.Values MatePlacement.Arrange(character.PositionX, character.PositionY, newMapInstance.Map, teamMates); + // The mate becomes a real entity on the map it is standing on: that is what lets + // it be targeted, buffed and killed like anything else that fights. + foreach (var mate in teamMates) + { + var handle = newMapInstance.EcsWorld.CreateMate( + (int)mate.MateTransportId, mate, newMapInstance, + mate.PositionX, mate.PositionY, 2); + mate.Entity = new Ecs.MateComponentBundle(handle, newMapInstance.EcsWorld); + } + var mateSpawns = teamMates.Select(s => s.GenerateIn(accountLanguage)).ToList(); if (invisible) { @@ -311,9 +321,19 @@ private async Task LeaveMapAsync(ClientSession session) var mapInstance = character.MapInstance; var channelId = session.Channel!.Id; await mapInstance.SendPacketAsync(outPacket, new EveryoneBut(channelId)); - await mapInstance.SendPacketsAsync(character.Mates.Values - .Where(s => s.IsTeamMember) - .Select(s => s.GenerateOut())); + var leaving = character.Mates.Values.Where(s => s.IsTeamMember).ToList(); + await mapInstance.SendPacketsAsync(leaving.Select(s => s.GenerateOut())); + + // The entity belongs to the map being left, so it goes with it. A new one is made + // on arrival; keeping this one would leave a mate standing in a world nobody is in. + foreach (var mate in leaving) + { + if (mate.Entity is { } handle) + { + mapInstance.EcsWorld.DestroyEntity(handle.Handle); + mate.Entity = null; + } + } session.ClearPlayerEntity(); await session.SendPacketAsync(new MapOutPacket()); } diff --git a/src/NosCore.GameObject/Services/MateService/Mate.cs b/src/NosCore.GameObject/Services/MateService/Mate.cs index eda516683..5e30eb069 100644 --- a/src/NosCore.GameObject/Services/MateService/Mate.cs +++ b/src/NosCore.GameObject/Services/MateService/Mate.cs @@ -35,6 +35,17 @@ public class Mate : MateDto /// public short PositionY { get; set; } + /// + /// The mate's place in the world while it is out, or null while it is not. + /// + /// + /// A mate that can be hit has to be an entity like any other combatant — the battle + /// service asks for an Arch handle, and giving mates a second notion of "thing that + /// fights" would mean maintaining two. The handle lives here rather than in a registry + /// because the mate is already the thing everyone holds. + /// + public Ecs.MateComponentBundle? Entity { get; set; } + public int MaxHp => NpcMonster.MaxHp; public int MaxMp => NpcMonster.MaxMp; diff --git a/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs b/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs new file mode 100644 index 000000000..9fb1707b4 --- /dev/null +++ b/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs @@ -0,0 +1,78 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using Microsoft.Extensions.Logging; +using NosCore.Data.Enumerations.I18N; +using NosCore.GameObject.Ecs.Interfaces; +using NosCore.GameObject.Infastructure; +using NosCore.GameObject.Networking.ClientSession; +using NosCore.GameObject.Services.BattleService; +using NosCore.GameObject.Services.BroadcastService; +using NosCore.Packets.ClientPackets.Mates; +using NosCore.Shared.Enumerations; +using NosCore.Shared.I18N; +using System.Threading.Tasks; + +namespace NosCore.PacketHandlers.Mates +{ + // A pet attacking what its owner points it at. The mate goes through the same + // IBattleService.Hit as everything else that fights: it is an entity on the map with the + // same components a monster has, so the skill resolver already treats it as one and there + // is no second damage path to keep in step. + public class UpetPacketHandler( + IBattleService battleService, + ISessionRegistry sessionRegistry, + ILogger logger, + ILogLanguageLocalizer logLanguage) + : PacketHandler, IWorldPacketHandler + { + public override async Task ExecuteAsync(UpetPacket packet, ClientSession session) + { + var character = session.Character; + + // Only the owner commands the mate, and only one that is actually out. Trusting the + // id would let a client drive somebody else's pet. + if (!character.Mates.TryGetValue(packet.MateTransportId, out var mate) + || !mate.IsTeamMember + || mate.Entity is not { } attacker) + { + return; + } + + var target = ResolveTarget(packet, session); + if (target == null) + { + return; + } + + // Cast id zero is the creature's own basic attack: a mate has no learned skills, so + // the resolver reads it off the NpcMonster exactly as it does for a monster. + await battleService.Hit(attacker, target, new HitArguments { SkillId = 0 }) + .ConfigureAwait(false); + } + + private IAliveEntity? ResolveTarget(UpetPacket packet, ClientSession session) + { + var map = session.Character.MapInstance; + IAliveEntity? candidate = packet.TargetType switch + { + VisualType.Player => sessionRegistry.TryGetCharacter(s => s.VisualId == packet.TargetId, out var player) + ? player + : null, + VisualType.Npc => map.FindNpc(s => s.VisualId == packet.TargetId), + VisualType.Monster => map.FindMonster(s => s.VisualId == packet.TargetId), + _ => null + }; + + if (candidate == null) + { + logger.LogError(logLanguage[LogLanguageKey.VISUALENTITY_DOES_NOT_EXIST]); + } + + return candidate; + } + } +} From 8245100933932f232b1234bec0a6fe0e60d542df Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 13:06:10 +0400 Subject: [PATCH 09/15] fix(mate): a hidden player's mates stay hidden from newcomers too The mirror of the leak already fixed: the arriving character's own mates were gated on their invisibility, but the loop telling them about the players already on the map was not gated on those players'. A pet's spawn packet names its owner, so an invisible character was announced by somebody else's screen. Co-Authored-By: Claude Opus 5 --- .../Services/MapChangeService/MapChangeService.cs | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index 18e5eafc8..21c327b63 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -229,11 +229,14 @@ await Task.WhenAll(mapSessions.Select(async s => : string.Empty; await session.SendPacketAsync(otherCharacter.GenerateIn(prefix)); - // And whatever is at their heel. Announcing only the arriving character's - // mates would leave every pet already on the map invisible to them. - await session.SendPacketsAsync(otherCharacter.Mates.Values - .Where(m => m.IsTeamMember) - .Select(m => m.GenerateIn(accountLanguage))); + // And whatever is at their heel — unless they are hidden, in which case the + // pet would announce them: its spawn packet names its owner. + if (!otherCharacter.Invisible) + { + await session.SendPacketsAsync(otherCharacter.Mates.Values + .Where(m => m.IsTeamMember) + .Select(m => m.GenerateIn(accountLanguage))); + } var shop = otherCharacter.Shop; if (shop != null) From c112a137ce74e01381cca45a83b7a12f0794d6f6 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 13:37:51 +0400 Subject: [PATCH 10/15] chore(mate): take the experience curve from NosCore.Algorithm MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The local MateXpTable and its tests are gone: NosCore.Algorithm 2.1.0 ships IMateExperienceService, which is where you said it belonged. The value is written when the mate is loaded rather than computed on the object — a data object has no business resolving a service, and the number only changes on level-up. The ten capture observations that pinned the divisors live with the curve now, as approval tables in that repository. Co-Authored-By: Claude Opus 5 --- .../Services/MateService/Mate.cs | 6 +- .../Services/MateService/MateService.cs | 7 +- .../Services/MateService/MateXpTable.cs | 73 ------------------- .../Services/MateService/MateServiceTests.cs | 3 +- .../Services/MateService/MateXpTableTests.cs | 71 ------------------ test/NosCore.Tests.Shared/TestHelpers.cs | 1 + 6 files changed, 14 insertions(+), 147 deletions(-) delete mode 100644 src/NosCore.GameObject/Services/MateService/MateXpTable.cs delete mode 100644 test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs diff --git a/src/NosCore.GameObject/Services/MateService/Mate.cs b/src/NosCore.GameObject/Services/MateService/Mate.cs index 5e30eb069..4ce8ca32d 100644 --- a/src/NosCore.GameObject/Services/MateService/Mate.cs +++ b/src/NosCore.GameObject/Services/MateService/Mate.cs @@ -50,7 +50,11 @@ public class Mate : MateDto public int MaxMp => NpcMonster.MaxMp; - public long XpLoad => MateXpTable.RequiredXp(Level, MateType); + /// + /// Written when the mate is loaded rather than computed here: the curve lives in + /// NosCore.Algorithm, and a data object has no business resolving a service. + /// + public long XpLoad { get; set; } public ScpPacket GenerateScp(RegionType language) { diff --git a/src/NosCore.GameObject/Services/MateService/MateService.cs b/src/NosCore.GameObject/Services/MateService/MateService.cs index f565730dc..0447bc813 100644 --- a/src/NosCore.GameObject/Services/MateService/MateService.cs +++ b/src/NosCore.GameObject/Services/MateService/MateService.cs @@ -6,6 +6,7 @@ using Mapster; using Microsoft.Extensions.Logging; +using NosCore.Algorithm.MateExperienceService; using NosCore.Core.Services.IdService; using NosCore.Dao.Interfaces; using NosCore.Data.Dto; @@ -19,7 +20,8 @@ namespace NosCore.GameObject.Services.MateService { public class MateService(IDao mateDao, List npcMonsters, - IIdService mateIdService, ILogger logger) : IMateService + IIdService mateIdService, IMateExperienceService mateExperienceService, + ILogger logger) : IMateService { public Task> LoadAsync(long characterId) { @@ -39,6 +41,9 @@ public Task> LoadAsync(long characterId) var mate = row.Adapt(); mate.NpcMonster = npcMonster; mate.MateTransportId = mateIdService.GetNextId(); + mate.XpLoad = mate.MateType == MateType.Pet + ? mateExperienceService.GetPetExperience(mate.Level) + : mateExperienceService.GetPartnerExperience(mate.Level); mates.Add(mate); } diff --git a/src/NosCore.GameObject/Services/MateService/MateXpTable.cs b/src/NosCore.GameObject/Services/MateService/MateXpTable.cs deleted file mode 100644 index 65ceda251..000000000 --- a/src/NosCore.GameObject/Services/MateService/MateXpTable.cs +++ /dev/null @@ -1,73 +0,0 @@ -// __ _ __ __ ___ __ ___ ___ -// | \| |/__\ /' _/ / _//__\| _ \ __| -// | | ' | \/ |`._`.| \_| \/ | v / _| -// |_|\__|\__/ |___/ \__/\__/|_|_\___| -// - -using NosCore.Data.Enumerations.Character; - -namespace NosCore.GameObject.Services.MateService -{ - public static class MateXpTable - { - private static readonly long[] RawCurve = BuildRawCurve(); - - public const byte MaxDescribedLevel = 255; - - public static long RequiredXp(byte level, MateType mateType) - { - var index = level < 1 ? 0 : level - 1; - if (index >= RawCurve.Length) - { - index = RawCurve.Length - 1; - } - - return RawCurve[index] / (mateType == MateType.Pet ? 20 : 5); - } - - private static long[] BuildRawCurve() - { - var curve = new long[MaxDescribedLevel + 1]; - var step = new double[curve.Length]; - var factor = 1d; - - step[0] = 540; - step[1] = 960; - curve[0] = 300; - - for (var i = 2; i < step.Length; i++) - { - step[i] = step[i - 1] + 420 + 120 * (i - 1); - } - - for (var i = 1; i < curve.Length; i++) - { - if (i < 79) - { - factor = i switch - { - 14 => 6 / 3d, - 39 => 19 / 3d, - 59 => 70 / 3d, - _ => factor - }; - - curve[i] = (long)(curve[i - 1] + factor * step[i - 1]); - continue; - } - - factor = i switch - { - 79 => 5000, - 82 => 9000, - 84 => 13000, - _ => factor - }; - - curve[i] = (long)(curve[i - 1] + factor * (i + 2) * (i + 2)); - } - - return curve; - } - } -} diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs index 79ef6d0b9..0abd0d96e 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -7,6 +7,7 @@ using Microsoft.Extensions.Logging.Abstractions; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; +using NosCore.Algorithm.MateExperienceService; using NosCore.Core.Services.IdService; using NosCore.Dao.Interfaces; using NosCore.Data.Dto; @@ -53,7 +54,7 @@ private static MateServiceImpl Build(IEnumerable rows, params NpcMonste rows.Where(predicate.Compile())); return new MateServiceImpl(dao.Object, creatures.ToList(), - new IdService(2000000), NullLogger.Instance); + new IdService(2000000), new MateExperienceService(), NullLogger.Instance); } [TestMethod] diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs deleted file mode 100644 index 5e272cd0f..000000000 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateXpTableTests.cs +++ /dev/null @@ -1,71 +0,0 @@ -// __ _ __ __ ___ __ ___ ___ -// | \| |/__\ /' _/ / _//__\| _ \ __| -// | | ' | \/ |`._`.| \_| \/ | v / _| -// |_|\__|\__/ |___/ \__/\__/|_|_\___| -// - -using Microsoft.VisualStudio.TestTools.UnitTesting; -using NosCore.Data.Enumerations.Character; -using NosCore.GameObject.Services.MateService; - -namespace NosCore.GameObject.Tests.Services.MateService -{ - [TestClass] - public class MateXpTableTests - { - [DataTestMethod] - [DataRow((byte)1, 15L)] - [DataRow((byte)3, 90L)] - [DataRow((byte)4, 165L)] - [DataRow((byte)5, 273L)] - [DataRow((byte)6, 420L)] - [DataRow((byte)14, 3720L)] - [DataRow((byte)86, 29312950L)] - [DataRow((byte)88, 39495200L)] - public void PetRequirementMatchesTheCapture(byte level, long expected) - { - Assert.AreEqual(expected, MateXpTable.RequiredXp(level, MateType.Pet), - $"a level {level} pet asked for a different amount than the captured sc_p reported"); - } - - [DataTestMethod] - [DataRow((byte)24, 117720L)] - [DataRow((byte)50, 2293816L)] - public void PartnerRequirementMatchesTheCapture(byte level, long expected) - { - Assert.AreEqual(expected, MateXpTable.RequiredXp(level, MateType.Partner), - $"a level {level} partner asked for a different amount than the captured sc_n reported"); - } - - [TestMethod] - public void PartnerNeedsFourTimesWhatAPetNeeds() - { - for (byte level = 1; level < 100; level++) - { - Assert.AreEqual(MateXpTable.RequiredXp(level, MateType.Pet) * 4, - MateXpTable.RequiredXp(level, MateType.Partner), - $"the pet/partner ratio broke at level {level}"); - } - } - - [TestMethod] - public void RequirementNeverGoesBackwards() - { - var previous = 0L; - for (byte level = 1; level < 100; level++) - { - var current = MateXpTable.RequiredXp(level, MateType.Pet); - Assert.IsTrue(current >= previous, - $"level {level} needs less experience than level {level - 1}"); - previous = current; - } - } - - [TestMethod] - public void AskingBeyondTheTableDoesNotThrow() - { - Assert.IsTrue(MateXpTable.RequiredXp(byte.MaxValue, MateType.Pet) > 0); - Assert.IsTrue(MateXpTable.RequiredXp(0, MateType.Pet) > 0); - } - } -} diff --git a/test/NosCore.Tests.Shared/TestHelpers.cs b/test/NosCore.Tests.Shared/TestHelpers.cs index fb82b7270..ea24f5a3f 100644 --- a/test/NosCore.Tests.Shared/TestHelpers.cs +++ b/test/NosCore.Tests.Shared/TestHelpers.cs @@ -312,6 +312,7 @@ public async Task GenerateSessionAsync(List? pack new Mock>().Object, new Mock>().Object, new List(), new List(),WorldConfiguration, Instance.LogLanguageLocalizer, Instance.PubSubHub.Object, Instance.Clock, ItemList, new HpService(), new MpService(), new SpeedService(), new Mock().Object, SessionGroupFactory, new CharacterInitializationService(), new Mock().Object, new NosCore.GameObject.Services.MateService.MateService(new Mock>().Object, new List(), new NosCore.Core.Services.IdService.IdService(2000000), + new NosCore.Algorithm.MateExperienceService.MateExperienceService(), NullLogger.Instance)), new CSkillPacketHandler(Instance.Clock), From f4f89cf044fbc689aeec185ce37f429b831dc9a4 Mon Sep 17 00:00:00 2001 From: Denis Date: Sun, 23 Aug 2026 14:12:57 +0400 Subject: [PATCH 11/15] refactor(mate): move mate packet generators to ECS extensions Review feedback on #2281: packet generation has no business living on the game object. GenerateScp/Scn/In/Out/Pst/Cond become extension methods on Mate in NosCore.GameObject.Ecs.Extensions, matching how every other entity does it. Also: drop the SpawnComponent comment in MapWorld.CreateMate, and replace the nested null check in MapChangeService with a where clause. --- .../Ecs/Extensions/MateExtensions.cs | 208 ++++++++++++++++++ src/NosCore.GameObject/Ecs/MapWorld.cs | 2 - .../MapChangeService/MapChangeService.cs | 9 +- .../Services/MateService/Mate.cs | 194 ---------------- .../Services/MateService/MateService.cs | 1 + .../Services/MateService/MateServiceTests.cs | 1 + 6 files changed, 213 insertions(+), 202 deletions(-) create mode 100644 src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs diff --git a/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs b/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs new file mode 100644 index 000000000..4b025ba21 --- /dev/null +++ b/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs @@ -0,0 +1,208 @@ +// __ _ __ __ ___ __ ___ ___ +// | \| |/__\ /' _/ / _//__\| _ \ __| +// | | ' | \/ |`._`.| \_| \/ | v / _| +// |_|\__|\__/ |___/ \__/\__/|_|_\___| +// + +using NosCore.Data.Enumerations.Character; +using NosCore.GameObject.Services.MateService; +using NosCore.Packets.Enumerations; +using NosCore.Packets.ServerPackets.Entities; +using NosCore.Packets.ServerPackets.Mates; +using NosCore.Packets.ServerPackets.Parcel; +using NosCore.Packets.ServerPackets.Player; +using NosCore.Packets.ServerPackets.Visibility; +using NosCore.Shared.Enumerations; +using System.Globalization; + +namespace NosCore.GameObject.Ecs.Extensions +{ + public static class MateExtensions + { + public static ScpPacket GenerateScp(this Mate mate, RegionType language) + { + return new ScpPacket + { + PetId = mate.PetSlot, + NpcMonsterVNum = mate.VNum, + TransportId = mate.MateTransportId, + Level = mate.Level, + Loyalty = mate.Loyalty, + Experience = mate.Experience, + Unknow1 = 0, + AttackUpgrade = mate.NpcMonster.AttackUpgrade, + DamageMinimum = mate.NpcMonster.DamageMinimum, + DamageMaximum = mate.NpcMonster.DamageMaximum, + Concentrate = mate.NpcMonster.Concentrate, + CriticalChance = mate.NpcMonster.CriticalChance, + CriticalRate = mate.NpcMonster.CriticalRate, + DefenceUpgrade = mate.NpcMonster.DefenceUpgrade, + CloseDefence = mate.NpcMonster.CloseDefence, + DefenceDodge = mate.NpcMonster.DefenceDodge, + DistanceDefence = mate.NpcMonster.DistanceDefence, + DistanceDefenceDodge = mate.NpcMonster.DistanceDefenceDodge, + MagicDefence = mate.NpcMonster.MagicDefence, + Element = mate.NpcMonster.Element, + FireResistance = mate.NpcMonster.FireResistance, + WaterResistance = mate.NpcMonster.WaterResistance, + LightResistance = mate.NpcMonster.LightResistance, + DarkResistance = mate.NpcMonster.DarkResistance, + Hp = mate.Hp, + MaxHp = mate.MaxHp, + Mp = mate.Mp, + MaxMp = mate.MaxMp, + IsTeamMember = mate.IsTeamMember, + XpLoad = mate.XpLoad, + CanPickUp = mate.CanPickUp, + Name = mate.DisplayName(language), + IsSummonable = mate.IsSummonable + }; + } + + public static ScnPacket GenerateScn(this Mate mate, RegionType language) + { + return new ScnPacket + { + PetId = mate.PetSlot, + NpcMonsterVNum = mate.VNum, + TransportId = mate.MateTransportId, + Level = mate.Level, + Loyalty = mate.Loyalty, + Experience = mate.Experience, + WeaponInstanceDetails = EmptySlot, + ArmorInstanceDetails = EmptySlot, + GauntletInstanceDetails = EmptySlot, + BootsInstanceDetails = EmptySlot, + AttackUpgrade = mate.NpcMonster.AttackUpgrade, + MinimumAttack = mate.NpcMonster.DamageMinimum, + MaximumAttack = mate.NpcMonster.DamageMaximum, + Precision = mate.NpcMonster.Concentrate, + CriticalRate = mate.NpcMonster.CriticalChance, + CriticalDamageRate = mate.NpcMonster.CriticalRate, + DefenceUpgrade = mate.NpcMonster.DefenceUpgrade, + Defence = mate.NpcMonster.CloseDefence, + DefenceDodge = mate.NpcMonster.DefenceDodge, + DistanceDefence = mate.NpcMonster.DistanceDefence, + DistanceDodge = mate.NpcMonster.DistanceDefenceDodge, + DodgeRate = mate.NpcMonster.MagicDefence, + ElementRate = mate.NpcMonster.Element, + FireResistance = mate.NpcMonster.FireResistance, + WaterResistance = mate.NpcMonster.WaterResistance, + LightResistance = mate.NpcMonster.LightResistance, + DarkResistance = mate.NpcMonster.DarkResistance, + Hp = mate.Hp, + HpMax = mate.MaxHp, + Mp = mate.Mp, + MpMax = mate.MaxMp, + IsTeamMember = mate.IsTeamMember, + LevelXp = (int)mate.XpLoad, + Name = mate.DisplayName(language), + MorphId = mate.Skin != 0 ? mate.Skin : -1, + IsSummonable = mate.IsSummonable, + SpDetails = null, + Skill1Details = null, + Skill2Details = null, + Skill3Details = null + }; + } + + /// + /// Owner and GroupEffect are what tell the client this is somebody's mate rather than a + /// map npc: in 2 1506 445562 26 26 2 100 100 0 0 3 626114 1 0 -1 Ratufu^pirate^(Feu) + /// + public static InPacket GenerateIn(this Mate mate, RegionType language) + { + return new InPacket + { + VisualType = VisualType.Npc, + VNum = mate.VNum.ToString(CultureInfo.InvariantCulture), + VisualId = mate.MateTransportId, + PositionX = mate.PositionX, + PositionY = mate.PositionY, + Direction = mate.Direction, + InNonPlayerSubPacket = new InNonPlayerSubPacket + { + InAliveSubPacket = new InAliveSubPacket + { + Hp = Percent(mate.Hp, mate.MaxHp, 100), + Mp = Percent(mate.Mp, mate.MaxMp, 100) + }, + Dialog = 0, + Faction = 0, + GroupEffect = 3, + Owner = mate.CharacterId, + SpawnEffect = SpawnEffectType.NoEffect, + IsSitting = false, + Morph = (short?)(mate.Skin != 0 ? mate.Skin : -1), + Name = mate.DisplayName(language), + Unknow1 = (byte)(mate.MateType == MateType.Partner ? 1 : 0) + } + }; + } + + public static OutPacket GenerateOut(this Mate mate) + { + return new OutPacket + { + VisualType = VisualType.Npc, + VisualId = mate.MateTransportId + }; + } + + /// + /// GroupOrder carries the mate type here, not a position in the party: + /// pst 2 22687 0 100 100 24471 3100 0 0 0 + /// + public static PstPacket GeneratePst(this Mate mate) + { + return new PstPacket + { + Type = VisualType.Npc, + VisualId = mate.MateTransportId, + GroupOrder = (int)mate.MateType, + HpLeft = Percent(mate.Hp, mate.MaxHp, 0), + MpLeft = Percent(mate.Mp, mate.MaxMp, 0), + HpLoad = mate.MaxHp, + MpLoad = mate.MaxMp, + Race = 0, + Gender = GenderType.Male, + Morph = 0, + BuffIds = null + }; + } + + public static CondPacket GenerateCond(this Mate mate) + { + return new CondPacket + { + VisualType = VisualType.Npc, + VisualId = mate.MateTransportId, + NoAttack = false, + NoMove = false, + Speed = mate.NpcMonster.Speed + }; + } + + private static string DisplayName(this Mate mate, RegionType language) + { + if (!string.IsNullOrEmpty(mate.Name)) + { + return mate.Name; + } + + return mate.NpcMonster.Name.TryGetValue(language, out var localized) + ? localized + : mate.NpcMonster.Name[RegionType.EN]; + } + + private static int Percent(int current, int maximum, int whenUnknown) => + maximum > 0 ? (int)(current / (float)maximum * 100) : whenUnknown; + + private static ScnPacket.ScEquipmentDetails EmptySlot => new() + { + ItemId = -1, + ItemRare = 0, + ItemUpgrade = 0 + }; + } +} diff --git a/src/NosCore.GameObject/Ecs/MapWorld.cs b/src/NosCore.GameObject/Ecs/MapWorld.cs index 7db907c51..efe2857ad 100644 --- a/src/NosCore.GameObject/Ecs/MapWorld.cs +++ b/src/NosCore.GameObject/Ecs/MapWorld.cs @@ -153,8 +153,6 @@ public Entity CreateMate( new PositionComponent(positionX, positionY, direction, mapInstance.MapInstanceId), new VisualComponent(0, 0, 0, 0, false, false, false), new NpcDataComponent(mate.VNum, mate.NpcMonster.Race, mate.Level, 0, mate.NpcMonster.Speed, 10), - // A mate never wanders and is never hostile on its own: it goes where its owner - // goes and hits what its owner points at. new SpawnComponent(positionX, positionY, false, false), new EffectComponent(0, 0), new TimingComponent(now, now), diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index 21c327b63..847b9ef36 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -329,13 +329,10 @@ private async Task LeaveMapAsync(ClientSession session) // The entity belongs to the map being left, so it goes with it. A new one is made // on arrival; keeping this one would leave a mate standing in a world nobody is in. - foreach (var mate in leaving) + foreach (var mate in leaving.Where(s => s.Entity.HasValue)) { - if (mate.Entity is { } handle) - { - mapInstance.EcsWorld.DestroyEntity(handle.Handle); - mate.Entity = null; - } + mapInstance.EcsWorld.DestroyEntity(mate.Entity!.Value.Handle); + mate.Entity = null; } session.ClearPlayerEntity(); await session.SendPacketAsync(new MapOutPacket()); diff --git a/src/NosCore.GameObject/Services/MateService/Mate.cs b/src/NosCore.GameObject/Services/MateService/Mate.cs index 4ce8ca32d..dba738528 100644 --- a/src/NosCore.GameObject/Services/MateService/Mate.cs +++ b/src/NosCore.GameObject/Services/MateService/Mate.cs @@ -5,16 +5,7 @@ // using NosCore.Data.Dto; -using NosCore.Data.Enumerations.Character; using NosCore.Data.StaticEntities; -using NosCore.Packets.Enumerations; -using NosCore.Packets.ServerPackets.Entities; -using NosCore.Packets.ServerPackets.Mates; -using NosCore.Packets.ServerPackets.Parcel; -using NosCore.Packets.ServerPackets.Player; -using NosCore.Packets.ServerPackets.Visibility; -using System.Globalization; -using NosCore.Shared.Enumerations; namespace NosCore.GameObject.Services.MateService { @@ -55,190 +46,5 @@ public class Mate : MateDto /// NosCore.Algorithm, and a data object has no business resolving a service. /// public long XpLoad { get; set; } - - public ScpPacket GenerateScp(RegionType language) - { - return new ScpPacket - { - PetId = PetSlot, - NpcMonsterVNum = VNum, - TransportId = MateTransportId, - Level = Level, - Loyalty = Loyalty, - Experience = Experience, - Unknow1 = 0, - AttackUpgrade = NpcMonster.AttackUpgrade, - DamageMinimum = NpcMonster.DamageMinimum, - DamageMaximum = NpcMonster.DamageMaximum, - Concentrate = NpcMonster.Concentrate, - CriticalChance = NpcMonster.CriticalChance, - CriticalRate = NpcMonster.CriticalRate, - DefenceUpgrade = NpcMonster.DefenceUpgrade, - CloseDefence = NpcMonster.CloseDefence, - DefenceDodge = NpcMonster.DefenceDodge, - DistanceDefence = NpcMonster.DistanceDefence, - DistanceDefenceDodge = NpcMonster.DistanceDefenceDodge, - MagicDefence = NpcMonster.MagicDefence, - Element = NpcMonster.Element, - FireResistance = NpcMonster.FireResistance, - WaterResistance = NpcMonster.WaterResistance, - LightResistance = NpcMonster.LightResistance, - DarkResistance = NpcMonster.DarkResistance, - Hp = Hp, - MaxHp = MaxHp, - Mp = Mp, - MaxMp = MaxMp, - IsTeamMember = IsTeamMember, - XpLoad = XpLoad, - CanPickUp = CanPickUp, - Name = DisplayName(language), - IsSummonable = IsSummonable - }; - } - - public ScnPacket GenerateScn(RegionType language) - { - return new ScnPacket - { - PetId = PetSlot, - NpcMonsterVNum = VNum, - TransportId = MateTransportId, - Level = Level, - Loyalty = Loyalty, - Experience = Experience, - WeaponInstanceDetails = EmptySlot, - ArmorInstanceDetails = EmptySlot, - GauntletInstanceDetails = EmptySlot, - BootsInstanceDetails = EmptySlot, - AttackUpgrade = NpcMonster.AttackUpgrade, - MinimumAttack = NpcMonster.DamageMinimum, - MaximumAttack = NpcMonster.DamageMaximum, - Precision = NpcMonster.Concentrate, - CriticalRate = NpcMonster.CriticalChance, - CriticalDamageRate = NpcMonster.CriticalRate, - DefenceUpgrade = NpcMonster.DefenceUpgrade, - Defence = NpcMonster.CloseDefence, - DefenceDodge = NpcMonster.DefenceDodge, - DistanceDefence = NpcMonster.DistanceDefence, - DistanceDodge = NpcMonster.DistanceDefenceDodge, - DodgeRate = NpcMonster.MagicDefence, - ElementRate = NpcMonster.Element, - FireResistance = NpcMonster.FireResistance, - WaterResistance = NpcMonster.WaterResistance, - LightResistance = NpcMonster.LightResistance, - DarkResistance = NpcMonster.DarkResistance, - Hp = Hp, - HpMax = MaxHp, - Mp = Mp, - MpMax = MaxMp, - IsTeamMember = IsTeamMember, - LevelXp = (int)XpLoad, - Name = DisplayName(language), - MorphId = Skin != 0 ? Skin : -1, - IsSummonable = IsSummonable, - SpDetails = null, - Skill1Details = null, - Skill2Details = null, - Skill3Details = null - }; - } - - /// - /// The spawn packet. Owner and GroupEffect are what tell the client this is somebody's - /// mate rather than a map npc: - /// in 2 1506 445562 26 26 2 100 100 0 0 3 626114 1 0 -1 Ratufu^pirate^(Feu) 0 -1 ... - /// - public InPacket GenerateIn(RegionType language) - { - return new InPacket - { - VisualType = VisualType.Npc, - VNum = VNum.ToString(CultureInfo.InvariantCulture), - VisualId = MateTransportId, - PositionX = PositionX, - PositionY = PositionY, - Direction = Direction, - InNonPlayerSubPacket = new InNonPlayerSubPacket - { - InAliveSubPacket = new InAliveSubPacket - { - Hp = MaxHp > 0 ? (int)(Hp / (float)MaxHp * 100) : 100, - Mp = MaxMp > 0 ? (int)(Mp / (float)MaxMp * 100) : 100 - }, - Dialog = 0, - Faction = 0, - GroupEffect = 3, - Owner = CharacterId, - SpawnEffect = SpawnEffectType.NoEffect, - IsSitting = false, - Morph = (short?)(Skin != 0 ? Skin : -1), - Name = DisplayName(language), - Unknow1 = (byte)(MateType == MateType.Partner ? 1 : 0) - } - }; - } - - public OutPacket GenerateOut() - { - return new OutPacket - { - VisualType = VisualType.Npc, - VisualId = MateTransportId - }; - } - - /// - /// The health bar in the party frame. GroupOrder carries the mate type here, not a - /// position in the party: pst 2 22687 0 100 100 24471 3100 0 0 0 - /// - public PstPacket GeneratePst() - { - return new PstPacket - { - Type = VisualType.Npc, - VisualId = MateTransportId, - GroupOrder = (int)MateType, - HpLeft = MaxHp > 0 ? (int)(Hp / (float)MaxHp * 100) : 0, - MpLeft = MaxMp > 0 ? (int)(Mp / (float)MaxMp * 100) : 0, - HpLoad = MaxHp, - MpLoad = MaxMp, - Race = 0, - Gender = GenderType.Male, - Morph = 0, - BuffIds = null - }; - } - - public CondPacket GenerateCond() - { - return new CondPacket - { - VisualType = VisualType.Npc, - VisualId = MateTransportId, - NoAttack = false, - NoMove = false, - Speed = NpcMonster.Speed - }; - } - - private string DisplayName(RegionType language) - { - var name = Name; - if (string.IsNullOrEmpty(name)) - { - name = NpcMonster.Name.TryGetValue(language, out var localized) - ? localized - : NpcMonster.Name[RegionType.EN]; - } - - return name; - } - - private static ScnPacket.ScEquipmentDetails EmptySlot => new() - { - ItemId = -1, - ItemRare = 0, - ItemUpgrade = 0 - }; } } diff --git a/src/NosCore.GameObject/Services/MateService/MateService.cs b/src/NosCore.GameObject/Services/MateService/MateService.cs index 0447bc813..c20eb7d19 100644 --- a/src/NosCore.GameObject/Services/MateService/MateService.cs +++ b/src/NosCore.GameObject/Services/MateService/MateService.cs @@ -4,6 +4,7 @@ // |_|\__|\__/ |___/ \__/\__/|_|_\___| // +using NosCore.GameObject.Ecs.Extensions; using Mapster; using Microsoft.Extensions.Logging; using NosCore.Algorithm.MateExperienceService; diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs index 0abd0d96e..023b494ba 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -4,6 +4,7 @@ // |_|\__|\__/ |___/ \__/\__/|_|_\___| // +using NosCore.GameObject.Ecs.Extensions; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.VisualStudio.TestTools.UnitTesting; using Moq; From 79641778e540647de31984f13a79f77441de67ba Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 24 Aug 2026 00:57:29 +0400 Subject: [PATCH 12/15] chore(mate): cut the comments, drop the BOMs The remarks blocks and the multi-line explanations are gone; what is left is a line where a reader would otherwise get it wrong, and the capture lines that pin a field. 148 comment lines down to 121 across a 1100-line diff. Eleven files had also picked up a UTF-8 BOM, each showing as a whole-line diff against master. Co-Authored-By: Claude Opus 5 --- .../Ecs/Components/PlayerMatesComponent.cs | 2 +- .../Ecs/Extensions/MateExtensions.cs | 6 ++--- src/NosCore.GameObject/Ecs/MapWorld.cs | 2 +- .../Handlers/Mate/MateFollowHandler.cs | 12 +++------- .../Messaging/WolverineDependencyRegistrar.cs | 2 +- .../Services/BattleService/CaptureService.cs | 2 +- .../MapChangeService/MapChangeService.cs | 2 +- .../Services/MateService/IMateService.cs | 2 +- .../Services/MateService/Mate.cs | 22 ++++--------------- .../Services/MateService/MatePlacement.cs | 12 ++++------ .../Services/MateService/MateService.cs | 2 +- .../CharacterScreen/SelectPacketHandler.cs | 2 +- .../Game/GameStartPacketHandler.cs | 2 +- .../Mates/UpetPacketHandler.cs | 12 ++++------ .../Services/MateService/MateServiceTests.cs | 12 ++++------ test/NosCore.Tests.Shared/TestHelpers.cs | 2 +- 16 files changed, 31 insertions(+), 65 deletions(-) diff --git a/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs b/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs index 3a0389f4a..ce0ef93bd 100644 --- a/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs +++ b/src/NosCore.GameObject/Ecs/Components/PlayerMatesComponent.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| diff --git a/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs b/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs index 4b025ba21..67222d743 100644 --- a/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs +++ b/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs @@ -107,8 +107,7 @@ public static ScnPacket GenerateScn(this Mate mate, RegionType language) } /// - /// Owner and GroupEffect are what tell the client this is somebody's mate rather than a - /// map npc: in 2 1506 445562 26 26 2 100 100 0 0 3 626114 1 0 -1 Ratufu^pirate^(Feu) + /// in 2 1506 445562 26 26 2 100 100 0 0 3 626114 1 0 -1 Ratufu^pirate^(Feu) /// public static InPacket GenerateIn(this Mate mate, RegionType language) { @@ -150,8 +149,7 @@ public static OutPacket GenerateOut(this Mate mate) } /// - /// GroupOrder carries the mate type here, not a position in the party: - /// pst 2 22687 0 100 100 24471 3100 0 0 0 + /// GroupOrder carries the mate type, not a party position: pst 2 22687 0 100 100 ... /// public static PstPacket GeneratePst(this Mate mate) { diff --git a/src/NosCore.GameObject/Ecs/MapWorld.cs b/src/NosCore.GameObject/Ecs/MapWorld.cs index efe2857ad..c16b7c101 100644 --- a/src/NosCore.GameObject/Ecs/MapWorld.cs +++ b/src/NosCore.GameObject/Ecs/MapWorld.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| diff --git a/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs b/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs index 89036dd12..491512079 100644 --- a/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs +++ b/src/NosCore.GameObject/Messaging/Handlers/Mate/MateFollowHandler.cs @@ -17,16 +17,13 @@ namespace NosCore.GameObject.Messaging.Handlers.Mate { - // Keeps a character's mates at their heel. A mate that stays where it was summoned looks - // broken long before it looks unfinished, so it moves on the same event the character's own - // step publishes rather than on a timer of its own. + // Mates follow on the same event the owner's own step publishes. [UsedImplicitly] public sealed class MateFollowHandler { [UsedImplicitly] public async Task Handle(CharacterMovedEvent evt) { - // Only a player has mates, and the event is declared on the wider interface. if (evt.Character is not PlayerComponentBundle character) { return; @@ -43,9 +40,7 @@ public async Task Handle(CharacterMovedEvent evt) foreach (var mate in mates) { - // The entity carries the position everything else reads — a monster deciding - // whom to hit, a skill deciding what is in range. Moving only the packet would - // leave the mate visibly in one place and actually in another. + // The entity carries the position aggro and range checks read, not the packet. if (mate.Entity is { } handle) { handle.PositionX = mate.PositionX; @@ -61,8 +56,7 @@ public async Task Handle(CharacterMovedEvent evt) Speed = mate.NpcMonster.Speed }; - // A hidden owner's mates are still theirs: broadcasting them would draw the - // character back onto everybody's screen, and the spawn packet even names them. + // Broadcasting a hidden owner's mates would put the owner back on screen. if (character.Invisible) { await character.SendPacketAsync(move).ConfigureAwait(false); diff --git a/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs b/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs index 8810af6ff..db2f31549 100644 --- a/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs +++ b/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| diff --git a/src/NosCore.GameObject/Services/BattleService/CaptureService.cs b/src/NosCore.GameObject/Services/BattleService/CaptureService.cs index 46945ecbd..cb727c577 100644 --- a/src/NosCore.GameObject/Services/BattleService/CaptureService.cs +++ b/src/NosCore.GameObject/Services/BattleService/CaptureService.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index 847b9ef36..bccd1dae3 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -1,4 +1,4 @@ - + using NodaTime; using NosCore.Algorithm.ExperienceService; using NosCore.Algorithm.HeroExperienceService; diff --git a/src/NosCore.GameObject/Services/MateService/IMateService.cs b/src/NosCore.GameObject/Services/MateService/IMateService.cs index cab4d0e02..59e3be34d 100644 --- a/src/NosCore.GameObject/Services/MateService/IMateService.cs +++ b/src/NosCore.GameObject/Services/MateService/IMateService.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| diff --git a/src/NosCore.GameObject/Services/MateService/Mate.cs b/src/NosCore.GameObject/Services/MateService/Mate.cs index dba738528..d9533cfe2 100644 --- a/src/NosCore.GameObject/Services/MateService/Mate.cs +++ b/src/NosCore.GameObject/Services/MateService/Mate.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -17,34 +17,20 @@ public class Mate : MateDto public byte PetSlot { get; set; } - /// - /// Where the mate is standing right now, which is not where it was stored. MapX and MapY - /// are the square it was last saved on; these two move with the owner. - /// + /// Where the mate is now; MapX/MapY are where it was saved. public short PositionX { get; set; } /// public short PositionY { get; set; } - /// - /// The mate's place in the world while it is out, or null while it is not. - /// - /// - /// A mate that can be hit has to be an entity like any other combatant — the battle - /// service asks for an Arch handle, and giving mates a second notion of "thing that - /// fights" would mean maintaining two. The handle lives here rather than in a registry - /// because the mate is already the thing everyone holds. - /// + /// The mate's ECS entity while it is out, null while it is not. public Ecs.MateComponentBundle? Entity { get; set; } public int MaxHp => NpcMonster.MaxHp; public int MaxMp => NpcMonster.MaxMp; - /// - /// Written when the mate is loaded rather than computed here: the curve lives in - /// NosCore.Algorithm, and a data object has no business resolving a service. - /// + /// Set on load; the curve lives in NosCore.Algorithm. public long XpLoad { get; set; } } } diff --git a/src/NosCore.GameObject/Services/MateService/MatePlacement.cs b/src/NosCore.GameObject/Services/MateService/MatePlacement.cs index 76f27c395..3fca25ef9 100644 --- a/src/NosCore.GameObject/Services/MateService/MatePlacement.cs +++ b/src/NosCore.GameObject/Services/MateService/MatePlacement.cs @@ -14,19 +14,15 @@ namespace NosCore.GameObject.Services.MateService /// public static class MatePlacement { - // Tried in order, so the first walkable square wins and a mate against a wall tucks in - // somewhere rather than standing in it. + // Tried in order: the first walkable square wins. private static readonly (short X, short Y)[] Offsets = [(1, 1), (-1, 1), (1, -1), (-1, -1), (1, 0), (-1, 0), (0, 1), (0, -1)]; /// - /// Places every mate on its own walkable square around the owner. + /// Places every mate on its own walkable square around the owner. Squares are reserved + /// as handed out so a pet and a partner do not stack; with none free the mate stands on + /// the owner. /// - /// - /// A character can have a pet and a partner out at once, so squares are reserved as they - /// are handed out; giving both the same offset would stack them. With nothing free the - /// mate stands on the owner — untidy, and better than being left across the map. - /// public static void Arrange(short ownerX, short ownerY, Map.Map map, IEnumerable mates) { var taken = new HashSet<(short, short)>(); diff --git a/src/NosCore.GameObject/Services/MateService/MateService.cs b/src/NosCore.GameObject/Services/MateService/MateService.cs index c20eb7d19..8a34fd9d6 100644 --- a/src/NosCore.GameObject/Services/MateService/MateService.cs +++ b/src/NosCore.GameObject/Services/MateService/MateService.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| diff --git a/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs b/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs index d01b6e1c4..b825c7664 100644 --- a/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs +++ b/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| diff --git a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs index 4403e6c4a..c6bfad416 100644 --- a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| diff --git a/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs b/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs index 9fb1707b4..2a4aa8825 100644 --- a/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs @@ -18,10 +18,8 @@ namespace NosCore.PacketHandlers.Mates { - // A pet attacking what its owner points it at. The mate goes through the same - // IBattleService.Hit as everything else that fights: it is an entity on the map with the - // same components a monster has, so the skill resolver already treats it as one and there - // is no second damage path to keep in step. + // A mate goes through IBattleService.Hit like anything else that fights - it is an entity + // with a monster's components, so there is no second damage path. public class UpetPacketHandler( IBattleService battleService, ISessionRegistry sessionRegistry, @@ -33,8 +31,7 @@ public override async Task ExecuteAsync(UpetPacket packet, ClientSession session { var character = session.Character; - // Only the owner commands the mate, and only one that is actually out. Trusting the - // id would let a client drive somebody else's pet. + // Trusting the id would let a client drive somebody else's pet. if (!character.Mates.TryGetValue(packet.MateTransportId, out var mate) || !mate.IsTeamMember || mate.Entity is not { } attacker) @@ -48,8 +45,7 @@ public override async Task ExecuteAsync(UpetPacket packet, ClientSession session return; } - // Cast id zero is the creature's own basic attack: a mate has no learned skills, so - // the resolver reads it off the NpcMonster exactly as it does for a monster. + // Cast id zero is the creature's own basic attack, read off the NpcMonster. await battleService.Hit(attacker, target, new HitArguments { SkillId = 0 }) .ConfigureAwait(false); } diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs index 023b494ba..b89bc4881 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| @@ -221,7 +221,6 @@ public async Task APartnerIsFlaggedDifferentlyFromAPetOnSpawnAsync() [TestMethod] public async Task TheHealthBarCarriesTheMateTypeWhereAPlayerCarriesAPartyPositionAsync() { - // pst 2 22687 0 100 100 24471 3100 0 0 0 — the third field is the mate type. var service = Build(new[] { new MateDto { MateId = 1, CharacterId = CharacterId, VNum = PartnerVNum, MateType = MateType.Partner, Hp = 435, Mp = 100 } @@ -238,8 +237,7 @@ public async Task TheHealthBarCarriesTheMateTypeWhereAPlayerCarriesAPartyPositio [TestMethod] public async Task ADespawnNamesTheSameIdTheSpawnDidAsync() { - // A mismatch here leaves the pet drawn on everybody else's screen for ever, and - // nothing throws. + // A mismatch leaves the pet drawn on everybody else's screen, silently. var service = Build(new[] { new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet } @@ -253,8 +251,7 @@ public async Task ADespawnNamesTheSameIdTheSpawnDidAsync() [TestMethod] public async Task OnlyOneMateOfEachTypeIsEverOutAsync() { - // Two rows can claim the slot — two captures racing, or a database edited by hand — - // and the second would spawn on top of the first with nothing raised anywhere. + // Two rows can claim one slot, and the second would spawn on top of the first. var service = Build(new[] { new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, IsTeamMember = true }, @@ -296,8 +293,7 @@ private static GameObject.Map.Map OpenGround() [TestMethod] public async Task TwoMatesNeverStandOnTheSameSquareAsync() { - // A character can have a pet and a partner out at once. Giving both the same offset - // stacks them, and nothing complains. + // A pet and a partner out at once must not share an offset. var service = Build(new[] { new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, IsTeamMember = true }, diff --git a/test/NosCore.Tests.Shared/TestHelpers.cs b/test/NosCore.Tests.Shared/TestHelpers.cs index ea24f5a3f..6d3be481b 100644 --- a/test/NosCore.Tests.Shared/TestHelpers.cs +++ b/test/NosCore.Tests.Shared/TestHelpers.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| From 38f1b037067f392d6a342333d80cc952b396feef Mon Sep 17 00:00:00 2001 From: Denis Date: Mon, 24 Aug 2026 01:02:49 +0400 Subject: [PATCH 13/15] chore: put back the two BOMs master actually has Stripping them blindly was wrong for these two: master carries a BOM on both, so removing it turned a small change into a whole-file diff. Match master, do not strip. Co-Authored-By: Claude Opus 5 --- src/NosCore.GameObject/Ecs/MapWorld.cs | 2 +- .../CharacterScreen/SelectPacketHandler.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/NosCore.GameObject/Ecs/MapWorld.cs b/src/NosCore.GameObject/Ecs/MapWorld.cs index c16b7c101..efe2857ad 100644 --- a/src/NosCore.GameObject/Ecs/MapWorld.cs +++ b/src/NosCore.GameObject/Ecs/MapWorld.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| diff --git a/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs b/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs index b825c7664..d01b6e1c4 100644 --- a/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs +++ b/src/NosCore.PacketHandlers/CharacterScreen/SelectPacketHandler.cs @@ -1,4 +1,4 @@ -// __ _ __ __ ___ __ ___ ___ +// __ _ __ __ ___ __ ___ ___ // | \| |/__\ /' _/ / _//__\| _ \ __| // | | ' | \/ |`._`.| \_| \/ | v / _| // |_|\__|\__/ |___/ \__/\__/|_|_\___| From 20e617e79431010162d01249fa13c1d708a00f61 Mon Sep 17 00:00:00 2001 From: Denis Date: Fri, 28 Aug 2026 17:17:34 +0400 Subject: [PATCH 14/15] style: drop the comments, upstream keeps only what a reader would get wrong Co-Authored-By: Claude Opus 5 --- .../Ecs/Components/MateStateComponent.cs | 4 ---- .../Ecs/Extensions/MateExtensions.cs | 6 ------ src/NosCore.GameObject/Ecs/MateComponentBundle.cs | 5 ----- .../Services/BattleService/CaptureService.cs | 3 --- .../Services/MapChangeService/MapChangeService.cs | 11 ----------- .../Services/MateService/MatePlacement.cs | 8 -------- .../Services/MateService/MateService.cs | 5 ----- .../Game/GameStartPacketHandler.cs | 2 -- src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs | 2 -- .../Services/MateService/MateServiceTests.cs | 5 ----- 10 files changed, 51 deletions(-) diff --git a/src/NosCore.GameObject/Ecs/Components/MateStateComponent.cs b/src/NosCore.GameObject/Ecs/Components/MateStateComponent.cs index dc9dc1bde..3e54047c8 100644 --- a/src/NosCore.GameObject/Ecs/Components/MateStateComponent.cs +++ b/src/NosCore.GameObject/Ecs/Components/MateStateComponent.cs @@ -8,8 +8,4 @@ namespace NosCore.GameObject.Ecs.Components; -/// -/// What makes an entity somebody's mate rather than a monster: the stored row it came from and -/// the character it belongs to. -/// public record struct MateStateComponent(Mate Mate, long OwnerId); diff --git a/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs b/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs index 67222d743..85ef3b081 100644 --- a/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs +++ b/src/NosCore.GameObject/Ecs/Extensions/MateExtensions.cs @@ -106,9 +106,6 @@ public static ScnPacket GenerateScn(this Mate mate, RegionType language) }; } - /// - /// in 2 1506 445562 26 26 2 100 100 0 0 3 626114 1 0 -1 Ratufu^pirate^(Feu) - /// public static InPacket GenerateIn(this Mate mate, RegionType language) { return new InPacket @@ -148,9 +145,6 @@ public static OutPacket GenerateOut(this Mate mate) }; } - /// - /// GroupOrder carries the mate type, not a party position: pst 2 22687 0 100 100 ... - /// public static PstPacket GeneratePst(this Mate mate) { return new PstPacket diff --git a/src/NosCore.GameObject/Ecs/MateComponentBundle.cs b/src/NosCore.GameObject/Ecs/MateComponentBundle.cs index a58f42586..90158c69a 100644 --- a/src/NosCore.GameObject/Ecs/MateComponentBundle.cs +++ b/src/NosCore.GameObject/Ecs/MateComponentBundle.cs @@ -4,9 +4,6 @@ namespace NosCore.GameObject.Ecs; -// A mate on the map is a monster that belongs to somebody: it stands, takes hits, carries buffs -// and cooldowns, and dies. Giving it the monster's component set rather than a set of its own -// is what lets the battle service treat it as a combatant without a second notion of one. [ComponentBundle( typeof(EntityIdentityComponent), typeof(HealthComponent), @@ -27,8 +24,6 @@ namespace NosCore.GameObject.Ecs; { public Arch.Core.Entity Handle => Entity; - // A monster answers with the square it spawned on; a mate follows its owner, so the live - // position is the only meaningful one. public short MapX => PositionX; public short MapY => PositionY; } diff --git a/src/NosCore.GameObject/Services/BattleService/CaptureService.cs b/src/NosCore.GameObject/Services/BattleService/CaptureService.cs index cb727c577..d55295c56 100644 --- a/src/NosCore.GameObject/Services/BattleService/CaptureService.cs +++ b/src/NosCore.GameObject/Services/BattleService/CaptureService.cs @@ -96,9 +96,6 @@ await mateDao.TryInsertOrUpdateAsync(new MateDto Hp = monster.NpcMonster.MaxHp, Mp = monster.NpcMonster.MaxMp, IsSummonable = true, - // A pet you have just caught walks out beside you; it does not go into storage - // for you to fetch later. Only if the pet slot is already taken does it wait, - // because a character may keep one pet and one partner out at a time. IsTeamMember = !mateDao.Where(s => s.CharacterId == character.CharacterId && s.MateType == MateType.Pet && s.IsTeamMember)!.Any() }).ConfigureAwait(false); diff --git a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs index bccd1dae3..ce31b1045 100644 --- a/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs +++ b/src/NosCore.GameObject/Services/MapChangeService/MapChangeService.cs @@ -229,8 +229,6 @@ await Task.WhenAll(mapSessions.Select(async s => : string.Empty; await session.SendPacketAsync(otherCharacter.GenerateIn(prefix)); - // And whatever is at their heel — unless they are hidden, in which case the - // pet would announce them: its spawn packet names its owner. if (!otherCharacter.Invisible) { await session.SendPacketsAsync(otherCharacter.Mates.Values @@ -261,15 +259,10 @@ await session.SendPacketsAsync(otherCharacter.Mates.Values } } - // The mates arrive with their owner, each on its own walkable square: their - // stored square belongs to whichever map they were last saved on, and reusing - // it here would put a pet through a wall. var teamMates = character.Mates.Values.Where(s => s.IsTeamMember).ToList(); MatePlacement.Arrange(character.PositionX, character.PositionY, newMapInstance.Map, teamMates); - // The mate becomes a real entity on the map it is standing on: that is what lets - // it be targeted, buffed and killed like anything else that fights. foreach (var mate in teamMates) { var handle = newMapInstance.EcsWorld.CreateMate( @@ -281,8 +274,6 @@ await session.SendPacketsAsync(otherCharacter.Mates.Values var mateSpawns = teamMates.Select(s => s.GenerateIn(accountLanguage)).ToList(); if (invisible) { - // A hidden owner keeps their mates to themselves: a visible pet with an - // Owner field on it announces the character it belongs to. await session.SendPacketsAsync(mateSpawns); } else @@ -327,8 +318,6 @@ private async Task LeaveMapAsync(ClientSession session) var leaving = character.Mates.Values.Where(s => s.IsTeamMember).ToList(); await mapInstance.SendPacketsAsync(leaving.Select(s => s.GenerateOut())); - // The entity belongs to the map being left, so it goes with it. A new one is made - // on arrival; keeping this one would leave a mate standing in a world nobody is in. foreach (var mate in leaving.Where(s => s.Entity.HasValue)) { mapInstance.EcsWorld.DestroyEntity(mate.Entity!.Value.Handle); diff --git a/src/NosCore.GameObject/Services/MateService/MatePlacement.cs b/src/NosCore.GameObject/Services/MateService/MatePlacement.cs index 3fca25ef9..aef22a569 100644 --- a/src/NosCore.GameObject/Services/MateService/MatePlacement.cs +++ b/src/NosCore.GameObject/Services/MateService/MatePlacement.cs @@ -9,20 +9,12 @@ namespace NosCore.GameObject.Services.MateService { - /// - /// Puts a character's mates around them. - /// public static class MatePlacement { // Tried in order: the first walkable square wins. private static readonly (short X, short Y)[] Offsets = [(1, 1), (-1, 1), (1, -1), (-1, -1), (1, 0), (-1, 0), (0, 1), (0, -1)]; - /// - /// Places every mate on its own walkable square around the owner. Squares are reserved - /// as handed out so a pet and a partner do not stack; with none free the mate stands on - /// the owner. - /// public static void Arrange(short ownerX, short ownerY, Map.Map map, IEnumerable mates) { var taken = new HashSet<(short, short)>(); diff --git a/src/NosCore.GameObject/Services/MateService/MateService.cs b/src/NosCore.GameObject/Services/MateService/MateService.cs index 8a34fd9d6..78b6b660e 100644 --- a/src/NosCore.GameObject/Services/MateService/MateService.cs +++ b/src/NosCore.GameObject/Services/MateService/MateService.cs @@ -56,11 +56,6 @@ public Task> LoadAsync(long characterId) { mate.PetSlot = slot++; - // A character keeps one pet and one partner out at a time. Two rows can - // claim the slot — two captures racing, or a database edited by hand — and - // the second would spawn on top of the first with no error anywhere. The - // reader decides, so a bad row costs a mate that stays in the list rather - // than a broken map. if (!mate.IsTeamMember) { continue; diff --git a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs index c6bfad416..eda3db396 100644 --- a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs @@ -150,8 +150,6 @@ await session.SendPacketAsync(new TwkPacket(session.Account.Name, session.Charac await session.SendPacketsAsync(MateService.GenerateScPackets(session.Character.Mates.Values, session.Character.AccountLanguage)); await session.SendPacketAsync(new ScPStcPacket { MaxMateCountTenths = 0 }); - // Party init even for a solo player: the client wants pinit and a self-row pst so - // its party frame is in a known state for later joins and leaves. await session.SendPacketAsync(session.Character.Group.GeneratePinit()); await session.SendPacketsAsync(session.Character.Group.GeneratePst()); await session.SendPacketsAsync(session.Character.Mates.Values diff --git a/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs b/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs index 2a4aa8825..424674e4d 100644 --- a/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Mates/UpetPacketHandler.cs @@ -18,8 +18,6 @@ namespace NosCore.PacketHandlers.Mates { - // A mate goes through IBattleService.Hit like anything else that fights - it is an entity - // with a monster's components, so there is no second damage path. public class UpetPacketHandler( IBattleService battleService, ISessionRegistry sessionRegistry, diff --git a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs index b89bc4881..80c4d5278 100644 --- a/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs +++ b/test/NosCore.GameObject.Tests/Services/MateService/MateServiceTests.cs @@ -146,8 +146,6 @@ public async Task TheCreatureNameIsUsedWhenTheMateWasNeverRenamedAsync() var packet = (await service.LoadAsync(CharacterId))[0].GenerateScp(RegionType.EN); - // The serializer turns the space into a caret on the way out; the packet itself - // carries the name as it is. Assert.AreEqual("Joyeux Mouton", packet.Name); } @@ -178,9 +176,6 @@ public async Task ScpReportsTheExperienceTheCaptureReportsAsync() [TestMethod] public async Task TheSpawnPacketMarksTheMateAsBelongingToItsOwnerAsync() { - // in 2 1506 445562 26 26 2 100 100 0 0 3 626114 1 0 -1 Ratufu^pirate^(Feu) 0 -1 ... - // Owner and GroupEffect are what separate a mate from a map npc; without them the - // client draws it as scenery and will not let the owner command it. var service = Build(new[] { new MateDto { MateId = 1, CharacterId = CharacterId, VNum = ChickenVNum, MateType = MateType.Pet, Hp = 78, Mp = 5 } From 95638f78d10150a97606fdfc2d2afa47d025163c Mon Sep 17 00:00:00 2001 From: Denis Date: Sat, 29 Aug 2026 16:32:07 +0400 Subject: [PATCH 15/15] style: leave the registrar's own comments alone, and keep the one comment to two lines The three blank comment separators in WolverineDependencyRegistrar belong to code this branch does not change, so the diff no longer touches them. Co-Authored-By: Claude Opus 5 --- .../Messaging/WolverineDependencyRegistrar.cs | 3 +++ src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs | 5 ++--- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs b/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs index db2f31549..e2eed4c62 100644 --- a/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs +++ b/src/NosCore.GameObject/Messaging/WolverineDependencyRegistrar.cs @@ -28,12 +28,14 @@ namespace NosCore.GameObject.Messaging; // Single source of truth for every GameObject-side registration that both MSDI // (for Wolverine codegen) and Autofac (for runtime resolution) need to see. +// // AutofacServiceProviderFactory.Populate() copies MSDI registrations into the // Autofac container at host-build time, so anything registered here becomes // visible to both: Wolverine at codegen, Autofac at runtime. Registering a // service here and again on the Autofac side duplicates the registration and // is the most common cause of "An item with the same key..." style drift // bugs — so bootstrap and tests both call this and nowhere else. +// // DAO/DbContext side is mirrored separately by PersistenceModule.MirrorTo so // this assembly doesn't have to reference NosCore.Database. public static class WolverineDependencyRegistrar @@ -79,6 +81,7 @@ public static void RegisterDependencies(IServiceCollection services) // the ISingletonService marker interface (implemented by classes that own // shared state: caches, queues, per-entity maps). Everything else is // transient so short-lived handlers don't accidentally share mutable state. + // // Matched suffixes cover the vocabulary we actually use across the codebase: // *Service, *Provider, *Resolver, *Calculator, *Catalog, *Queue, *Ai. // New classes can add a suffix here if they want auto-discovery, or they diff --git a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs index eda3db396..26a61d5ee 100644 --- a/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs +++ b/src/NosCore.PacketHandlers/Game/GameStartPacketHandler.cs @@ -143,9 +143,8 @@ await session.SendPacketAsync(new TwkPacket(session.Account.Name, session.Charac // // sqst bf // Session.SendPacket("act6"); // Session.SendPacket(Session.Character.GenerateFaction()); - // p_clear wipes one panel that holds both the party and the mate list, so the two - // bursts have to follow it rather than straddle it. The capture puts them in this - // order: p_clear, the sc packets, sc_p_stc, then pinit. + // p_clear wipes one panel holding both the party and the mate list, so both bursts + // have to follow it rather than straddle it. await session.SendPacketAsync(new PclearPacket()); await session.SendPacketsAsync(MateService.GenerateScPackets(session.Character.Mates.Values, session.Character.AccountLanguage)); await session.SendPacketAsync(new ScPStcPacket { MaxMateCountTenths = 0 });