Walkpath Problem

[Lemongrass3110]

I found an infinity loop when trying to loot an item.

How to reproduce?

1. Drop an item.
2. Put a character on the right cell beside the dropped item.
3. Walk to the right cell next to the chracter.
4. Try to grab the item.
5. You will be stuck in an infinity loop walking against the character and back to the cell where you stood.

I think that might be because of the cell stack limit(1 unit per cell) and when trying to grab the item it might want to place my character on the same cell as the other character.

[RadianFord]

Yea confirmed. I tested this one..

@offtopic I was wondering if this can do something with it?
/// Uncomment to enable the Cell Stack Limit mod.
/// It's only config is the battle_config custom_cell_stack_limit.
/// Only chars affected are those defined in BL_CHAR
//#define CELL_NOSTACK

[chriser-]

confirmed too, happend to me by accident yesterday, but I i did not care because I was thinking this was a hickup :D

[chriser-]

I had a quick look into it and the problem is exactly as @Lemongrass3110 described.
You try to walk one cell to the left to pick up the item there, which would place you on the same cell as the other char.
official_cell_stack_limit: 1 is preventing this to happen.
official_cell_stack_limit: 2 is a workaround.

[cydh]

meh, this kinda can be exploited by players and flood ur server with "hickup"

[Playtester]

This is official behavior.

[chriser-]

wow really? o.o
I think this behavior is still really bad...
maybe it is also a bug on the official server.

[MrAntares]

@Playtester No, this is a stupid behavior.... C'mon, this is obviously a bug and it is annoying at least make a conf for it...

[Lemongrass3110]

I tested it on fRO/euRO and there the behaviour is as follows:

1. The character runs into the second character ONCE
2. Targets the right cell beside the second character
3. Walks to this cell(=> looking to the right side)

@Playtester told me on skype, that it depends if some kind of "repeat action mode" is running on the official server. Hope I understood that right.

[Playtester]

Yeah I case we can say that the repeated action is a problem. On official it goes into a loop too but only if you target a monster with auto-attack or just keep the mouse button pressed, but if you only press once to pick up the character will move one east and then stand there.

I'm not sure why we repeat the action for looting as I'm pretty sure I haven't added command queues for looting commands, so I assume the client keeps sending us a request to get the item. Maybe we need to tell the client to give up somehow but I'm not sure what data to send. But in any case, you can easily stop the loop by clicking elsewhere.

Apart from the repeat bug, it's still official behavior and I wouldn't consider it a bug. It's just that to prevent cell stacking you have to find a closest freecell and you need to start somewhere. You always have the option to switch off cell stacking by increasing the limit to a very high number.

[MrAntares]

I simply don't understand why cell stacking is such a big issue... we can still stack up people on cells even with all this hassle. Just cast a skill during walk or simply use a jump skill like back sliding. If I could I would turn off the whole official walkpath thing, because it's just annoying, but unfortunately the non-official walkpath wasn't fixed and monsters lag through walls... I'll try setting the stack limit, thanks, but I'm a bit afraid that it will cause other issues with skills where stacking does make a difference (Grimtooth). If the client wasn't looping around it would be much more better.

I'm not sure if it was my luck or it is an actual rule, but I noticed that if a player drops an item, it always falls to the ground on the left(west) side... soo this is pretty much a trigger for this looping can't get movement. Maybe the item should appear on the right(east) side, where the other player stands by default during pickup, so the whole looping could be averted. At least it would be logical, if this right hand standing on pickup is written in stone.

[chriser-]

items can drop on every side of the unit.

[Lemongrass3110]

@Playtester
This is not item pickup problem. Everything regarding this problem happens in:

1. clif_parse_WalkToXY
2. unit_walktoxy
3. unit_walktoxy_sub

I guess something is wrong here because the server tells the player that he can go there but will not let the player reach the cell because of the stack limit.
As far as I have seen we check CHK_PASS here which is correct, because if we do not he will get rejected directly. But somehow we have to stop this whole loop at the point when the unit is moved on top of another unit. The steps that should be done are:

1. Set the walking direction to the opposite direction
2. Walk to the last cell
3. Stop walking

Update:
Basically I think we just miss the part of the logic regarding "CELL_CHKSTACK". This cell check type is defined but never really used. Might be useful for this case though.

[Playtester]

@MrAntares
Well for me the no cell stacking was important to implement because of how different the experience is. It was pretty much exactly this case here that made me notice how it works on official servers. On officials you can't just approach the same monsters from the east when a friend is already attacking it, you will always be moved one cell east again and again. You had to click on a free cell to join your friend. On emulators it was possible to just attack. It was a huge difference in he gameplay experience, so I felt I had to get this right on emulators as well.

I also like that it makes players spread around the mob that is attacking instead of all of them attacking from the same cell. It's quite different. Especially if you are a healer it's nice that you don't have all your party members standing on the same cell.

