Fix issue with position calculation in move function

[omarcopires]

Pull Request Prelude

• I have followed proper The Forgotten Server code styling.
• I have read and understood the contribution guidelines before making this PR.
• I am aware that this PR may be closed if the above-mentioned criteria are not fulfilled.

Changes Proposed

This pull request fixes an issue with the move function that was causing incorrect position calculation. The issue was due to the use of the + operator to add a Position object to a directionOffset object, which was resulting in unexpected behavior. The correct behavior is to create a new Position object with the correct coordinates.

To fix the issue, the code has been modified to create a new Position object using the x and y values of the current position, and the x and y values of the directionOffset object. The z value is left unchanged since we're assuming the character is moving on the same floor.

This fix has been tested and verified to work correctly in a variety of scenarios.

Issues addressed:
Close #4316

[projectMWX]

Just to clarify, made this fix from my phone laying in bed. Was going to check to see if there was a magic method set anywhere and if so, why the addition isn't applying properly. But if not, Omarcopires is happy and tested with no problems.

[omarcopires]

Just to clarify, made this fix from my phone laying in bed. Was going to check to see if there was a magic method set anywhere and if so, why the addition isn't applying properly. But if not, Omarcopires is happy and tested with no problems.

I correctly applied the fix, but now it's working as it should.

[projectMWX]

Position.directionOffset is not a valid Position even with the z co-ord. The meta requires both sides of the addition to be Positions. And we can't create Position offsets i.e Position(-1, 0 0) as they require unsigned co-ordinates

[MillhioreBT]

This is not correct, the Position are tables, it is enough that the first table has the Position metatable to call the __add metamethod.

as Position are tables also tables can be Position.
I don't know what solves this, if there is nothing to solve, in fact the code you propose is wrong.
this is a quadruple double indexing:

In any case, if there is a problem, it is just a matter of converting the directionOffset table to Position table instead simple table.

[projectMWX]

Have you even bothered to test the code? I wrote it from my phone laying in bed to help a dude on OTAcademy and to be quite honest, didn't want to be tagged into this PR or anything affiliated with TFS for this exact reason.

As it stands, you can't create a Position with a negative value, it's that simple. The code I wrote for him fixes it. It's up to you to include it or create a separate metamethod.

[nekiro]

Position.directionOffset is not a valid Position even with the z co-ord. The meta requires both sides of the addition to be Positions. And we can't create Position offsets i.e Position(-1, 0 0) as they require unsigned co-ordinates

You can create them explicitly like 65535, y, z, if z is 0 then __add will add nothing to the z position, so its safe to use 0 as z
-1 probably underflows later in the c++ world and does addition on 65535 anyway, which overflows the number, so it should work, but someone has to check it.

[projectMWX]

Position.directionOffset is not a valid Position even with the z co-ord. The meta requires both sides of the addition to be Positions. And we can't create Position offsets i.e Position(-1, 0 0) as they require unsigned co-ordinates

You can create them explicitly like 65535, y, z, if z is 0 then __add will add nothing to the z position, so its safe to use 0 as z -1 probably underflows later in the c++ world and does addition on 65535 anyway, which overflows the number, so it should work, but someone has to check it.

I see the logic, but definitely isn't working based on what I've seen. I don't have a copy of latest to test, but the reason it's gone un-noticed for this long is due to the fact that most stairs wont have a default south blocked tile. I think the lua fails and halts the script before the server can process the underflow/overflow, but it's only a guess.

[nekiro]

Position.directionOffset is not a valid Position even with the z co-ord. The meta requires both sides of the addition to be Positions. And we can't create Position offsets i.e Position(-1, 0 0) as they require unsigned co-ordinates

You can create them explicitly like 65535, y, z, if z is 0 then __add will add nothing to the z position, so its safe to use 0 as z -1 probably underflows later in the c++ world and does addition on 65535 anyway, which overflows the number, so it should work, but someone has to check it.

I see the logic, but definitely isn't working based on what I've seen. I don't have a copy of latest to test, but the reason it's gone un-noticed for this long is due to the fact that most stairs wont have a default south blocked tile. I think the lua fails and halts the script before the server can process the underflow/overflow, but it's only a guess.

There is no checking in lua that would "halt" the execution, calls from lua go directly to C lua api, because __add is registered there.
-1 gives a warning, because of the code that checks number validity in getNumber<T>, this is "recent" addition.
Even though it gives that warning the code will still go thru and work.
The fix in this case would be to replace -1 in Position.directionOffset to use 65535 and no other changes to the code.

By defining it like that you can see, how ugly this hack actually is.

To be entirely honest about how this should be handled. TFS should have Point (or something with a better name) metatable, which would define __add __sub and other operators to work directly with positions, in this case we could simply do Point(0, 2) or Point(-1, 0, 0) and add it to position. It would accept signed integers, so no issues with that. Setting -1 to Position is an ugly and weird hack to make it seem like its an offset.
Or just parse signed integers (int32_t) in getPosition() and cast properly, when constructing Position (less work)