raduprv / Eternal-Lands

http://www.eternal-lands.com
Other
155 stars 56 forks source link

Enhancing clicking on unwalkable tiles #168

Closed feeltheburn closed 2 years ago

feeltheburn commented 2 years ago

Currently, if an unwalkable tile is clicked, you get the green "X" mark and nothing happens. (The X is not visible in some places due to 3d objects like ground tiles covering it.)

This is particularly noticeable right around edges of 3D objects that may appear to be partly walkable but aren't marked as such. 3D objects of course don't follow walkable tiles precisely. But trying to click these areas can be frustrating during game play, especially when doing things like chasing a spawn in a 3d-crowded area like woods.

Two suggestions:

1- If the tile is deemed unwalkable, don't show the X at all. That'll at least give the player some feedback in the form of no feedback.

2- For unwalkable tiles that have at least one walkable tile directly next to them, have the character walk to that tile instead. (If more than one, walk to the one closest to the player based on x/y?)

This second option would be a massive QoL update, as clicking near the edges of rocks and such won't cause the player to go to a complete stop. And that is the area it's actually noticeable.

(This second option may also help improve the issue of clicking on the circular tab map window and not moving, likely clicking directly on a tree or such.)

gamil-zirak commented 2 years ago

@feeltheburn I submitted a pull request #169 to not highlight unwalkable tiles.

For suggestion 2 I'm not sure that I would want this feature if I'm on a map where I'm trying to be precise in my movements such as in a maze or where there is ground damage. Let's say you click on an unwalkable fence with walkable tiles on either side. The client would need to chose which side of the fence to send you to, could we define a good rule for this?

pjbroad commented 2 years ago

Should we also highlight the actor with the red or amber cross?

gamil-zirak commented 2 years ago

Good question bluap. It really a preferences decision. I tested this and when walking on a map with a lot of 3d objects, I find the constant highlighting in different colors distracting. Others may like having an explicit highlight telling them that a tile is unwalkable. I hesitate to propose that we add yet another option to the options config but I can be swayed if it's valuable enough for others to have this option.

feeltheburn commented 2 years ago

For suggestion 2 I'm not sure that I would want this feature if I'm on a map where I'm trying to be precise in my movements such as in a maze or where there is ground damage. Let's say you click on an unwalkable fence with walkable tiles on either side. The client would need to chose which side of the fence to send you to, could we define a good rule for this?

I'm having a hard time picturing this as an issue, even knowing the maps as well as I do. In the places where it could even be an issue, are those places regularly used? Are they not places to be more careful with clicks in the first place?

Compared to the "daily grind" most players go through, myself included, when trying to click near a wall or rock or something and you end up full-stopping (like the stupid "you're too far away" behavior clicking critters... "Oh, I'm too far away, I'll just completely stop and let them get even further away" ... sorry, off on a tangent on my #1 pet peeve, heh)... right, a huge chunk of the time when you click near such objects, you end up just full-stopping because even though it partly looks like it's walkable, it's not because part of the rock or wall is on it, and players can't see the walk grid.

Just from general gameplay I'd say a good general rule would be as I mentioned, if more than one walkable tile exists, use the one closest to the player. You have the player's x/y, the clicked tile's x/y, use a walkable tile between them.

A check that would pretty much ensure your scenario doesn't happen: Check only the 3 tiles closest to the player, go to one of those if open. That way the "other side of a fence" isn't an option.

unwalkable-tile-clicking

But the simple fact is that people aren't clicking that location to stop, they're clicking to go there. Anything is a massive improvement on a complete stop, just like the you are too far... there I go again. ;)

feeltheburn commented 2 years ago

Should we also highlight the actor with the red or amber cross?

I'm not sure this is necessary. I think "no indication" of movement (and not actually moving) would be enough.

Maybe leave it just like that for a while and try it out. If it's deemed necessary, can add it in later.

gvissers commented 2 years ago

A check that would pretty much ensure your scenario doesn't happen: Check only the 3 tiles closest to the player, go to one of those if open. That way the "other side of a fence" isn't an option.

That is not necessarily true. If '#' is an unwalkable fence tile that was clicked, 'x' is the rest of the fence, and 'P' is the player position, 'o' is one of your yellowish neighbour tiles, but is on the other side of the fence:

...x..
...x..
..o#..
xxxx..
......
P.....

Anyway, I have cursed many a times chasing rabbits because the actor stopped walking, but that was usually because I was 'too far away', not because I clicked an unwalkable tile (I believe that issue has been solved? ISTR it has been brought up at least). I'm not sure the client can provide a good alternative tile in all situations.

feeltheburn commented 2 years ago

It does seem that you're trying very hard to find the least likely incredibly rare scenario to shoot down something that would fix the other 99.9% of areas.

Fine, only if one side is open then. We're looking mostly at near rocks/mountains, walls and things like furniture that take up multiple tiles anyway.

