[RDY] Bresenham overhaul

[kevingranade]

I started poking at line_to() vs shadowcasting agreement, and things unraveled a bit.
The two main things here are 48b7e90 and ebec1d5, the rest is kind of opportunistic tidying.
The first is a bug in the bresenham algorithm, and the second is an intentional behavior I think is the wrong way to go about things.
Basically monster pathfinding would work by trying to draw a line to the target using several different origin points in the starting square, and using the first one that worked. This was intentionally a modified version of a line-drawing algorithm that operated backwards from usual to bias monsters toward advancing in parallell lines as they approached their target. You can see what this is trying to avoid if you revert ebec1d5, a clump of monsters approaching the player will form into a line and get in each others way to a pathological degree.
Instead of biasing the vision function to have this effect, I added random variation to monsters as they move toward a target.

Fixes #5668

My main open questions (not open any more):
[x] Should this apply to sound and scent tracking as well?
[ ] Should some monsters get a speed bump to compensate for their wandering? As it is they effectively lose a bit of speed due to taking very non-ideal paths toward the player, they act a LOT more like a disorganized horde though, I like the effect a lot.
[x] Should some monsters not do this? Possibly gating it on the "STUMBLES" flag?

Then todos:
[x] Fix/replace line_to() unit test.
[ ] Write some unit tests comparing monster and player vision, ideally they'll be totally symmetric.

[Rivet-the-Zombie]

• I'm all for it, if it's not too resource-intensive - it may speed it up; I'll be the first to admit that I don't really understand such things.
• I don't think we should be boosting anybody's speed along with these changes right now. It would be better to implement the changes and wait to see if a speed boost is necessary, so that we can later adjust it with a bit more knowledge of what effect the new pathfinding has on its own, if it actually needs to be adjusted at all.
• Maybe they could exhibit additional path scattering if they have the STUMBLES flag, in addition to their normal stumbling action?

[Coolthulhu]

Not a new bug; but this doesn't check NPCs. But maybe that's good, as NPCs are iterated over linearly.

[kevingranade]

That's a good point, how do they end up attacking NPCs? or do they? ISTR reports of them just standing next to NPCs and getting killed.

[Coolthulhu]

Probably not a big performance issue, but squares_in_direction builds the entire line just to discard all but the first point.

Overall looks good on paper. Didn't see the new hordes yet, but I can imagine how it looks.

Their bashing and pushing chances could be lowered, perhaps tying square selection to bash/push somehow.

As for the speed buff: it sounds like only zombies in euclidean distance mode will need it. Hacking away might be better than just giving them a flat buff.
For example, penalizing them for attempting to move diagonally (even if they move orthogonally due to square selection) rather than for actually moving diagonally.

[DavidKeaton]

Is it the shambling that you wish to add this speed variation to? Would a random adjustment (+-) to their speed make them uneven enough to imitate shambling?

[kevingranade]

No additional cost to doing this with scent and sound, it would just get applied after a target is chosen instead of in the vision pathing specific code.
I'm fine with wait and see, it doesn't slow zombies down enough to break the game, they're just a bit easier to get away from than they were before.
There are two places that could add additional stumbling, one is the check that excludes squares from consideration if they don't move the monster closer to its target, and one is the already extant one that adds extra random motion to handle stumbling, though I'm considering taking the second one out. I also haven't played with the probabilities at all.
Yea squares_in_direction is fairly wasteful, but it hasn't been a problem so far.
Aah crap you're right Coolthulu, they won't have any slowdown at all from this in square distance mode, so any compensation for this should be distance mode aware.

To be clear, my goal with adjusting speed is that when testing this, zombies were much easier to run away from (although as Coolthulu points out, I was only testing with ecludian distance), to the point of being able to skip every other turn while walking away from them and still maintain distance.

[DavidKeaton]

Well they are (mostly) shambler zombies, so they won't be very fast.

[BevapDin]

This should probably call critter_at so it can recognize the player character and any NPCs as well.

This would also allow to get rid of the explicit usage of g->u at line 424.

[i2amroy]

Speed buffs have the disadvantage of letting zombies that actually reach the player hit more, though, so if you're going to go that method you're probably better of just cutting the amount of moves it costs them to perform a "bad" path move instead of being able to move in the correct method. Maybe handwave it as them stumbling particularly fast or something to account for it.

As noted nothing like that should be needed in the normal old rougelike movement style due to a zig-zag diagonal path and a straight one costing the same, though, which AFAIK is the type of movement most players play with (mainly because it's the default setting).

[kevingranade]

Speed buffs have the disadvantage of letting zombies that actually reach the player hit more,

I'm not sure I agree it's a disadvantage, zombies could stand a little buff. :)

[Coolthulhu]

zombies could stand a little buff

Agreed. I play with Fast Zombies mod and the buffed zeds are well doable. The boost to movement speed is way more dangerous than their extra attack speed. The extra attack speed is only really dangerous when it allows them to bypass player's ability to dodge/block (due to the whole 1 dodge/turn thing).

[BevapDin]

Is using next here correct? It looks like it should be candidate.

[Coolthulhu]

I think it is supposed to work like this: first mon checks the exact tile it wants to move to and marks it as next, then it doesn't switch from that tile if it contains a hostile target.

