Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fake parts for filling vehicle gaps #56143

Merged
merged 40 commits into from
Jun 8, 2022

Conversation

kevingranade
Copy link
Member

@kevingranade kevingranade commented Mar 16, 2022

Summary

Features "Fill gaps in vehicle walls when necessary to maintain vehicle integrity."

Purpose of change

Completes #53722, which is a continuation of #42462, which is an extension of #41277
Fixes #5684

As vehicles turn, they skew and open gaps in previously aligned lines of obstacles. This creates a weird situation where a vehicle that is driving in a cardinal direction fully protects its occupants, but a vehicle driving at any other angle can get hitchhikers.

Describe the solution

On vehicle refresh, the outline of the vehicle is scanned for eligible obstacles and protuberances that have skewed away from their neighbors, when found these obstacles have fake parts added to the vehicle to fill the gaps.
Vehicle damage code forwards all damage instances to the real part instead of the fake part until the real part is destroyed, at which point the fake part is marked for removal as well. Updates to the state of the real part are echoed to the fake parts on vehicle refresh as the fake parts are stripped off of the vehicle and recreated.

Describe alternatives you've considered

Too many to count, but this approach was selected because it mostly contains the change to the vehicle code instead of sprawling across all of the codebase that has to interact with the map.

Testing

Multiple layers of tests were added to exercise this feature, and the propensity for the overmap_terrain_coverage test to trigger several problematic use cases in mapgen has also been extremely valuable.

  • have one lingering issue, which is that ramming other vehicles generates src/vehicle.cpp:7390 [int vehicle::get_non_fake_part(int)] Returning -1 for get_non_fake_part on part_num -1. errors.
    Turns out there was just a typo in the vehicle->vehicle collision code.

Additional context

When merging definitely do a merge even with the relatively messy history to preserve authorship.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON Code: Tests Measurement, self-control, statistics, balancing. Mods: Dark Skies Anything to do with Dark Skies Above, alien invasion mod with no zombies json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Mar 16, 2022
@kevingranade kevingranade marked this pull request as ready for review March 17, 2022 01:30
@Inglonias
Copy link
Contributor

That's a lot of changes. Best of luck

@github-actions github-actions bot added the <Enhancement / Feature> New features, or enhancements on existing label Apr 4, 2022
@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Apr 4, 2022
@kevingranade
Copy link
Member Author

Ok two major changes:

  1. There was a pre-existing issue that came up in testing because I was hammering on it, which is that when a vehicle is placed and then split in mapgen, the split code would call back into the global map instance to place the pieces instead of placing them on the tinymap being generated, the main things causing this were the wreckage generation code, which can cause vehicle splits now, and some map extras which would call map::destroy() and end up splitting vehicles that way. I fixed this by passing references to the current map being acted on into the vehicle damage functions so that they can target the right map.
  2. There was a major problem with the fake parts code in that it would frequently strip the fake parts back out and then replace them, but this really can not happen because doing so invalidates part indexes and causes the state of the vehicle to go out of sync with the map cache. I addressed this by removing calls that would strip fake parts out with the exception of part cleanup code and unracking code, both of which are careful to invalidate and regenerate the vehicle cache afterwards.

I've hammered on the automated tests a ton (50 odd test suite invocations), and I'm pretty confident the errors I was seeing cropping up there periodically are resolved, but I haven't had a chance to do more than some light driving around and ramming stuff testing in-game for some more random testing.

I'll be pushing fixes tot he things clang-tidy is complaining about as soon as I can, but this PR really does need some fairly heavy testing before it goes in.

@esotericist
Copy link
Contributor

did some quick in-game testing. made a cube van with a partly opaque cabin.
image
image

started running it at angles and immediately got some weirdness.
image
image

removed the mirrors, and i'm pretty sure proxy tiles are not correctly inheriting visibility:
image

i was able to determine they correctly function for passability and rigidity, and that damage correctly propagates to the right part when a proxy is hit.

the rendering needs a band-aid, and x look around shows that there's a vehicle there but not what the tile is supposed to be, which is awkward.

but right now the primary issue i can observe is the the proxies seem to be transparent, and it is in fact bidirectional:
image

