User:Crush/Move Code Rewrite
Todo list for movement code rewrite
Client
Coordinate system mess up
The position of a being is stored in the being class with different systems:
- mX, mY
- Pixel coordinates of the center of the tile the being is on
- mStepX, mStepY (also referred to as "offset")
- Difference between real coordinates and mX, mY calculated from the time since the last step and the current walk direction.
- mPx, mPy
- "real" pixel coordinate, calculated from mX + offsetX in Being::logic every tick
This shows a tile-based design pattern which is not suitable for pixel-based movement.
Also, some of these variables are not encapsulated properly and poorly named.
The client should use only pixel coordinates internally (stored in two protected unsigned integers) whenever possible. When the tile coordinates are needed (the only situations I could think of are route finding and path blocking) it should be done by getting the pixel coordinates and dividing them by 32.
Delayed reaction on Being::setDestination()
Another problem is that when setting a new destination while the being is already moving along a path does not cancel the move immediately but only after the next tile of the path has been reached.
Server
Unnecessary abstraction
Currently the server allows to move on the same tile for free. I can't see any good reason for this when we want pixel-accurate gameplay but a lot of possibilities for bugs which can be introduced by this.
Netcode
Use new client->server move commands:
PGMSG_START_MOVE B direction int16 startX int16 startY PGMSG_STOP_MOVE B direction int16 stopX int16 stopY
START_MOVE makes the server route-find the character to startX:startY and then proceed with moving the character to the furthest location in direction which can be reached in a straight line.
STOP_MOVE makes the server abort any current move commands of the character, route-find it to stopX:stopY and turn it into direction
Any messages which make the character perform actions which require it to stop moving (like sitting or attacking) include an implicite STOP_MOVE command and thus should also contain a location and direction.
The client can ommit its position to save traffic when it believes that its current position is obviouse (for example when the character was not moving for a while). In this case the server assumes the position where it currently sees the character.
Discussion
- What I don't like about START_MOVE/STOP_MOVE combination is that when the connection is lost, the player might keep moving for quite a while. Another thing is that since the stop move includes the current player position, the server will always lag behind, and other clients will lag behind even further. So for the character to stop on the same position for everybody, it will often have to walk back some pixels. Third, this method needs a message each time a being changes direction, which happens rather more often than a change of destination. None of these issues exist with our current method of communicating the destination, so maybe somebody can explain what's wrong with our current method? --Bjørn 19:08, 11 August 2008 (CEST)
- In the 0.0 branch, players are centered on the tiles. In the transition to pixel based movement, the positions returned by the pathfinding algorithm have been interpreted as being on the center by adding half the tile width, as far as I know mainly so that the routes taken by beings don't look odd. Why does this rewrite want to get rid of that? What is the proposed solution to having beings walk exactly between two tiles? (something our tileset isn't really prepared for I think) --Bjørn 19:16, 11 August 2008 (CEST)