Vehicle holes #1610

joveeater · 2022-06-09T15:11:25Z

Summary

SUMMARY: Features "Closes the holes created when turning vehicles"

Purpose of change

Prevent the integrity of car walls changing when you turn them.

Describe the solution

First, the new coord_translate, the first commit contains just this if you want it by itself. It works using a const table describing every rotation in terms of a skew between -45 and 45 degrees (y+=x*tan(a)), followed by transposing and/or mirroring x and/or y. This is roughly 4-5 times faster for a 5x7 vehicle and gets better with bigger sizes.

The central logic for vehicle holes is that it checks moves against the unrotated version of vehicles to see if they skip any tiles. If they do, it checks the parts on both of the skipped tiles for opacity/obstruction and if they both have it it triggers the handling. I'm sure you know how the holes work in practice, it's only diagonal moves that can trigger this and it's the two tiles you're squeezing between that get checked.

This does different things depending on where it's triggered. Player movement they're told "You can't squeeze through there." If the player tries to select it as an adjacent tile they're told "You can't reach through there." Pathfinding will not consider it as a path and monsters/npcs that try anyway will fail. Projectiles will be made to hit one of the two tiles randomly before moving on and it suppresses the ability to always see adjacent creatures.

It also affects shadow casting. This is done with a cache built at the same time as the transparency cache by iterating through all of the vehicles and checking all the diagonal moves that can be made around them. This cache stores 2 bits per tile for if light is blocked entering the square going north west or north east, then adjacent tiles are checked for southern directions. When shadowcasting it just ignores tiles that are blocked from the right direction.

Describe alternatives you've considered

For true alternatives there's the fake parts of course. I really don't mind if you'd rather go with that solution.

Past that, removing the ability to select adjacent tiles wholesale might be too indiscriminate, it might be better to do specific handling for different cases and while this is ready, it's not finished. There are still things like fields or scent and I'm sure some others that people with a better knowledge of the engine could maybe point me towards, that aren't using it. Explosions are also using opacity rather than obstruction and I'd like to handle automatically opening doors if there's one blocking your way. I hope to look at these things over the next month or so, but for now the most gameplay important ones are there.

Testing

I've added the ability to spawn vehicles to the vision tests along with 2 simple tests stood inside and outside of a truck at 45 degrees. There are also tests that the new rotation matches the old and that coord_translate_reverse is a true reverse. I want to do a lot more with at least one test for each of the triggered behaviors. Going through a vehicle hole at all is a weird edge case that probably won't be tested manually so I think it's prudent.

Additional context

View from outside:

Lack of view from inside:

joveeater · 2022-06-09T15:20:06Z

Oh before I forget you can cut that cache size down by 8 as well by using the symmetry and mirroring/flipping results.

Zireael07 · 2022-06-09T15:29:45Z

Isn't this kinda a competing solution to 56143 which was just merged recently?

EDIT: Whoops, this is BN, not CDDA.

KheirFerrum · 2022-06-09T21:31:22Z

I'd set it down as a draft. You can do that by looking over to the right bar where it lists reviewers. There should be a line that says "Still in progress? Convert to draft". In the future, when publishing a PR to start with you can use "create draft pull request" instead of "create pull request".

thelonestander · 2022-06-16T16:36:19Z

This looks a lot better than those horrible "fake" parts being visible. Thanks for this.

Coolthulhu · 2022-06-20T22:55:04Z

src/lightmap.cpp

+                break;
+            case quadrant::SW:
+                return ( p.x > 1 && p.y < MAPSIZE_Y - 1 &&
+                         ( *blocked_caches[p.z + OVERMAP_DEPTH] )[p.x - 1][p.y + 1][1] );


This could probably be optimized a bit by using a padded cache instead of map-sized one.

Will have to profile it either way, so maybe it will turn out that optimizing this bit is not needed.

src/map_field.cpp

Coolthulhu · 2022-06-20T22:58:00Z

src/pathfinding.cpp

