Implements GameAction/Event for Update Health #172
Conversation
| if (objectId.IsPlayer()) | ||
| { | ||
| var tmpLandblock = this.Session.Player.Position.LandblockId; | ||
| var pl = (Player)LandblockManager.GetObjectByGuid(tmpLandblock, objectId); |
There was a problem hiding this comment.
you already have the player.... this.Session.Player
There should be next to zero reason to return objects from the Landblock / LandblockManager. That was a red flag to me, and this is the incorrect usage. It's also not safe to extract as the player may cross a landblock boundary while running across the open world.
| @@ -52,6 +52,17 @@ public static void RelocateObject(WorldObject worldObject) | |||
| } | |||
|
|
|||
| /// <summary> | |||
| /// Find a worldobject by its guid in the specified landblock | |||
| /// </summary> | |||
| public static WorldObject GetObjectByGuid(LandblockId blockId, ObjectGuid objectId) | |||
There was a problem hiding this comment.
Remove this method. This might be needed eventually, but for now it's a red flag for "you're doing it wrong."
Source/ACE/Entity/Landblock.cs
Outdated
| /// <summary> | ||
| /// get the WorldObject specified by objectID | ||
| /// </summary> | ||
| public WorldObject GetWorldObjectByGuid(ObjectGuid objectId) |
There was a problem hiding this comment.
Remove this method. This might be needed eventually, but for now it's a red flag for "you're doing it wrong."
…us remove GetObjectByGuid from Landblock and LandblockManager
|
The last commit (b5480f2) has the requested changes |
Source/ACE/Entity/Landblock.cs
Outdated
| // check adjacent landblocks for the targetId | ||
| foreach (var block in adjacencies) | ||
| { | ||
| lock (objectCacheLocker) |
There was a problem hiding this comment.
Good thinking, but unfortunately this particular lock isn't going to help you. You're locking THIS landblock's mutex and poking your head into other landblocks. You need to lock block.objectCacheLocker.
Source/ACE/Entity/Landblock.cs
Outdated
| } | ||
| } | ||
| } | ||
| float healthPercentage = (float)pl.Health.Current / (float)pl.Health.MaxValue; |
There was a problem hiding this comment.
pl can be null at this point, too (player portals off the block). Just a quick null check will prevent exceptions.
This is limited for Players currently but should be adjustable once creatures are in.