There are a lot more situations where it matters for me like for example you have several looter mobs around and drop on item. On emulators they all walked on the cell of the item and cluttered together there, whereas on official they all spread out immediately to a free cell.

And of course grimtooth hunting. This is a better protection than the "make monsters spread out" config.

But as said you can switch off the cell stacking. I don't think it will make the walkpath display break because that only breaks if the walkpath is larger than 14 cells.

@Lemongrass3110
Hmm but the way it works on official and that's how I implemented it:

1. Client sends "Go to XY"
2. Server walks unit to XY, regardless of whether there is something in the way or not.
3. At move end, server will check if XY is free, if not, it will issue a move command to the closest freecell.

The question is why after step 3, the character starts walking to the item again. I can only think that the client sends another move request.

[cydh]

then let's call it feature lol since when get the closest freecell always be to the east. :P
if item (i), player (p), and player (u) who want to take it from south to north, it will ended the u will stand at the east of the i

  [i]
  [p] [u] (resulting [u] will stands here before picking)
  [u] (1st action targeting to pick the [i])

[Playtester]

Yes, I investigated a lot on Aegis to figure out the exact order of cell priority. East has highest priority, then N, W, S, NE, NW, SW, SE.

[cydh]

yeah I know that, u do it perfect... almost? just because except that endless loop that @Lemongrass3110 said.
in official server, (example by using first post pic, apple, WL, GM), when GM click the apple it walk to WL position and because no stack allowed, GM backs to first location then it stops to pick the apple.
I was trying to debugging, but lost somewhere.

[Lemongrass3110]

@Playtester @cydh
http://youtu.be/i4lGYTLsGBs

[cydh]

can u convert that video to light novel or somewhat that I can read? haha

[Lemongrass3110]

I already did: #198 (comment)

[Playtester]

Hmm so if you are in the "Item <-> Obstacle <-> You" situation (with no cells in between) and then click, you won't even start moving?

[Lemongrass3110]

Yes as you can see in the video.
And since this is an official server(but not kRO) I can't really understand
our current implementation.

[Playtester]

Sounds to me that we simply need to add a block for client commands that issue a move request one cell to the west when there is an obstacle.

[Lemongrass3110]

To me it at least looks like a bigger problem. Since we also don't have the "if you can't move to the left anymore -> walk back" feature

[Playtester]

We have. I implemented that. That's what causing the loop.

[reigneil]

is this clientside or serverside issue., any progress? my test server reproduce this also using 2014-06-13aRagexe caught in an endless loop trying to pickup loot.

[Lemongrass3110]

Definitely a serverside problem. No info on the progress though

[Playtester]

Sorry, my health doesn't allow me to code for long times and since this would require debugging a lot to fix, I currently cannot do it.

If someone could tell me how it loops (the order of function calls done repeatedly) I think I could find a way to fix it.

[Lemongrass3110]

I will look it up for you. But it is not really a serverside loop.

It is because you send "It is OK to walk to x/y" but then stop walking
without reaching the desired destination. That seems to trigger an infinite
"retry" loop in the client. So this is caused by sending wrong packets
maybe. Might have to capture the packets that are sent on fRO.

[Playtester]

Hmmm yeah that's a bit harder then if it's missing client info since I don't know what the client expects. We have packets for "can't reach target for attack" that causes you to follow your target in that case. Maybe if the target is BL_ITEM this should not be sent to the client?

[Lemongrass3110]

Ok, so the packet capture shows the following packets:

1. 5F032D4810
   => 5F03 means packet id 035F which is a move request(clif_parse_WalkToXY)
   => 2D and 48 are our target coordinates
   => 10 is our direction, which gets ignored serverside anyways

2. 8700541BF30B2E0812D48188
   => 8700 means packet id 0087 which is client is walking(clif_walkok)
   => 541B F30B is the server tick
   => 2E and 08 are our current coordinates
   => 12 is our current direction
   => D4 and 81 is our current target
   => 88 seem to be constant values we also send always

3. 87001E1DF30B2D4812D88188
   another clif_walkok, but as you can see, some values(not talking about the servertick) have changed.
   => 2D and 48 are our current coordinates(obviously)
   => 12 is our current direction
   => D8 and 81 is our new target

4. 5F032D4810
   another walk request from the client

5. No answer from the server, not even worth an answer it seems.

To sum everything up:
Basically we just have to ignore the incoming walk request!!!

The setup is the same as discussed before. Character blocking the way to the item lying on the left cell next to him. Looting character coming from right.

[RadianFord]

I applied the fix of lemon should i delete it or its fine to stay?

[Lemongrass3110]

You should delete my version. It would do a double check I think

[RadianFord]

@Lemongrass3110 okay thanks!

[Playtester]

Lemongrass's version would also block movement from any direction other than just west, but it's not needed to block any other direction as it won't lead to a loop, it also only checks if the setting is exactly = 1. Other than that there won't be any conflicts having both. I still recommend removing it as it will updating easier to have as little changes as possible.