Refactor Point3D/GameLocation #472
Conversation
|
This is to remove some duplication around coordinates and to make the code around movement easier to read. It touches a lot of places and therefore there is also a lot of places to introduce accidental regressions. I am going to convert this to a PR in a month and merge it a week later if there doesn't surface a big problem in the meantime. |
|
|
||
| namespace DOL.GS.Geometry; | ||
|
|
||
| public struct Vector |
There was a problem hiding this comment.
I was wondering, why don't you use System.Numerics.Vector3? It should be more optimized with its use of SIMD instructions but maybe you really want integers?
There was a problem hiding this comment.
One can change the internal implementation of the Vector class later on. As we need integers at some point I feel like using int upfront is maybe the better call, but I'd be surprised if it is noticable at all either way.
| client.Player.Y = realY; | ||
| client.Player.Z = realZ; | ||
| } | ||
| client.Player.Position = Position.Create(newZone.ZoneRegion.ID, coordinate: newLocation, heading: (ushort)(headingflag & 0xFFF)); |
There was a problem hiding this comment.
I think, you can have a data race condition, C# says an assignment to an int (for 32bits, a long for 64bits) is atomic but not for a struct.
I did the same on my server and sometimes, I had an invalid position in other parts of code but I continue to have some even with an Interlocked call.
There was a problem hiding this comment.
How/Where did you have invalid positions? This sounds more like a bug rather than a race condition. I could make Position and Coordinate a class of course, but not sure if it is actually possible to cause a race condition like this.
There was a problem hiding this comment.
I did some testing and could trigger a race condition with struct assignments (regardless of the byte length btw). This is fixable by making Position a class (as expected), however given that the GameLiving.Position is inside an immutable class (Motion) this is already only a problem in static objects (the ones that are not subclasses of GameLiving), where the Position is rarely changed.
It would be valuable to know where/how you had invalid positions, as this is likely a bug still.
There was a problem hiding this comment.
Yes, it's fine with a class because a reference assignment is atomic in C#.
I don't think it will be very useful, my server is heavily modified and I use Vector3 which use XMM registers internally (maybe it's even worse for data race conditions).
Add Angle, Coordinate, Vector, Position Add Motion Add DataObjectPositionExtensions
Pets are not porting back and forth as often while running Also mitigates staccato movement on older clients
NetDwarf
force-pushed
the
RefactorPoint3D
branch
from
524d244
to
71920e0
Compare
Add GameObject.Position and .Coordinate Obsolete GameObject.X/.Y/.Z Remove inheritance from Point3D in all classes (also GameObject) Add GameObject.Orientation Obsolete GameObject.Heading Obsolete GameLocation (replaced by Position) Various deprecations and refactorings relating to Geometry
Make overshoot vector respect Z-axis Make Motion.FullDistance/.RemainingDistance ignore Z-axis
Replace .GetLocationAfter with .GetPositionAfter (and return Position) Replace .CurrentLocation with CurrentPosition accordingly .Start is now a Position
Deprecate old version
Replace ushort+Coordinate parameter with Position Replace GameLiving.GroundTargetLocation with .GroundTargetPosition
NetDwarf
force-pushed
the
RefactorPoint3D
branch
from
71920e0
to
71cf1a0
Compare
To alleviate migration
Previously the player was ported to the actual non-instance instead
Also rename other occurrences of locations that are coordinates This is to reduce ambiguity
NetDwarf
force-pushed
the
RefactorPoint3D
branch
from
3f8730e
to
0ac11e6
Compare
Squashed commit of the following: commit f4d1210 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Fri Jun 14 20:28:26 2024 -0400 improv: remove old Geometry types commit 0dd0055 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Fri Jun 14 20:17:55 2024 -0400 fix: fix OnPlayerMove not being called commit 5eaf744 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Fri Jun 14 20:03:24 2024 -0400 fix: tentatively fix failure to interrupt threads on exit commit 4a81eaf Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Fri Jun 14 18:43:42 2024 -0400 fix: fix exception in format in GameTimer.cs commit a7b55b7 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Fri Jun 14 18:43:18 2024 -0400 improv: cache Region in Position commit fd55949 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Fri Jun 14 18:27:14 2024 -0400 Fix Areas, I think commit 507a728 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Fri Jun 14 18:05:23 2024 -0400 Fix overflow when calculating very long distances commit aa3e1e5 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Fri Jun 14 17:11:03 2024 -0400 improv: improve "Zone does not exist" error log in AddToWorld commit dc292cd Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Sun Jun 9 18:01:06 2024 -0400 Fix some issues commit f59ed07 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Sun Jun 9 15:59:17 2024 -0400 Apply position changes to Gondwana code commit b0d12d2 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Sat Jun 8 16:14:51 2024 -0400 Last batch of importing the DOLSharp positions - still need to change Gondwana-specific code commit 966394b Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Fri Jun 7 19:23:28 2024 -0400 More of the position changes, still not finished commit a873193 Author: Amber Ehrlich <miuna.oshino@gmail.com> Date: Sun Jun 2 19:58:37 2024 -0400 positions, part 1
No description provided.