@@ -293,6 +293,9 @@ std::vector<tripoint> map::route( const tripoint &f, const tripoint &t,
        const auto &pf_cache = get_pathfinding_cache_ref( cur.z );
        const auto cur_special = pf_cache.special[cur.x][cur.y];

+        int curPart;
+        const vehicle *curVeh = veh_at_internal( cur, curPart );


snake_case preferred to camelCase.

Coolthulhu · 2022-06-20T23:02:55Z

src/lightmap.cpp

@@ -811,6 +877,7 @@ void cast_zlight_segment(
    const array_of_grids_of<T> &output_caches,
    const array_of_grids_of<const T> &input_arrays,
    const array_of_grids_of<const bool> &floor_caches,
+    const array_of_grids_of<const bool[2]> &blocked_caches,


Would be better to use a named struct here instead of [2].
I assume the values describe whether NW-SE and NE-SW diagonals are blocked. With a named struct, it could be explicitly described.

Coolthulhu · 2022-06-21T19:44:59Z

I'm getting debugmsgs when exiting an unrotated 2 seater helicopter.

joveeater · 2022-06-28T00:35:38Z

I'm not sure this is the problem you had I found a problem with gasses that would cause unexpected movement in rotated vehicle messages when it tried to spread out of bounds. I think this might be a larger problem, maptile_at returns a null_submap tile and the gas code just goes with it. I've fixed the vehicle rotation side, but otherwise I've left that behaviour the same.

My very limited profiling suggests that scent mapping is the only thing noticeably affected by this but it's a huge cycle hog. Adding vehicle rotation without a cache tanked performance, with a cache it's still about 10-20% slower than upload. I've made a bunch of optimizations that pull this back to about 20% faster. Mostly it's been small things like rearranging coefficients for faster arithmetic or reducing the temporary structure size. The biggest change was to fold the block and reduce scent properties into a single value called scent_transfer. Rather than returning a map of booleans and using them to select between doing nothing, using a coefficient of 2 or using a coefficient of 10, it puts 0, 2 or 10 (actually 0, 1, 5 after rearrangement) into the map. This gets rid of all the branches other than the vehicle holes related ones that I've added. I figure mispredictions on them should be rare.

Obviously those numbers are very rough and I'm not including building the blockers map, just the actual scent update. Scent updating is still a big performance draw and so is building the map. I might come back and take a look at them but that's another PR.

Coolthulhu · 2022-07-08T14:38:32Z

src/action.cpp

+        return cata::nullopt;
+    }
+
+    if( g->m.obstructed_by_vehicle_rotation( g->u.pos(), *dir + g->u.pos() ) ) {


This isn't the perfect place for it. OK for now, but there are cases when this fails. The most obvious one is quantum tunneling bionic.

Coolthulhu · 2022-07-08T14:51:42Z

In my testing, there were two cases where it clearly failed:

Spawn a truck trailer rotated -45 degrees
Spawn a zombie dog on the other side and get close to it so that you are "diagonally adjacent"
You shouldn't be able to see the dog and dog shouldn't be able to see you (so vision bits work)
Try firing a gun at the dog
It fails to shoot for a mysterious reason (might not be caused here, but makes testing harder)
Try firing one tile behind the dog
Now it works and manages to kill the dog
Repeat with copper spear's reach attack
This time adjacent attacks work just fine

The reach attack case is easy: you set tripoint last_point = path.back();, but you should probably set it to pos().
It seems that the gun failure is for the same reason: first "previous position" is set to end point.
I'd expect both cases to start working if the enemy is no longer adjacent.

joveeater · 2022-07-10T21:02:25Z

Reach attacks was like you said, though we do need to handle that last tile as well. I think what you're seeing with the dog is intended. I don't know what gun you used but a truck trailer isn't enough to stop a glock 17. The dog dying is expected, so long as the vehicle wall gets bashed first. I can understand how the code looks a lot like I made the same mistake as I did with reach attacks but at that point we haven't traced the projectile yet so that "end" is still initialized to the source. It confused me as well when I took a closer look at it but it is sound and I couldn't find any problems in testing.

I'm not sure about the fail to shoot, I couldn't reproduce that but I've noticed another problem similar to one of the ramp ones, vehicles across 2 submaps could get missed on refresh, so I've moved the cache refresh to do_vehicle_caching which doesn't have that problem and should be getting called less anyway.

Coolthulhu · 2022-07-10T21:06:49Z

The dog dying is expected, so long as the vehicle wall gets bashed first.

It doesn't. It certainly doesn't get "bashed through". If the wall is positioned orthogonally, the bullet is stopped completely.

joveeater · 2022-07-10T21:23:46Z

With a glock 17 the bullet getting through is random in both cases. Most of the time it won't but being rotated doesn't seem to change that. If you're getting cases where it's not bashing either of the two walls, then you're seeing something very different to me but I can't recreate it.

Coolthulhu · 2022-07-11T16:53:08Z

Try a self-bow.
The most obvious way to reproduce:

Spawn -45 degree car
Spawn self-bow and some arrows
Spawn a debug monster diagonally adjacent, right out of the car, so that you can see it but not melee it
Set your skills and stats sky high so you don't miss
Aim at a tile behind the monster
Notice how arrows almost always hit the monster
Aim behind adjacent door or quarterpanel
Shoot a few times
Notice that arrows hit the obstacle and if they do drop, they drop onto the floor under you
Save+load and repeat (to test whether it might be affected by debug)

It very clearly phases through the diagonals if you're adjacent and aim behind.

I'm testing it with the ramps PR, so it may be interfering. I'll check the ramps one separately now, maybe it can be merged earlier.

joveeater · 2022-07-11T17:18:23Z

Yeah, I can't reproduce this. It might be ramps, but for me I get "you hear crash" and the arrows end up at my feet or destroyed.

joveeater · 2022-07-11T17:40:14Z

Right, I got it. It's the dog, I think it wandered away while I was loading the bow or something.

joveeater · 2022-07-28T01:35:09Z

Bug fixes for removed parts, it'll no longer try to add them to the cache. Also adds vehicle holes to npc aiming logic so they won't try to shoot through them.

Coolthulhu · 2022-07-30T11:35:21Z

Also adds vehicle holes to npc aiming logic so they won't try to shoot through them.

Nice
I'll need to check if the following behaviors are checked:

Player auto-aim through transparent diagonals
Spells through diagonals
Explosions
Melee range abilities (hulk smash, EMP bionic)

I won't require them now, but they're certainly desired.

Coolthulhu · 2022-07-30T21:56:37Z

Debugmsgs on "new explosions" inside a rotated vehicle. This should probably be fixed before merge.

Diagonals that aren't properly prevented yet, but aren't necessary in this PR:

Spells
Auto-aim

Explosions and EMP bionic are handled.

joveeater · 2022-08-01T00:49:20Z

Alright, debugmsg on explosions is fixed and I went a bit nuts and searched the source for "passable" and "transparent". I have added holes support to auto-aim (and targetable_creatures in general), spells, knockback, a whole bunch of monster attacks, flinging creatures, NPC and activities picking up items, vomiting, ranged aoes, electricity spread, fungus spores, peeking, pit traps and shell casings.

Coolthulhu · 2022-08-04T16:36:04Z

src/map.cpp

            // Exit before checking the last square, it's still visible even if opaque.
            if( new_point.x == T.x && new_point.y == T.y ) {
                return false;
            }
-            if( !this->is_transparent( tripoint( new_point, T.z ) ) ) {
+            if( !this->is_transparent( tripoint( new_point, T.z ) ) ||
+                obscured_by_vehicle_rotation( tripoint( last_point, T.z ), tripoint( new_point, T.z ) ) ) {


This bit may be mega expensive in the worst case scenario.
Every tile checks 2 vehicles. Vast majority of the time they'll be null, but the constant cost of the check is there.
Monsters use this function to find other monsters and this is already a huge performance hog whenever opposing factions are on the same map.

Did you test the performance of say, 100 zombies and 10 dogs standing in open field? I suspect that there would be a big drop in "fps" with the vehicle checks.
It looks like it is cacheable. Maybe even already cached in the form of diagonal blocks.

map::sees is one of those places where profiling is an unfortunate necessity.

Good point. It's fine normally but I hadn't tried any interfaction stuff. I'll just move both this and obstructed over to a cache.

Coolthulhu · 2022-08-04T16:37:22Z

src/map.cpp

            // Exit before checking the last square, it's still reachable even if it is an obstacle.
            if( new_point.x == t.x && new_point.y == t.y ) {
                return false;
            }

            const int cost = this->move_cost( new_point );
-            if( cost < cost_min || cost > cost_max ) {
+            if( cost < cost_min || cost > cost_max ||
+                get_map().obstructed_by_vehicle_rotation( tripoint( last_point, t.z ), tripoint( new_point,


This get_map() here is wrong, we're already in map class.

Coolthulhu · 2022-08-04T16:38:19Z

src/map.cpp

@@ -6437,6 +6461,51 @@ bool map::clear_path( const tripoint &f, const tripoint &t, const int range,
    return is_clear;
 }

+bool map::obstructed_by_vehicle_rotation( const tripoint &from, const tripoint &to ) const


Could this reuse a cache to avoid having to "grab" vehicles?

Coolthulhu · 2022-08-04T16:38:53Z

src/map.cpp

+}
+
+
+bool map::obscured_by_vehicle_rotation( const tripoint &from, const tripoint &to ) const


Same here: if it was possible for this to use a cache, it would be nice.

Coolthulhu · 2022-08-04T17:00:21Z

src/monattack.cpp

+                pnt.x = prev_point.x;
+            } else {
+                pnt.y = prev_point.y;
+            }


You repeated this block a number. A helper function would be good.
It could look like this:

cata::optional<tripoint> diagonal_blocker( const map &m, const tripoint &from, const tripoint &to ) { if( m.obstructed_by_vehicle_rotation( from, to ) ) { //50% chance of bouncing off each intervening tile if( one_in( 2 ) ) { return tripoint{ from.x, to.y, to.z }; } else { return tripoint{ to.x, from.y, to.z }; } } return cata::nullopt; }

Used like

cata::optional<tripoint> blocker = diagonal_blocker( get_map(), prev_point, pnt ); if( blocker ) { pnt = *blocker; // Do blocked stuff } else { // Do unblocked stuff }

This is a good idea. When I was going through I was thinking now there's a directional component to obstruction it would be nice at some point in the future to refactor out all of the line tracing stuff to a core similar to bresenham() with a set of common predicates and helper functions like get_first_impassable or whatever.

However, it's looking like the real problem with 3D fov is that a lot of this stuff should have floor checks as well. Hopefully I'll find some time for it fairly shortly, but I'll clean all of this up and get some single point of truth on what counts as impassable.

For now it's fine if you block all 3D diagonal movement that goes horizontally through vehicle holes if start or end are in the vehicle.
At the moment, shooting through vehicle roof/floor is mega rare. It doesn't need to be "good", just predictable, so that we can tell if something is a bug or just a limitation.

I should have been more clear sorry. I want to do that as a separate PR with the intention of making attacks not go through floors in general. I've not actually looked into any of it yet so I'm not 100% this is really the problem but apparently stuff going through floors is a thing.

Coolthulhu · 2022-08-14T11:48:55Z

I'm getting serious inconsistencies in vision processing with 3D vision enabled:

The test failures are not related to the PR, just a RNG-failing test. I restarted them.

joveeater · 2022-08-15T01:20:46Z

I've fixed the problem with the 3d vision and mostly it's the same solution as the 2d version with one big difference. If you stand orthogonal to a pillar in 3d it won't cast a shadow at all and then this affects car walls that you're orthogonal too as well. Their blocked property won't matter because one of the two casts hitting the tile will be the wrong direction. I've put in a bit of an ugly hack to handle these cases by checking both relevant blocked properties when looking at an orthogonal tiles and updating the starting minor appropriately.

Coolthulhu · 2022-08-15T09:37:29Z

The hack doesn't really work (screenshot below).
I guess if it's a natural consequence of the pillar bug, it's in the gray zone of "bug not introduced in the PR". But we should have a bug report for it after the merge.
For now, it would be better to remove the hack, unless you can somehow make it work in all cases.

I tested performance. Not through real profiling, just a manual test. I didn't see a noticeable slowdown, so it's OK.

Ideal option would be to have vehicle diagonal vision blocking be a debug option until it works 100%, but it looks good enough without it.

joveeater · 2022-08-16T14:04:57Z

Ah yeah, good point, I've just removed it for now.

Coolthulhu · 2022-08-18T21:32:01Z

I thought gas-creating monsters can produce gases through diagonals, but it looks like it's not specific to diagonals. It might be gases leaking upwards or something like that.

Coolthulhu · 2022-08-18T21:33:33Z

Let's risk it and give it a go.

thelonestander · 2022-08-19T14:28:30Z

WOW. This is a great time to be on BN. Thank you so much @joveeater!!

Firestorm01X2 · 2022-08-19T14:57:28Z

Link to DDA solution for reference:
CleverRaven/Cataclysm-DDA#56143

joveeater marked this pull request as draft June 9, 2022 21:42

joveeater force-pushed the vehicle-holes branch from de22376 to 28fcc7b Compare June 15, 2022 19:54

joveeater marked this pull request as ready for review June 15, 2022 20:02

joveeater changed the title ~~Vehicle holes POC~~ Vehicle holes Jun 16, 2022

Coolthulhu self-assigned this Jun 19, 2022

Coolthulhu reviewed Jun 20, 2022

View reviewed changes

src/map_field.cpp Show resolved Hide resolved

Coolthulhu reviewed Jun 20, 2022

View reviewed changes

Coolthulhu removed their assignment Jun 21, 2022

joveeater force-pushed the vehicle-holes branch from d6b5230 to 880fcd9 Compare June 28, 2022 00:32

Coolthulhu self-assigned this Jul 6, 2022

Coolthulhu reviewed Jul 8, 2022

View reviewed changes

joveeater force-pushed the vehicle-holes branch 2 times, most recently from f3aa067 to d7113d3 Compare July 10, 2022 21:00

Coolthulhu removed their assignment Jul 11, 2022

joveeater force-pushed the vehicle-holes branch from d7113d3 to 32791e6 Compare July 11, 2022 21:43

joveeater force-pushed the vehicle-holes branch 2 times, most recently from 9fce88d to 217756b Compare July 28, 2022 01:31

joveeater force-pushed the vehicle-holes branch from 217756b to 9773f38 Compare August 1, 2022 00:49

Coolthulhu reviewed Aug 4, 2022

View reviewed changes

joveeater added 5 commits August 5, 2022 14:00

Prevents moving through vehicle holes

80793fd

Bug fix and npc los

b2878d0

Added vehicle holes to a wide range of areas

5fcff77

tidy

7075f37

Caching

b680fa5

joveeater force-pushed the vehicle-holes branch from c70ebf1 to b680fa5 Compare August 6, 2022 01:29

fix 3d vision for vehicle holes

73718eb

Remove awful 3d vision hack

35cd3d6

Coolthulhu merged commit 61a1645 into cataclysmbnteam:upload Aug 18, 2022

sonphantrung mentioned this pull request Aug 23, 2022

Port changes from https://github.com/cataclysmbnteam/Cataclysm-BN/pull/1610 CleverRaven/Cataclysm-DDA#60399

Closed

		}


		bool map::obscured_by_vehicle_rotation( const tripoint &from, const tripoint &to ) const

Vehicle holes #1610

Vehicle holes #1610

Conversation

joveeater commented Jun 9, 2022 • edited Loading

Summary

Purpose of change

Describe the solution

Describe alternatives you've considered

Testing

Additional context

joveeater commented Jun 9, 2022

Zireael07 commented Jun 9, 2022 • edited Loading

KheirFerrum commented Jun 9, 2022

thelonestander commented Jun 16, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Coolthulhu commented Jun 21, 2022

joveeater commented Jun 28, 2022

Choose a reason for hiding this comment

Coolthulhu commented Jul 8, 2022

joveeater commented Jul 10, 2022

Coolthulhu commented Jul 10, 2022

joveeater commented Jul 10, 2022

Coolthulhu commented Jul 11, 2022

joveeater commented Jul 11, 2022

joveeater commented Jul 11, 2022

joveeater commented Jul 28, 2022

Coolthulhu commented Jul 30, 2022

Coolthulhu commented Jul 30, 2022

joveeater commented Aug 1, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Coolthulhu commented Aug 14, 2022

joveeater commented Aug 15, 2022

Coolthulhu commented Aug 15, 2022

joveeater commented Aug 16, 2022

Coolthulhu commented Aug 18, 2022

Coolthulhu commented Aug 18, 2022

thelonestander commented Aug 19, 2022

Firestorm01X2 commented Aug 19, 2022

joveeater commented Jun 9, 2022 •

edited

Loading

Zireael07 commented Jun 9, 2022 •

edited

Loading