A mount changes the sprite and nothing else

src/NosCore.GameObject/Services/SpeedCalculationService/SpeedCalculationService.cs

@@ -35,14 +35,16 @@ public byte CalculateSpeed(INonPlayableEntity nonPlayableEntity)
 
         public byte CalculateSpeed(ICharacterEntity characterEntity)
         {
-            var defaultSpeed = speedService.GetSpeed(characterEntity.Class);
-            if (characterEntity.VehicleSpeed != null)
+            // IsVehicled and not "VehicleSpeed is not null": the component declares that field as
+            // a plain byte and the interface widens it to byte?, so the null branch could never
+            // be taken - the service answered VehicleSpeed always, which is 0 on foot. Nothing
+            // reported it because nothing called the service at all.
+            if (characterEntity.IsVehicled)
             {
-                return (byte)characterEntity.VehicleSpeed;
-
+                return characterEntity.VehicleSpeed ?? 0;
             }
 
-            return CalculateSpeed(characterEntity, defaultSpeed);
+            return CalculateSpeed(characterEntity, speedService.GetSpeed(characterEntity.Class));
         }
     }
 }

src/NosCore.GameObject/Services/TransformationService/TransformationService.cs

@@ -31,7 +31,8 @@ namespace NosCore.GameObject.Services.TransformationService
 {
     public class TransformationService(IClock clock, IExperienceService experienceService,
             IJobExperienceService jobExperienceService, IHeroExperienceService heroExperienceService, ILogger<TransformationService> logger,
-            ILogLanguageLocalizer<LogLanguageKey> logLanguage, IOptions<WorldConfiguration> worldConfiguration)
+            ILogLanguageLocalizer<LogLanguageKey> logLanguage, IOptions<WorldConfiguration> worldConfiguration,
+            SpeedCalculationService.ISpeedCalculationService speedCalculationService)
         : ITransformationService
     {
         public async Task RemoveSpAsync(ClientSession session)
@@ -159,6 +160,11 @@ public async Task ChangeVehicleAsync(ClientSession session, Item item)
             var character = session.Character;
             character.IsVehicled = true;
             character.VehicleSpeed = item.Speed;
+
+            // cond carries Speed, not VehicleSpeed, and Speed is written once at login from
+            // the class table. Without this line the mount changes the sprite and nothing
+            // else: the player rides at walking pace, and no packet or log says otherwise.
+            character.Speed = speedCalculationService.CalculateSpeed(character);
             character.MorphUpgrade = 0;
             character.MorphDesign = 0;
             character.Morph = item.SecondMorph == 0
@@ -201,6 +207,7 @@ public async Task RemoveVehicleAsync(ClientSession session)
 
             character.IsVehicled = false;
             character.VehicleSpeed = 0;
+            character.Speed = speedCalculationService.CalculateSpeed(character);
 
             var mapInstance = character.MapInstance;
             var condPacket = session.Character.GenerateCond();

test/NosCore.GameObject.Tests/Services/SpeedCalculationService/SpeedCalculationServiceTests.cs

@@ -57,12 +57,31 @@ public void VehicleSpeedOverridesDefaultSpeed(int characterClassInt)
 
             var charMock = new Mock<ICharacterEntity>();
             charMock.SetupGet(x => x.Class).Returns(characterClass);
+            // Both, because a mount sets both. Setting only VehicleSpeed described a state that
+            // cannot happen, and the old null check happened to answer it correctly.
+            charMock.SetupGet(x => x.IsVehicled).Returns(true);
             charMock.SetupGet(x => x.VehicleSpeed).Returns(50);
 
             var speed = SpeedCalculationService.CalculateSpeed(charMock.Object);
             Assert.AreEqual(50, speed);
         }
 
