The Adventurer's skill list, and the rows a class change leaves behind

src/NosCore.GameObject/Ecs/Extensions/PlayerBundleExtensions.cs

@@ -945,7 +945,7 @@ public static async Task GenerateMailAsync(this ClientSession session, IEnumerab
     public static async Task ChangeClassAsync(this ClientSession session, CharacterClassType classType,
         IOptions<WorldConfiguration> worldConfiguration,
         IExperienceService experienceService, IJobExperienceService jobExperienceService, IHeroExperienceService heroExperienceService,
-        IItemGenerationService itemProvider)
+        IItemGenerationService itemProvider, Services.SkillService.ISkillService skillService)
     {
         var character = session.Character;
         var inventoryService = character.InventoryService;
@@ -983,6 +983,18 @@ await session.SendPacketAsync(new SayiPacket
         character.Hp = character.MaxHp;
         character.Mp = character.MaxMp;
 
+        // The old class's skills are no longer usable and the new one's are not there yet:
+        // without this you change job and keep the previous bar, full of icons the client
+        // refuses to cast because they do not belong to the class.
+        character.Skills.Clear();
+
+        // Emptying the list is not enough - the rows behind it survive, and the next login loads
+        // them straight back on top of the new class's. LearnClassSkillsAsync then grants what
+        // the job level allows, and the change has just put that back to 1, so it starts from
+        // the first skill.
+        await skillService.ForgetUnlearnableSkillsAsync(character).ConfigureAwait(false);
+        await skillService.LearnClassSkillsAsync(character).ConfigureAwait(false);
+
         var itemsToAdd = worldConfiguration.Value.BasicEquipments.TryGetValue(classType.ToString(), out var byOrigin)
             && byOrigin.TryGetValue(StarterOrigin.CreateAndUpgrade, out var pack)
             ? pack

src/NosCore.GameObject/Messaging/Handlers/Nrun/ChangeClassHandler.cs

@@ -33,7 +33,8 @@ public sealed class ChangeClassHandler(
         IExperienceService experienceService,
         IJobExperienceService jobExperienceService,
         IHeroExperienceService heroExperienceService,
-        NosCore.GameObject.Services.ItemGenerationService.IItemGenerationService itemProvider) : INrunEventHandler
+        NosCore.GameObject.Services.ItemGenerationService.IItemGenerationService itemProvider,
+        Services.SkillService.ISkillService skillService) : INrunEventHandler
     {
         public NrunRunnerType Runner => NrunRunnerType.ChangeClass;
 
@@ -86,7 +87,7 @@ await session.SendPacketAsync(new SayiPacket
             }
 
             await session.ChangeClassAsync(classType, worldConfiguration, experienceService,
-                jobExperienceService, heroExperienceService, itemProvider);
+                jobExperienceService, heroExperienceService, itemProvider, skillService);
         }
     }
 }

src/NosCore.GameObject/Services/SkillService/ISkillService.cs

@@ -8,5 +8,15 @@ public interface ISkillService
         Task LoadSkill(ICharacterEntity character);
 
         Task<bool> LearnClassSkillsAsync(ICharacterEntity character);
+
+        /// <summary>
+        /// Deletes the skills the character cannot learn right now - wrong class, or a job level
+        /// they no longer have - from memory and from the database both.
+        /// </summary>
+        /// <remarks>
+        /// A class change already emptied the in-memory list; the rows behind it stayed, and
+        /// came back on the next login. See the implementation for what that did.
+        /// </remarks>
+        Task ForgetUnlearnableSkillsAsync(ICharacterEntity character);
     }
 }

src/NosCore.GameObject/Services/SkillService/SkillService.cs

