From 1f58dc2b53833a38466518781c853bf2fc0a427b Mon Sep 17 00:00:00 2001 From: hondacrx Date: Thu, 23 Dec 2021 20:17:08 -0500 Subject: [PATCH] Entities/Unit: Cleanup all the direct accesses to m_unitMovedByMe. Refactor the field to be protected. Add assertions to catch dangling pointers. Port From (https://github.com/TrinityCore/TrinityCore/commit/edc75831194bc2419e3abada47121fadf4b2aa8d) --- Source/Game/Entities/Player/Player.Fields.cs | 2 - Source/Game/Entities/Player/Player.cs | 17 ++----- Source/Game/Entities/Unit/Unit.Fields.cs | 3 +- Source/Game/Entities/Unit/Unit.Movement.cs | 51 +++++++++++--------- Source/Game/Handlers/SpellHandler.cs | 16 +++--- 5 files changed, 41 insertions(+), 48 deletions(-) diff --git a/Source/Game/Entities/Player/Player.Fields.cs b/Source/Game/Entities/Player/Player.Fields.cs index 7c10c01e0..f8046043f 100644 --- a/Source/Game/Entities/Player/Player.Fields.cs +++ b/Source/Game/Entities/Player/Player.Fields.cs @@ -209,8 +209,6 @@ namespace Game.Entities public List m_clientGUIDs = new(); public List m_visibleTransports = new(); public WorldObject seerView; - // only changed for direct client control (possess, vehicle etc.), not stuff you control using pet commands - public Unit m_unitMovedByMe; Team m_team; ReputationMgr reputationMgr; QuestObjectiveCriteriaManager m_questObjectiveCriteriaMgr; diff --git a/Source/Game/Entities/Player/Player.cs b/Source/Game/Entities/Player/Player.cs index 6890cebbc..5ef7cb0ca 100644 --- a/Source/Game/Entities/Player/Player.cs +++ b/Source/Game/Entities/Player/Player.cs @@ -4013,7 +4013,7 @@ namespace Game.Entities public override bool CanAlwaysSee(WorldObject obj) { // Always can see self - if (m_unitMovedByMe == obj) + if (GetUnitBeingMoved() == obj) return true; ObjectGuid guid = m_activePlayerData.FarsightObject; @@ -5544,7 +5544,7 @@ namespace Game.Entities initialSetup.ServerExpansionLevel = (byte)WorldConfig.GetIntValue(WorldCfg.Expansion); SendPacket(initialSetup); - SetMover(this); + SetMovedUnit(this); } public void SendInitialPacketsAfterAddToMap() @@ -7228,18 +7228,7 @@ namespace Game.Entities SetViewpoint(target, allowMove); if (allowMove) - SetMover(target); - } - - public void SetMover(Unit target) - { - m_unitMovedByMe.m_playerMovingMe = null; - m_unitMovedByMe = target; - m_unitMovedByMe.m_playerMovingMe = this; - - MoveSetActiveMover packet = new(); - packet.MoverGUID = target.GetGUID(); - SendPacket(packet); + SetMovedUnit(target); } public Item GetWeaponForAttack(WeaponAttackType attackType, bool useable = false) diff --git a/Source/Game/Entities/Unit/Unit.Fields.cs b/Source/Game/Entities/Unit/Unit.Fields.cs index fe90511a9..1d3a63b65 100644 --- a/Source/Game/Entities/Unit/Unit.Fields.cs +++ b/Source/Game/Entities/Unit/Unit.Fields.cs @@ -42,8 +42,9 @@ namespace Game.Entities MotionMaster i_motionMaster; public uint m_movementCounter; //< Incrementing counter used in movement packets TimeTrackerSmall movesplineTimer; - public Player m_playerMovingMe; MovementForces _movementForces; + protected Unit m_unitMovedByMe; // only ever set for players, and only for direct client control + protected Player m_playerMovingMe; // only set for direct client control (possess effects, vehicles and similar) //Combat protected List attackerList = new(); diff --git a/Source/Game/Entities/Unit/Unit.Movement.cs b/Source/Game/Entities/Unit/Unit.Movement.cs index 208fd1f41..7c63d4f73 100644 --- a/Source/Game/Entities/Unit/Unit.Movement.cs +++ b/Source/Game/Entities/Unit/Unit.Movement.cs @@ -123,7 +123,7 @@ namespace Game.Entities } } - Player playerMover = GetPlayerBeingMoved(); // unit controlled by a player. + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); // unit controlled by a player. if (playerMover) { // Send notification to self @@ -244,7 +244,7 @@ namespace Game.Entities if (charmer) { player = charmer.ToPlayer(); - if (player && player.m_unitMovedByMe != this) + if (player && player.GetUnitBeingMoved() != this) player = null; } } @@ -287,7 +287,7 @@ namespace Game.Entities else RemoveUnitMovementFlag(MovementFlag.DisableCollision); - Player playerMover = GetPlayerBeingMoved(); + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { MoveSetFlag packet = new(disable ? ServerOpcodes.MoveSplineEnableCollision : ServerOpcodes.MoveEnableCollision); @@ -322,7 +322,7 @@ namespace Game.Entities else RemoveUnitMovementFlag2(MovementFlag2.CanSwimToFlyTrans); - Player playerMover = GetPlayerBeingMoved(); + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { MoveSetFlag packet = new(enable ? ServerOpcodes.MoveEnableTransitionBetweenSwimAndFly : ServerOpcodes.MoveDisableTransitionBetweenSwimAndFly); @@ -349,7 +349,7 @@ namespace Game.Entities else RemoveUnitMovementFlag2(MovementFlag2.CanTurnWhileFalling); - Player playerMover = GetPlayerBeingMoved(); + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { MoveSetFlag packet = new(enable ? ServerOpcodes.MoveSetCanTurnWhileFalling : ServerOpcodes.MoveUnsetCanTurnWhileFalling); @@ -375,7 +375,7 @@ namespace Game.Entities else RemoveUnitMovementFlag2(MovementFlag2.CanDoubleJump); - Player playerMover = GetPlayerBeingMoved(); + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { MoveSetFlag packet = new(enable ? ServerOpcodes.MoveEnableDoubleJump : ServerOpcodes.MoveDisableDoubleJump); @@ -672,7 +672,7 @@ namespace Game.Entities RemoveUnitMovementFlag(MovementFlag.DisableGravity); - Player playerMover = GetPlayerBeingMoved(); + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { MoveSetFlag packet = new(disable ? ServerOpcodes.MoveDisableGravity : ServerOpcodes.MoveEnableGravity); @@ -816,8 +816,8 @@ namespace Game.Entities } } } - - public override void ProcessPositionDataChanged(PositionFullTerrainStatus data) + + public override void ProcessPositionDataChanged(PositionFullTerrainStatus data) { base.ProcessPositionDataChanged(data); ProcessTerrainStatusUpdate(data.LiquidStatus, data.LiquidInfo); @@ -922,7 +922,7 @@ namespace Game.Entities if (!enable && IsTypeId(TypeId.Player)) ToPlayer().SetFallInformation(0, GetPositionZ()); - Player playerMover = GetPlayerBeingMoved(); + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { MoveSetFlag packet = new(enable ? ServerOpcodes.MoveSetCanFly : ServerOpcodes.MoveUnsetCanFly); @@ -955,7 +955,7 @@ namespace Game.Entities RemoveUnitMovementFlag(MovementFlag.WaterWalk); - Player playerMover = GetPlayerBeingMoved(); + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { MoveSetFlag packet = new(enable ? ServerOpcodes.MoveSetWaterWalk : ServerOpcodes.MoveSetLandWalk); @@ -988,7 +988,7 @@ namespace Game.Entities RemoveUnitMovementFlag(MovementFlag.FallingSlow); - Player playerMover = GetPlayerBeingMoved(); + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { MoveSetFlag packet = new(enable ? ServerOpcodes.MoveSetFeatherFall : ServerOpcodes.MoveSetNormalFall); @@ -1036,7 +1036,7 @@ namespace Game.Entities } } - Player playerMover = GetPlayerBeingMoved(); + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { MoveSetFlag packet = new(enable ? ServerOpcodes.MoveSetHovering : ServerOpcodes.MoveUnsetHovering); @@ -1107,6 +1107,17 @@ namespace Game.Entities } } + public void SetMovedUnit(Unit target) + { + m_unitMovedByMe.m_playerMovingMe = null; + m_unitMovedByMe = target; + m_unitMovedByMe.m_playerMovingMe = ToPlayer(); + + MoveSetActiveMover packet = new(); + packet.MoverGUID = target.GetGUID(); + ToPlayer().SendPacket(packet); + } + public void SetControlled(bool apply, UnitState state) { if (apply) @@ -1254,7 +1265,7 @@ namespace Game.Entities RemoveUnitMovementFlag(MovementFlag.Root); } - Player playerMover = GetPlayerBeingMoved();// unit controlled by a player. + Player playerMover = GetUnitBeingMoved()?.ToPlayer();// unit controlled by a player. if (playerMover) { MoveSetFlag packet = new(apply ? ServerOpcodes.MoveRoot : ServerOpcodes.MoveUnroot); @@ -1624,14 +1635,7 @@ namespace Game.Entities SendMessageToSet(data, true); } - public Unit GetUnitBeingMoved() - { - Player player = ToPlayer(); - if (player) - return player.m_unitMovedByMe; - - return null; - } + public Unit GetUnitBeingMoved() { return m_unitMovedByMe; } Player GetPlayerBeingMoved() { @@ -1759,7 +1763,8 @@ namespace Game.Entities moveUpdateTeleport.MovementForces = _movementForces.GetForces(); Unit broadcastSource = this; - Player playerMover = GetPlayerBeingMoved(); + // should this really be the unit _being_ moved? not the unit doing the moving? + Player playerMover = GetUnitBeingMoved()?.ToPlayer(); if (playerMover) { float x, y, z, o; diff --git a/Source/Game/Handlers/SpellHandler.cs b/Source/Game/Handlers/SpellHandler.cs index 923da5b6d..60db3f009 100644 --- a/Source/Game/Handlers/SpellHandler.cs +++ b/Source/Game/Handlers/SpellHandler.cs @@ -37,7 +37,7 @@ namespace Game Player user = GetPlayer(); // ignore for remote control state - if (user.m_unitMovedByMe != user) + if (user.GetUnitBeingMoved() != user) return; Item item = user.GetUseableItemByPos(packet.PackSlot, packet.Slot); @@ -133,7 +133,7 @@ namespace Game Player player = GetPlayer(); // ignore for remote control state - if (player.m_unitMovedByMe != player) + if (player.GetUnitBeingMoved() != player) return; // additional check, client outputs message on its own @@ -243,8 +243,8 @@ namespace Game if (obj) { // ignore for remote control state - if (GetPlayer().m_unitMovedByMe != GetPlayer()) - if (!(GetPlayer().IsOnVehicle(GetPlayer().m_unitMovedByMe) || GetPlayer().IsMounted()) && !obj.GetGoInfo().IsUsableMounted()) + if (GetPlayer().GetUnitBeingMoved() != GetPlayer()) + if (!(GetPlayer().IsOnVehicle(GetPlayer().GetUnitBeingMoved()) || GetPlayer().IsMounted()) && !obj.GetGoInfo().IsUsableMounted()) return; obj.Use(GetPlayer()); @@ -255,7 +255,7 @@ namespace Game void HandleGameobjectReportUse(GameObjReportUse packet) { // ignore for remote control state - if (GetPlayer().m_unitMovedByMe != GetPlayer()) + if (GetPlayer().GetUnitBeingMoved() != GetPlayer()) return; GameObject go = GetPlayer().GetGameObjectIfCanInteractWith(packet.Guid); @@ -272,7 +272,7 @@ namespace Game void HandleCastSpell(CastSpell cast) { // ignore for remote control state (for player case) - Unit mover = GetPlayer().m_unitMovedByMe; + Unit mover = GetPlayer().GetUnitBeingMoved(); if (mover != GetPlayer() && mover.IsTypeId(TypeId.Player)) return; @@ -472,7 +472,7 @@ namespace Game void HandleCancelChanneling(CancelChannelling cancelChanneling) { // ignore for remote control state (for player case) - Unit mover = _player.m_unitMovedByMe; + Unit mover = _player.GetUnitBeingMoved(); if (mover != _player && mover.IsTypeId(TypeId.Player)) return; @@ -483,7 +483,7 @@ namespace Game void HandleTotemDestroyed(TotemDestroyed totemDestroyed) { // ignore for remote control state - if (GetPlayer().m_unitMovedByMe != GetPlayer()) + if (GetPlayer().GetUnitBeingMoved() != GetPlayer()) return; byte slotId = totemDestroyed.Slot;