Fix: Position missing update and LandblockId db object fix #243
Conversation
| { | ||
| // load the default position | ||
| Position newCharacterPosition = CharacterPositionExtensions.StartingPosition(character.Id); | ||
| Position newRecallPosition = CharacterPositionExtensions.InvalidPosition(character.Id, PositionType.LastPortal); |
There was a problem hiding this comment.
This will be set on demand in the future, when the character uses a portal.
| @@ -97,8 +107,7 @@ public Position InFrontOf(double distanceInFront = 3.0f) | |||
|
|
|||
| public Position(uint characterId, PositionType type, uint newCell, float newPositionX, float newPositionY, float newPositionZ, float newRotationX, float newRotationY, float newRotationZ, float newRotationW) | |||
| { | |||
| LandblockId = new LandblockId(Cell); | |||
|
|
|||
| LandblockId = new LandblockId(newCell); | |||
There was a problem hiding this comment.
Bug fix: this was saving the previous value as the LandblockId.
| @@ -277,8 +277,28 @@ public bool IsNameAvailable(string name) | |||
|
|
|||
| // Loads the positions into the player object and resets the landlock id | |||
| LoadCharacterPositions(c); | |||
| c.Location = c.Positions[PositionType.Location]; | |||
| c.Location.LandblockId = new LandblockId(c.Location.Cell); | |||
There was a problem hiding this comment.
This line is no longer necessary to set LandblockId, as the position's LandblockId should now be set when changing the Cell or LandblockId of the Position.
| else if (c.Positions.ContainsKey(PositionType.Sanctuary)) | ||
| c.Location = c.Positions[PositionType.Sanctuary]; | ||
|
|
||
| // Check to see if the database position is valid |
There was a problem hiding this comment.
I fully see that this is unnecessary, but prevents an imaginary scenario where the position has become 0 for xyz coordinates and the landblock is invalid. This may happen if someone uses the reset-pos command to set location as an invalid position.
There was a problem hiding this comment.
Do X/Y/Z really matter? Just seems like unnecessary overhead. Will also likely cause problems in dungeons that go outside the bounds of "normal" X/Y/Z values of the open world.
There was a problem hiding this comment.
The only real case this check prevents is in where someone has used @reset-pos 1 and their location has become invalid.
I will remove this check and the database debugging command as well, too try and prevent attempt at preventing invalid locations.
…ulator#243. Tests fix for Issue ACEmulator#242
Source/ACE.Entity/Position.cs
Outdated
| { | ||
| get | ||
| { | ||
| return new LandblockId(Cell); |
There was a problem hiding this comment.
This doesn't feel helpful. Essentially re-allocating a new LandblockId every time it is accessed. I agree the section below in the constructor was a bug, but I don't see the bug in this part.
There was a problem hiding this comment.
I do see how creating a new object every time will be inefficient, so I will work to create private accessed variable, instead.
| else if (c.Positions.ContainsKey(PositionType.Sanctuary)) | ||
| c.Location = c.Positions[PositionType.Sanctuary]; | ||
|
|
||
| // Check to see if the database position is valid |
There was a problem hiding this comment.
Do X/Y/Z really matter? Just seems like unnecessary overhead. Will also likely cause problems in dungeons that go outside the bounds of "normal" X/Y/Z values of the open world.
fantoms
force-pushed
the
fix-PosisitionMissing
branch
3 times, most recently
from
7fafda5
to
cef5fb3
Compare
…ic to character database, converted getpositions to dictionary. * Updated comments and variable name, attempting too avoid confusion
fantoms
force-pushed
the
fix-PosisitionMissing
branch
from
cef5fb3
to
1c8fd64
Compare
|
I've made changes to each of the commented sections - the code is now ready for another review. |
This code was created to address issue #242.
-Fixed bugs in
LandblockIdin position and server now resets the position if missing.-Added checks to prevent a missing location from crashing the server.
-Removed unnecessary
lastportalrecall position save, this is set on demand, this also prevents invalid data from entering the database.Please give this a test by deleting a previously created character's position location (position type 1) and then try logging in, you should be at the current starting dungeon.