@@ -18,6 +18,11 @@ public class SkillService(IDao<CharacterSkillDto, Guid> characterSkillDao, List<
     {
         public async Task LoadSkill(ICharacterEntity character)
         {
+            // Characters who changed class before the deletion below existed are still carrying
+            // the old rows. Clearing them at login, and not only at the next class change, means
+            // those characters heal themselves instead of staying broken for ever.
+            await ForgetUnlearnableSkillsAsync(character).ConfigureAwait(false);
+
             var characterSkills = characterSkillDao.Where(x => x.CharacterId == character.VisualId).Adapt<List<CharacterSkill>>() ?? new List<CharacterSkill>();
             var skillToUse = skills.Where(x => characterSkills.Select(s => s.SkillVNum).Contains(x.SkillVNum));
             character.Skills.Clear();
@@ -48,22 +53,97 @@ await character.SendPacketAsync(new SkiPacket
             }).ConfigureAwait(false);
         }
 
+        /// <summary>
+        /// The Adventurer's skills, listed one by one.
+        ///
+        /// <b>They cannot be selected by class like every other one</b>, and that is this table's
+        /// trap: class 0 does not mean "Adventurer", it is the scrap container where 193 entries
+        /// end up - the emotes, the stat courses, the shop and rest actions, passives. Filtering
+        /// by class 0 gave an Adventurer <i>all</i> of them, and the bar filled with icons the
+        /// client will not cast.
+        ///
+        /// The range is 200 to 210 inclusive. 209 is in it: Skill.dat gives it class 0, cast id
+        /// 16, LevelMinimum 1 and the name Capture - it is the Adventurer's pet catcher, which is
+        /// why CharNewPacketHandler grants it to every new character and why its comment there
+        /// talks about <c>u_s 16</c>. Left out, an Adventurer cannot catch anything.
+        ///
+        /// 211 and 212 are excluded on purpose: the file calls them "Ultra Super Cheating Skill"
+        /// and "Admin Cheating Skill".
+        ///
+        /// Open, and deliberately not guessed at: 300 to 306 are also class 0, with LevelMinimum
+        /// 10 to 18 and names like "Strengthen Swing". They look like the upgraded forms of
+        /// 200-206 and they carry the SAME cast ids, so they cannot simply be added alongside.
+        /// Nothing in the files says how one replaces the other, so they stay out until it does.
+        /// </summary>
+        private static readonly short[] AdventurerSkills =
+            { 200, 201, 202, 203, 204, 205, 206, 207, 208, 209, 210 };
+
+        /// <summary>The skills this class can hold.</summary>
+        private IEnumerable<SkillDto> Learnable(ICharacterEntity character) =>
+            character.Class == CharacterClassType.Adventurer
+                ? skills.Where(s => AdventurerSkills.Contains(s.SkillVNum))
+                : skills.Where(s => s.Class == (byte)character.Class);
+
+        /// <summary>
+        /// A class change was only half done. The change empties the in-memory list and learns the
+        /// new class's skills, but nothing ever deleted the rows behind the old ones - so the next
+        /// login loaded both sets back.
+        ///
+        /// That is not a cosmetic leftover. Cast ids are numbered per class and start at zero, so
+        /// an Archer who used to be an Adventurer ended up knowing two skills answering to cast 0:
+        /// Swing (melee) and Archery (ranged). Which one the resolver returned came down to
+        /// dictionary order.
+        ///
+        /// The visible symptom was a basic attack computed off the <b>wrong weapon</b>: Swing is a
+        /// melee skill, a melee skill selects the secondary-weapon profile on an Archer, and the
+        /// bow in the main hand counted for nothing.
+        ///
+        /// The job level is part of the question and not only the class: a class change puts the
+        /// job level back to 1, so a row for a skill of the <i>destination</i> class that needs
+        /// job 20 is just as unusable as one belonging to the class left behind.
+        /// </summary>
+        public async Task ForgetUnlearnableSkillsAsync(ICharacterEntity character)
+        {
+            var keep = Learnable(character)
+                .Where(skill => skill.LevelMinimum <= character.JobLevel)
+                .Select(skill => skill.SkillVNum)
+                .ToHashSet();
+            var characterId = character.VisualId;
+
+            foreach (var stale in characterSkillDao.Where(x => x.CharacterId == characterId)?
+                         .Where(x => !keep.Contains(x.SkillVNum)).ToList() ?? [])
+            {
+                await characterSkillDao.TryDeleteAsync(stale.Id).ConfigureAwait(false);
+                character.Skills.TryRemove(stale.SkillVNum, out _);
+            }
+        }
+
         public async Task<bool> LearnClassSkillsAsync(ICharacterEntity character)
         {
             var classByte = (byte)character.Class;
             var learned = false;
-            foreach (var skill in skills.Where(s => s.Class == classByte && s.LevelMinimum <= character.JobLevel))
+            foreach (var skill in Learnable(character).Where(s => s.LevelMinimum <= character.JobLevel))
             {
                 if (character.Skills.ContainsKey(skill.SkillVNum))
                 {
                     continue;
                 }
 
+                // The row's existing database id is reused when there is one. With a fresh Guid
+                // every time, each call inserted one more row for the same skill: in memory it did
+                // not show, because the dictionary is keyed by skill number and collapses them,
+                // but the rows piled up behind it.
+                var characterId = character.VisualId;
+                var skillVNum = skill.SkillVNum;
+                var existing = await characterSkillDao
+                    .FirstOrDefaultAsync(x => x.CharacterId == characterId && x.SkillVNum == skillVNum)
+                    .ConfigureAwait(false);
+
                 var entry = new CharacterSkill
                 {
-                    Id = Guid.NewGuid(),
-                    CharacterId = character.VisualId,
-                    SkillVNum = skill.SkillVNum,
+                    Id = existing?.Id ?? Guid.NewGuid(),
+                    CharacterId = characterId,
+                    SkillVNum = skillVNum,
                     Skill = skill,
                 };
                 if (character.Skills.TryAdd(skill.SkillVNum, entry))

src/NosCore.PacketHandlers/Command/ChangeClassPacketHandler.cs

@@ -28,14 +28,15 @@ namespace NosCore.PacketHandlers.Command
     public class ChangeClassPacketHandler(IPubSubHub pubSubHub,
         IOptions<WorldConfiguration> worldConfiguration, IExperienceService experienceService,
         IJobExperienceService jobExperienceService, IHeroExperienceService heroExperienceService,
-        IItemGenerationService itemProvider)
+        IItemGenerationService itemProvider,
+        NosCore.GameObject.Services.SkillService.ISkillService skillService)
         : PacketHandler<ChangeClassPacket>, IWorldPacketHandler
     {
         public override async Task ExecuteAsync(ChangeClassPacket changeClassPacket, ClientSession session)
         {
             if ((changeClassPacket.Name == session.Character.Name) || string.IsNullOrEmpty(changeClassPacket.Name))
             {
-                await session.ChangeClassAsync(changeClassPacket.ClassType, worldConfiguration, experienceService, jobExperienceService, heroExperienceService, itemProvider);
+                await session.ChangeClassAsync(changeClassPacket.ClassType, worldConfiguration, experienceService, jobExperienceService, heroExperienceService, itemProvider, skillService);
                 return;
             }