[kevingranade]

I had forgotten about the other reason that map::sees() did a weird scanning thing, and that was to find a clear line between the player and their target to send projectiles down, so I needed to reimplement that, but explicitly now.
This turned into a monster, took quite a while to figure out what was up with the nuances of bresenham handling, and that it isn't a given that you can draw a bresenham line to anywhere that shadowcasting says you can see.
If you want to see the extent to which they do NOT agree, run tests/cata_test bresenham_vs_shadowcasting. Unfortunately in the meanwhile I fixed some bugs in my new vs old shadowcasting test, and discovered that they don't always agree either. Worse, I'm not sure which one is objectively more correct. I'm just not ready to dive into that morass again right now, so I just disabled the tests by default.

[kevingranade]

And now it's really a monster, I remembered I removed the last use case for the "biasing" arguments to map::seen() and line_to(), so in the last commit I stripped them out.

[Coolthulhu]

I can't get zombies to reliably stumble around transparent terrain. With this setup:

.-...@
Z.....

(Z is regular zombie, @ player and - is a transparent terrain that won't get bashed down, like reinforced glass)
Zombie will bash the glass indefinitely.

I can't get them to stumble in the open either. Except for crawling zombies, for some reason - those stumble reliably both in the open and next to a glass.

[Coolthulhu]

A more minor issue that may have existed for a few versions, but may have been added (or at least exacerbated?) in this PR:
Missing a ranged attack through a tiny opening is very unlikely to hit the walls. For example:

..........|..........
@.........0.........Z
..........|..........

Due to how missing simply picks a different target point rather than a different bresenham for the line, the only way to hit any of the walls to the side of the window is to have the miss cause a non-visible new target point to be picked.
As long as the player can see the new target, the shot will not hit any of the walls, because all the LoS lines will go through the window.

[Coolthulhu]

Rest seems to look OK

EDIT: Forgot to ask: what about the euclidean/square discrepancy? I didn't see it addressed and it could well make euclidean zombies 1.5 times slower than their square counterparts when moving in a straight line.

[kevingranade]

Gah, thanks for catching those, I broke the stumbling logic at some point in refactoring. I think the firing through a window thing was pre-existing, but it's fixed now too.

[kevingranade]

I indeed left distance type alone, ecludian movement zombies will be slowed down more by their stumbling than roguelike zombies will, I'm not too worried about that part, the thing that bugs me is that running away from zombies horizontally or vertically is much more effective than running away from them diagonally :/
Now that I've remembered that, I really need to adjust for it, the balance change is acceptable, the inconsistency is not.

[kevingranade]

I'm trying to quantify the effect of this, and while I thought I saw it happening, I'm not sure I wasn't doing something dumb like checking horizontal movement first, then diagonal, and by the time I checked diagonal my player had gotten cold and slowed down.

So something the monster will definitely do is take a "diamond step", that is mis-step in one direction followed by a mis-step in the opposite direction later (possibly with "correct" steps in between).

horizontal/vertical movement:
regular step costs 1, moves 1 closer to player
diamond step costs 2.82 moves, moves 2 closer to player

diagonal movement
regular step costs 1.41, moves 1.41 closer to player
diamond step costs 2, moves 1.41 closer to player

As far as I can tell the ratio of wasted moves is the same, 1.41 == 2.82 / 2 == 2 / 1.41
Am I missing something?

Anyway, if the only remaining issue is that roguelike and trig zombies are different, I don't really think it's worth hacking around, that's just part of what you're selecting when you choose one over the other.

[Coolthulhu]

Doesn't closest_squares function always return 3 points? I think it would be possible for zombie to execute a "v-move", like this:

..1.
@.2Z

With zed starting at Z, then moving to 1, then to 2.

This would move the zed at 50% speed in square and 1/(1+1.41)=41% speed in euclidean and happen (2/3)*(1/3)=22% of cases where zombie gets 2 moves to a player in a straight line.

[kevingranade]

I don't think that happens because of the first check:

if( !plans.empty() && rl_dist( candidate, plans.back() ) >=
    rl_dist( pos(), plans.back() ) ) {
    continue;
}

Technically it's slightly closer, but rl_dist returns an integer, so it's truncated to 1.

Also in testing I never see zombies making that move.

[BevapDin]

The changes here disable the use of certain vehicle parts for crafting and usage of items from impassable terrain (e.g. display rack).

The function inventory::form_from_map uses map::accessible_items to quickly skip squares that can not be accessed and that are therefor ignored. map::accessible_items checks for a clear path via map::clear_path, which used to return true if only the last square was impassable (and all squares before that are passable).

Now it returns false in that case. Because display racks and welding rigs (and similar) are impassable per definition (and can never be passable), their squares are completely ignored for crafting, always.

Considering that map::clear_path is widely used (e.g. monattack.cpp), I suspect there are other similar problems.

[kevingranade]

Ok, this is how map::sees() works as well, but I assumed (oops) that clear_path() required the terminal square to be clear as well, will follow up with a fix.

[Rivet-the-Zombie]

Oh crap I didn't check this with crafting stuff; just guns and such. My bad!