We can actually have a QoL update for players without shooting down every possible chance for it. Perhaps brainstorming for the betterment of play for players instead of just shooting down because "incredibly rare chance" might happen?

This can be fixed. The game actually can be improved for players. We're not talking just about chasing a rabbit so don't limit it to that scenario. Harvesters move around, too. Explorers do. I constantly find myself stopping because I click near a rock, and there's shit all I can do changing the maps to fix that.

We have square walking tiles with non-square 3d objects. Saying the player should just stop for clicking a non-walkable tile when 99.999% of the time they're wanting to go where they clicked is not helpful, even if you find that 0.001% of the time it might cause a problem. Solutions can be had, so there is no rush to just shit on the idea of improving player movement.

gvissers commented 2 years ago

It does seem that you're trying very hard to find the least likely incredibly rare scenario to shoot down something that would fix the other 99.9% of areas.

Not really, I am only trying to find potential problems, and see if they could not lead to bigger issues than the original problem. I haven't even really thought it all through yet, I was just pointing out that the scenario you sketched does not necessarily hold in all cases.

gamil-zirak commented 2 years ago

I think your suggestion Burn is good for objects that are roughly circular or rectangular. When the objects form a long barrier either because of their shape or because of the way multiple objects are placed next to each other, I can see it causing issues. Either way we do want to account for the more rare or complex situations - even if we decide to do nothing in those cases - because a minor and rare inconvenience is more noticeable than something just working most of the time. Let's keep the brainstorming going on how to make suggestion number 2 a good change.

gvissers commented 2 years ago

Proposal:

For unwalkable tiles with at least one walkable neighbour tile, plan the path to the target tile as if it were walkable, than return the path with the last (unwalkable) tile removed. That should give you the path to the nearest accesible neighbour[*] (though it may be a diagonal neighbour, so not strictly the nearest tile), and we don't have to analyse the map to see if it would work.

How does that sound?

[*] probably not strictly true with the current pathfinder implementation, but very, very unlikely to fail. And in any case, I have plans to fix the path finder.

gamil-zirak commented 2 years ago

I like this idea. Having a diagonal neighbor chosen as the closest object is acceptable in my opinion because the point is to keep walking towards the destination and not stop far from it. If a player clicks a tile under a large object with no walkable neighbors, the player would not walk or they would stop walking. This should be less of an issue for clicking in the main game window but for minimap walkers they're more likely to click the middle of a large unwalkable object. I wouldn't worry about this because it's an overall improvement for them also.

gamil-zirak commented 2 years ago

Regarding whether to highlight unwalkable tiles when they're clicked. I think we're ok with the pull request as it is and not highlight the clicked tile. If we change our mind and we decide to add a highlight or make one optional, the proposed pathfinder changes will complicate how we do that highlighting.

gvissers commented 2 years ago