@esotericist
Copy link
Contributor

esotericist commented Apr 6, 2022

loaded up my actual save to do for-reals playtesting.

first thing i noticed:
image

my first surprise was that the fake parts are rendering for this flatbed truck when they didn't for the cube van above, but something more pressingly weird is happening with the door fake part:
image

i can only open/close the real door, not the fake one, which... could actually be problematic in some configurations. (note that opening the real door does correctly change the state of the fake door.)

back to rendering:
the first time i started up my rv this run, i got correctly rendering proxy parts when i started to turn:
image

then as soon as i turn off the engine:
image

whatever's happening here, it seems to work correctly the first time a vehicle is loaded in a game session, or the first time a vehicle rotates after a game session starts, then breaks once the player is no longer driving the vehicle until a new session. (turning off the engine is not necessary; i was able to reproduce the 'no tile' result above by simply letting go of the controls while the vehicle was otherwise stopped).

so far i have yet to discern any way of getting it back to the valid state other than save and quit, then restart.

bonus fun: a recurrence of an old map memory bug:
image

in some circumstances (i had trouble reproducing this on demand, but it happened a couple times when i wasn't trying to do it) the fake part can get remembered as it passes out of view, similar to some of the old problems we have historically had e.g. dragging a cart in darkness.

@github-actions github-actions bot added Map / Mapgen Overmap, Mapgen, Map extras, Map display Mods Issues related to mods or modding labels Apr 20, 2022
@kevingranade
Copy link
Member Author

I need to flip to a tiles build to double check for the specific issues you're reporting, but I did find a critical error that is almost certainly related.
There's a cache of vehicle-relative coordinates to item index called relative_parts that is updated in vehicle::refresh(), when I overhauled fake part maintenance to avoid removing and re-adding fake parts on refresh (to prevent map cache desync), I failed to have refresh() rebuild relative_parts, so it would lose track of the fake parts on some kinds of refresh operation and then re-add them on other flavors of refresh.
I also need to clean up the code, what I have is working, but it's very messy.

@github-actions github-actions bot added the Vehicles Vehicles, parts, mechanics & interactions label Apr 24, 2022
@kevingranade
Copy link
Member Author

kevingranade commented Apr 26, 2022

I've finally had a chance to do a tiles build and test, and I'm working my way through the issues you reported.

  • A diagonally oriented cube van's mirrors are rendered properly, This seems to be related to the relative_parts cache and is fixed.
  • "the rendering needs a band-aid, and x look around shows that there's a vehicle there but not what the tile is supposed to be, which is awkward." This issue is still present, I'm pretty sure it needs a separate override in look_around.
  • Fake parts are transparent, or rather are not participating in transparency.
  • Inability to access doors via their fake parts is still occurring.

@kevingranade kevingranade force-pushed the fake-parts branch 2 times, most recently from 2324bc7 to 8cdfd9c Compare May 3, 2022 18:26
@github-actions github-actions bot added NPC / Factions NPCs, AI, Speech, Factions, Ownership and removed BasicBuildPassed This PR builds correctly, label assigned by github actions labels May 3, 2022
@andrei8l
Copy link
Contributor

andrei8l commented May 3, 2022

Collisions sometimes (temporarily) remove real parts

Screenshot from 2022-05-03 22-03-26

Another try from same angle

Screenshot from 2022-05-03 22-05-27

And it's not just a visual bug

Screenshot from 2022-05-03 22-04-48

But the missing parts do come back after moving a bit

Screenshot from 2022-05-03 22-05-36

Forgan.tar.gz

@kevingranade kevingranade merged commit befb7ab into CleverRaven:master Jun 8, 2022
@kevingranade kevingranade deleted the fake-parts branch June 10, 2022 15:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Code: Tests Measurement, self-control, statistics, balancing. <Enhancement / Feature> New features, or enhancements on existing [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Map / Mapgen Overmap, Mapgen, Map extras, Map display Mods: Dark Skies Anything to do with Dark Skies Above, alien invasion mod with no zombies Mods Issues related to mods or modding NPC / Factions NPCs, AI, Speech, Factions, Ownership Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Angled vehicles develop holes [$15]
8 participants