Automatic aim ai rework

[Coolthulhu]

Changes quite significantly the behavior of some hacked bots (tested on tanks, chickens and skitterbots) and turrets with area of effect attacks.

Hacked bots will no longer depend on distance from player to pick an attack and will not skip turns if they can't see the player. Before, they'd attack zombies with the same attack they'd use to attack the player.

Bots (skitters, tanks and chickens) can now zap enemies when hacked. Currently this is always successful when attempted (because player can only dodge tazing with uncanny_dodge). Stun times and damage copied from iuse::tazer.

Made mutant-ignoring turrets attack mutants driving vehicles, because weird animals don't drive vehicles.

Moved range setting of turrets from hardcoded to a json member.

Made turrets consider area of effect of their projectiles. They will ignore any target too close to attack without risking including self in the area of effect.

During my testing it worked nicely, so it's probably fully or at least mostly functional.
I changed the structure of monattack.cpp quite significantly. I moved aiming from functions like frag_tur up to the functions that use them, like multi_robot and chickenbot (otherwise hacked chickens and tanks would aim twice per attack).

I sent a conflicting PR yesterday (manual turrets - conflicting on incredibly minor things, but still), so this is more for a review right now.

[Rivet-the-Zombie]

Nice!

[KA101]

You removed the underscore elsewhere, may as well nail it here too.

[BevapDin]

Why a pointer and not a reference? Also, below, the this-> is redundant.

[BevapDin]

Player character and npcs can use the same branch, a npc is "kind of a player", npcs have the very same functions that a player has. Also, below the add_msg will be shown when the npcs or the monster is not visible to the player. add_msg_player_or_npc will handle that case.

And why is the npc hurt differently than the player? hurtall(rng(5,20)) vs apply_damag(rng(1,3)*rng(1,5))

[KA101]

Not sure why these weren't fixed but I'll implement in the merge.

[BevapDin]

This code "nicely" ignores the (non) visibility of digging or submerged monsters. That thing was handled in player::sees.
Edit: and it considers hallucinations, but I assume that most code in monattack does this.

[kevingranade]

If by "inline" you mean "copy the contents of player::sees(), please don't, it's just asking for it to go out of sync at some point in the future.
It's much preferable to extract it to some third location and have it included in both player and monster. Possibly reasonable to go in creature, but then again what would be really nice is a new class that has an instance included in both.
For now just avoid duplicating the code by extracting it somewhere as a helper function or something.

[BevapDin]

fire_t is very important, why have you removed it?

[BevapDin]

line_to will generate different lines depending on that t parameter, map::sees uses the same code. It's quite likely that one of those lines allows seeing the target , while another line goes through opaque terrain and does not allow the point to be seen. The sees function stores the number that will create a line through transparent terrain in the t parameter. Passing the same value to line_to ensure that the generate line is the same as the line used by sees.

[Coolthulhu]

OK, I get most of those, except the fire_t one.

I removed fire_t, because the only function that used it was flamethrower. All others would simply declare it, then forget about it as soon as auto_find_hostile_target returned. What is it used for? Does it ensure the fire trail follows the correct path or something?

EDIT: Tazing has different damage for player and NPC, because player taze code is from monster function, while NPC one is from iuse::tazer. Should I nerf NPC taze back to player taze level?

[BevapDin]

Instead of setting this here, why not as the default value when it's loaded from json: jo.get_int("range", 12)

[BevapDin]

map::sees checks several lines, if any of them allows seeing the target, it returns true and stores the "number" of that line in the t parameters. Later line_to uses that parameter to generate the same line that sees had been using. If line_to would use a different t value and thereby generate a different line, it would fire through solid/opaque terrain, even when the shooter can clearly see the target and knows a clear path to it.

You can try this out: go into ranged.cpp, there are several places that have

int t;
if(sees(..., t)) {
   traj = line_to(..., t);
} else {
   traj = line_to(..., 0);
}

Change them to always use 0 as t parameter and no sees check:

traj = line_to(..., 0);

Than try to aim along a wall:

@
###########Z
          #   Z
          #     Z

[Coolthulhu]

Is it OK if I retrace the lines for the bresenham value? It (the bresenham parameter) is only used in flamethrower code and map::sees is checked for every monster loaded, so it shouldn't be expensive to check it once more for one monster 5 tiles from us.

Flamethrowers are back to being perfectly accurate.

[BevapDin]

I removed fire_t, because the only function that used it was flamethrower. All others would simply declare it, then forget about it as soon as auto_find_hostile_target returned.

That is either a bug, or the functions call player::fire_gun, which does the the same process again: calling map::sees, taking the t parameter from there, so they don't need the already calculated value from fire_t.

Is it OK if I retrace the lines for the bresenham value?

Sure. I'm only aware of this bresenham line thing as I once tried to remove that parameter from all line_to and sees calls (always using the default line), and that had terrible results.

[KA101]

OK, rebase this and I'll see about fast-tracking it.

[Coolthulhu]

Seems to work after merge. At least I couldn't get it to not-work.

[KA101]

Differential stun-amounts are suboptimal. Knocking 'em all to 1-20 damage and 50x move-loss multiplier. Net effect, taser's less effective at stunlocking critters/NPCs and more effective at stunlocking you. Nerf.

[KA101]

Changed the message but in testing the skitterbot refused to target the nearby NPC, instead tazing me whenever I got within 2-3 tiles.

Bug: tazers don't have that much reach and the NPC was the active threat. Sorry, sending it back.

[Coolthulhu]

OK, I added tazing NPCs, either hostile or non-hostile (depending on mon's attitude). I didn't include your balancing change, because it kinda goes out of scope. Also because skitterbots are quite scary to new survivors already.

I nerfed NPC tazing to player tazing level. NPCs who somehow managed to acquired immunity to electricity should now be immune to tazing (I couldn't think of a good way of testing if it works, though).
Monsters will be tazed as hard as with a player-wielded tazer. IMO this makes sense, because hacked bots with tazers are probably going to be tank drones or chicken walkers rather than skitterbots. Eventually this should probably be split into tazer and "lightning gun", but I'd rather keep balancing out of this PR.

I discovered a problem with friendly monster AI vs. hostile NPCs, but it also is out of scope of this PR. Friendly mons can stumble into tiles occupied by hostile NPCs.

[BevapDin]

You can use player::add_msg_if_player_or_npc here. Is has the additional advantage that it prevents messages if the npc can not be seen by the player.

Same below, calling die on g->u is fine, you can skip the check for foe being g->u and call die directly. Note however that you can not erase g->u from the active npc list, that still needs a check for foe being a npc.

[KA101]

Fair enough on the change, that was to reconcile the three different standards (which you now have) and we can revisit it later if need be. Might want to implement BevapDin's there though.

[Coolthulhu]

That failed check is bot losing connection, not a compilation failure.

[KA101]

Nice to see someone clicking the details link. ;-)

[KA101]

Next is gonna be tweaking it to not use explosive rounds on squirrels, cats, etc but for now I think it's workable.