I have created a PR (#171) that allows the player to walk to the tile nearest to an unwalkable destination, if said tile is within 2 steps of the destination. Since it returns the first path found, it shoud avoid the "fence" scenario, at least if there is an equivalent tile on the actor's side of the fence. Small caveat: this relies on the maps marking unreachable tiles as unwalkable; in the following scenario:

###
# #
#d#x
###

where d is the unwalkable destination and the tile above it is not unwalkable but not reachable either, the proposed patch will fail because it assumes it can get within a single step of d, whereas the reachable tile x is two steps away from d.

gamil-zirak commented 2 years ago

Thanks so much. I've been testing this in Irsis which has a lot of unwalkable areas of various shapes and sizes. I've run into no issues at all. I also tested the scenario above where a walkable tile is unreachable and the player clicks a nearby unwalkable tile. While failure only means that the character will not walk to "x" I feel no loss of gameplay quality when experiencing this scenario.

gamil-zirak commented 2 years ago

After more testing I'm running into a problem. I was in NC south cave at [48.49]. There are some fruits in the unwalkable water that I decided to harvest while I was there. If I click too fast on the fruit - before the cursor change to harvest - or if I misclick right next to the fruit I will start the long journey back to the other side of the river. One workaround could be to explicitly set the harvest cursor but I don't know if there is a way to do this with the client as it currently works.

gvissers commented 2 years ago

Thanks for trying it out!

I took a look at the NC cave. I agree that this patch increases the probability of picking a destination on the wrong side of the river. It's not a new issue, if you click the river close enough to the the river bank on the other side you will also start walking back, but this patch only makes the issue worse.

I can't think of a good way to prevent this. Perhaps disable this feature if you are already within 2 tiles of the clicked destination tile? That still wouldn't help you if you click too early, but might make manoeuvring in tight situations better.

EDIT: hmm, just tried that and it doesn't help much. If I stand in the water and click between the two fruits, the destination is three tiles away and I still walk to the other side. Of course we can increase the cutoff distance, but finding an cutoff acceptable for all situations is probably impossible, and the larger we make it, the less useful this patch becomes.

gvissers commented 2 years ago

I have added a change that restricts the length of the path if an unwalkable tile is clicked, to 3 times the direct path length to the destination tile. That should give as some wriggle room, but prevent the worst excesses like in the NC cave. Path finding to a walkable tile should remain unaffected, so it should still be possible to cross a river/fence/hedge/etc by clicking the ground on the opposite side. Just not by clicking in the river anymore.

gamil-zirak commented 2 years ago

This is definitely a huge improvement. Thanks to your creative solutions. I wonder if we should still add the disabling of the feature within a certain distance of the destination because one could be harvesting towards the end of a long river or fence. I think an acceptable cutoff would be the maximum distance that one can harvest a resource. Based on the harvestable location I'm seeing it's between 3 and 4 steps. The walking issue we're trying to solve is when people are trying to get to longer distances but stop. I think that 4 steps is an acceptable cutoff as this feature doesn't really make that much of an improvement for walkers at such small distances. What does a pear hunter like @feeltheburn who does both a lot of walking/running and harvesting think about this?

gvissers commented 2 years ago

I have added a configuration option to toggle the neighbourhood search.

As for setting a cutoff distance: I personally don't think it would add much, but I'm willing to add it if people think it's useful.

gamil-zirak commented 2 years ago

We could test it for awhile without the cutoff distance, and if we encounter an issue, we deal with it then.

gvissers commented 2 years ago

I've been looking at the highlighting, and I thought we could just add a highlight when the pathfinder is successful. However, there are two problems:

I don't think we can resolve the second issue. As for the first, I was thinking we could highlight when either the pathfinder finds a path, or when always_pathfinding is turned off and the destination tile is walkable. Does that sound reasonable?

And another question: if the pathfinder plots a path to a nearby tile, which tile do we highlight? The tile that was clicked, or the tile the actor is walking to?

gamil-zirak commented 2 years ago

I've been looking at the highlighting, and I thought we could just add a highlight when the pathfinder is successful. However, there are two problems:

* People might be playing with `always_pathfinding` turned off (which is the default, I believe). I wonder how useful that is, but that would bypass the pathfinder completely for in-world clicks, and

* (very minor) the server's idea of what can be reached might differ from what the client thinks, so when the pathfinder fails, a direct MOVE_TO command is sent to see if the server can resolve it. Theoretically, the server could return a path where the client fails (e.g. when the maps do not match).

So the new pathfinding to a nearby walkable tile when the target is unwalkable is only present if the range of the walk cursor is extended. Otherwise we get the old method: the character will stop if it's already walking. It seems inconsistent but there's not much we can do to change walking on the server. I'm OK with this when I think of the change as only an enhancement to extended range walking. If we communicate the change this way, it should reduce confusion among players.

I don't think we can resolve the second issue. As for the first, I was thinking we could highlight when either the pathfinder finds a path, or when always_pathfinding is turned off and the destination tile is walkable. Does that sound reasonable?

This is reasonable to me.

And another question: if the pathfinder plots a path to a nearby tile, which tile do we highlight? The tile that was clicked, or the tile the actor is walking to?

I think it would be more consistent with your suggestions here to highlight in green the tile the actor is walking to because if I understand your suggestion for when always_pathfinding is disabled, there would be no highlight on the unwalkable tile. This way, unwalkable tiles are not highlighted regardless of the always_pathfinding status.

gvissers commented 2 years ago

All right, I have implemented the highlighting as described above, so:

From my testing it seems to work as planned. Please ;et me know if this solves the original issue.

gamil-zirak commented 2 years ago

Quick testing of each scenario above shows that the changes are working as expected. I'll continue to use the client with these changes and report any issues. Should we add something to the "Less strict pathfinding" option description to state that it requires "Extended range of the walk cursor" to be enabled? Players might not know that pathfinding is part of extending the range of the walk cursor.

gvissers commented 2 years ago

Quick testing of each scenario above shows that the changes are working as expected. I'll continue to use the client with these changes and report any issues.

Great, thanks for testing!

Should we add something to the "Less strict pathfinding" option description to state that it requires "Extended range of the walk cursor" to be enabled? Players might not know that pathfinding is part of extending the range of the walk cursor.

Yeah good idea, I've added a note to the help text. Ideally we would hide the option when extended range walking is disabled, but I can see no easy way to do that.

pjbroad commented 2 years ago

Agreed this works really nicely. Are we going to include it in the p1 release?

gvissers commented 2 years ago

I think we have taken this as far as we can easily go (yeah I have an idea on how to fix the issue with enclosed walkable tiles, but it's probably not worth implementing).

Are we going to include it in the p1 release?

AFAICT it works well, so if you have o objections, I can merge it into master.

pjbroad commented 2 years ago

No objection from me for merging.

gvissers commented 2 years ago

Ok, merged.

feeltheburn commented 2 years ago

Been occupado....

Testing today so far seems good.

feeltheburn commented 2 years ago

This is in the p1 release and no issues reported, so closing.