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

Fix impassable broken vehicle parts #70560

Merged
merged 15 commits into from
Jan 2, 2024
Merged

Conversation

worm-girl
Copy link
Contributor

@worm-girl worm-girl commented Dec 31, 2023

Summary

Bugfixes "Fix impassable broken vehicle parts"

Purpose of change

fixes #70555
fixes #70564

Vehicle part passability wasn't properly checking for whether certain parts were or were not broken when deciding whether to see if you could move through them. This was my fault!

Describe the solution

The check for free cargo space is now bypassed if the part is broken (it can't hold anything if it's broken and shouldn't obstruct anyone based on its contents). The check for cargo_passable also still runs if the part is broken.

Also adjusts some vehicle part sizes that I missed.

Describe alternatives you've considered

We could do more in-depth checks on broken parts. Right now they just get a pass as the game sets their capacity to 0 ml which skips the passability check. This allows IE a hulk to smash its way into a car without having to completely delete the tile, though said hulk will still get the cramped space debuff since it's huge. The bucket seat is no longer obstructing the space, but you're still cramming yourself in a too-small vehicle. IMO that makes enough sense that we shouldn't worry about it, but it could be made more granular in the future if desired.

Testing

  • Stepped on an empty cargo tile.
  • Stepped on a broken cargo tile.
  • Put a body in a cargo tile, and became unable to step there.
  • Stepped on a broken door.
  • Stepped on a working door.
  • Spawned a zombie and had it do the same.
  • Checked that the special case penalties for huge creatures being in vehicles still applied properly in broken parts.
  • Checked that inflatable boats are usable again.

Additional context

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` <Bugfix> This is a fix for a bug (or closes open issue) json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions labels Dec 31, 2023
@prharvey
Copy link
Contributor

Slightly unrelated to this change, but the cramped_space effect is applied incorrectly for monsters. You're doing it in monster::will_move_to which is only a check if a monster should move there, but doesn't actually move the monster. It should be done in monster::move_to.

Also note that it's probably not worth spending a lot of time on this, since I'm rewriting the pathfinder anyways: #70274 - I still need to integrate your changes into it.

@worm-girl
Copy link
Contributor Author

Slightly unrelated to this change, but the cramped_space effect is applied incorrectly for monsters. You're doing it in monster::will_move_to which is only a check if a monster should move there, but doesn't actually move the monster. It should be done in monster::move_to.

Also note that it's probably not worth spending a lot of time on this, since I'm rewriting the pathfinder anyways: #70274 - I still need to integrate your changes into it.

Thanks! I moved it to move_to and it seems like it's working properly.

@github-actions github-actions bot added the BasicBuildPassed This PR builds correctly, label assigned by github actions label Dec 31, 2023
@github-actions github-actions bot added the Monsters Monsters both friendly and unfriendly. label Jan 1, 2024
@github-actions github-actions bot added [JSON] Changes (can be) made in JSON Vehicles Vehicles, parts, mechanics & interactions labels Jan 1, 2024
@worm-girl worm-girl marked this pull request as ready for review January 1, 2024 02:09
@github-actions github-actions bot removed the astyled astyled PR, label is assigned by github actions label Jan 1, 2024
src/character.cpp Outdated Show resolved Hide resolved
src/character.cpp Outdated Show resolved Hide resolved
src/character.cpp Outdated Show resolved Hide resolved
src/character.cpp Outdated Show resolved Hide resolved
src/character.cpp Outdated Show resolved Hide resolved
worm-girl and others added 5 commits December 31, 2023 18:11
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
src/character.cpp Outdated Show resolved Hide resolved
@github-actions github-actions bot added the astyled astyled PR, label is assigned by github actions label Jan 1, 2024
prharvey added a commit to prharvey/Cataclysm-DDA that referenced this pull request Jan 1, 2024
@Maleclypse Maleclypse merged commit bef30a5 into CleverRaven:master Jan 2, 2024
24 of 26 checks passed
}
// Check all of this here to ensure the player can't sit in a comfortable seat and then drop 50 liters of junk in their own lap.
if( in_vehicle ) {
if( has_effect( effect_cramped_space ) ) {
Copy link
Member

@BrettDong BrettDong Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The condition of the if block at line 10943 becomes always false after your change at this line, right?

Line 10943:

if( !has_effect( effect_cramped_space ) ) {
    add_effect( effect_cramped_space, 2_turns, true );
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it looks like you're right. That shouldn't cause any problems but it should be removed.

@@ -10948,15 +10944,16 @@ void Character::process_effects()
add_effect( effect_cramped_space, 2_turns, true );
}
is_cramped_space = true;
return;
Copy link
Contributor

@inogenous inogenous Jan 2, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These early returns were the cause of #70389 before, which was fixed in #70409 . In this change, the return-statements here and on line 10956 are added back. Do you know if this change accidentally re-introduces #70389 again now if you sleep in a cramped space in a vehicle?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't, and they probably do. I didn't realize I was re-introducing the bug.

kevingranade added a commit that referenced this pull request Jun 12, 2024
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 <Bugfix> This is a fix for a bug (or closes open issue) [C++] Changes (can be) made in C++. Previously named `Code` [JSON] Changes (can be) made in JSON json-styled JSON lint passed, label assigned by github actions Monsters Monsters both friendly and unfriendly. Vehicles Vehicles, parts, mechanics & interactions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

player is too big to fit into the inflatable boat. Cant pass through broken car seats and broken trunks
5 participants