+        // On foot VehicleSpeed is 0, not null - the component declares it as a plain byte. Reading
+        // it whenever it "is not null" therefore answered 0 for everyone, and it went unnoticed
+        // because nothing called this service.
+        [TestMethod]
+        public void OnFootTheVehicleSpeedIsIgnoredRatherThanReturnedAsZero()
+        {
+            SpeedService.Setup(x => x.GetSpeed(CharacterClassType.Archer)).Returns((byte)12);
+
+            var charMock = new Mock<ICharacterEntity>();
+            charMock.SetupGet(x => x.Class).Returns(CharacterClassType.Archer);
+            charMock.SetupGet(x => x.IsVehicled).Returns(false);
+            charMock.SetupGet(x => x.VehicleSpeed).Returns((byte)0);
+
+            Assert.AreEqual(12, SpeedCalculationService.CalculateSpeed(charMock.Object));
+        }
+
         [TestMethod]
         public void DefaultMonsterSpeedIsNpcMonsterSpeed()
         {

test/NosCore.GameObject.Tests/Services/TransformationService/TransformationServiceTests.cs

@@ -4,6 +4,8 @@
 // |_|\__|\__/ |___/ \__/\__/|_|_\___|
 //
 
+using NosCore.GameObject.Ecs.Extensions;
+using NosCore.Algorithm.SpeedService;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
 using NosCore.Algorithm.ExperienceService;
@@ -39,7 +41,8 @@ public async Task SetupAsync()
                 new Mock<IHeroExperienceService>().Object,
                 Logger,
                 TestHelpers.Instance.LogLanguageLocalizer,
-                TestHelpers.Instance.WorldConfiguration);
+                TestHelpers.Instance.WorldConfiguration,
+                new GameObject.Services.SpeedCalculationService.SpeedCalculationService(new SpeedService()));
         }
 
         [TestMethod]
@@ -60,6 +63,7 @@ public async Task ChangingVehicleShouldSetVehicledState()
                 .WhenAsync(ChangingToVehicle)
                 .Then(CharacterShouldBeVehicled)
                 .And(VehicleSpeedShouldBeSet)
+                .And(TheSpeedTheClientSeesIsTheVehicles)
                 .ExecuteAsync();
         }
 
@@ -71,9 +75,25 @@ public async Task RemovingVehicleShouldResetState()
                 .WhenAsync(RemovingVehicle)
                 .Then(CharacterShouldNotBeVehicled)
                 .And(VehicleSpeedShouldBeZero)
+                .And(TheSpeedTheClientSeesIsBackOnFoot)
                 .ExecuteAsync();
         }
 
+        // VehicleSpeed alone changes nothing the player can feel: `cond` carries Speed, and Speed
+        // was written once at login from the class table. Asserting only VehicleSpeed is what let
+        // a mount that adds no speed look correct.
+        private void TheSpeedTheClientSeesIsTheVehicles()
+        {
+            Assert.AreEqual(20, Session.Character.Speed);
+            Assert.AreEqual(20, Session.Character.GenerateCond().Speed);
+        }
+
+        private void TheSpeedTheClientSeesIsBackOnFoot()
+        {
+            Assert.AreEqual(new SpeedService().GetSpeed(Session.Character.Class),
+                Session.Character.Speed);
+        }
+
         private void CharacterHasSpEquipped()
         {
             Session.Character.UseSp = true;

test/NosCore.PacketHandlers.Tests/Inventory/SpTransformPacketHandlerTests.cs

@@ -4,6 +4,7 @@
 // |_|\__|\__/ |___/ \__/\__/|_|_\___|
 //
 
+using NosCore.Algorithm.SpeedService;
 using Microsoft.VisualStudio.TestTools.UnitTesting;
 using Moq;
 using NosCore.Algorithm.ExperienceService;
@@ -47,7 +48,8 @@ public async Task SetupAsync()
             SpTransformPacketHandler = new SpTransformPacketHandler(TestHelpers.Instance.Clock,
                 new TransformationService(TestHelpers.Instance.Clock, new Mock<IExperienceService>().Object,
                     new Mock<IJobExperienceService>().Object, new Mock<IHeroExperienceService>().Object,
-                    new Mock<ILogger<TransformationService>>().Object, TestHelpers.Instance.LogLanguageLocalizer, TestHelpers.Instance.WorldConfiguration),
+                    new Mock<ILogger<TransformationService>>().Object, TestHelpers.Instance.LogLanguageLocalizer, TestHelpers.Instance.WorldConfiguration,
+                    new NosCore.GameObject.Services.SpeedCalculationService.SpeedCalculationService(new SpeedService())),
                 TestHelpers.Instance.GameLanguageLocalizer);
         }