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 eternal drowning when NPC is thrown into deep water. #3269

Merged
merged 2 commits into from
Sep 27, 2023

Conversation

cbnbugfix2
Copy link
Contributor

Summary

SUMMARY: Bugfixes "Fix NPCs eternally drowning after being thrown into deep water despite afterwards getting out of it"

Purpose of change

Prevent NPCs from drowning eternally if they are thrown into deep water, even after walking onto land again.

Describe the solution

The first commit fixes a bug where an NPC, after being thrown into water (for instance by a kevlar hulk), would eternally drown, even after getting onto land (unless debugging was used to constantly give them health, they would constantly take torso damage and quickly die). This fix also has the second effect that NPCs now will be treated as being submerged when walking into deep water, causing drowning after enough time. This second effect has drawbacks due to the behaviour of NPCs not currently having drowning avoidance programmed into it. The fix is inspired by similar code in monmove.cpp, at

Cataclysm-BN/src/monmove.cpp

Lines 1638 to 1664 in dc43e23

//Check for moving into/out of water
bool was_water = g->m.is_divable( pos() );
bool will_be_water = on_ground && can_submerge() && g->m.is_divable( destination );
// Attitude check is kinda slow, better gate it
if( was_water != will_be_water && !flies() ) {
if( attitude( &g->u ) != MATT_ATTACK ) {
// Nothing, no need to spam
} else if( was_water && !will_be_water && g->u.sees( p ) ) {
// Use more dramatic messages for swimming monsters
//~ Message when a monster emerges from water
//~ %1$s: monster name, %2$s: leaps/emerges, %3$s: terrain name
add_msg( m_warning, pgettext( "monster movement", "A %1$s %2$s from the %3$s!" ), name(),
swims() || has_flag( MF_AQUATIC ) ? _( "leaps" ) : _( "emerges" ),
g->m.tername( pos() ) );
} else if( !was_water && will_be_water && g->u.sees( destination ) ) {
//~ Message when a monster enters water
//~ %1$s: monster name, %2$s: dives/sinks, %3$s: terrain name
add_msg( m_warning, pgettext( "monster movement", "A %1$s %2$s into the %3$s!" ), name(),
swims() || has_flag( MF_AQUATIC ) ? _( "dives" ) : _( "sinks" ),
g->m.tername( destination ) );
}
}
setpos( destination );
footsteps( destination );
set_underwater( will_be_water );
which has a method named the same as the method that the first commit changes.

The second commit disables NPCs taking damage from drowning, which fixes the possible issue above where NPCs might unintentionally die from standing around in deep water without trying to get out of it.

The two commits together should have mostly the same effects as before, except:

  1. NPCs will no longer have the internal state of not being submerged in deep water if they walked into it (they will now be treated as being submerged in water), even though they will not take damage from drowning (this includes the Respirator CBM draining bionic energy to delay the onset of drowning, even though drowning damage will not happen to NPCs either way now). Before, an NPC walking into deep water would be treated as if it was not submerged in water.
  2. NPCs being thrown into deep water will now not drown eternally, specifically, they will not be treated as if they are submerged in deep water once they are on land again.

Testing

I tested it using debug mode, with an NPC walking into deep water and observing that it did not drown after 30 minutes of waiting in deep water using the quick-wait function, and also tested that an NPC no longer drowns eternally by positioning deep water and a kevlar hulk such that the kevlar hulk would throw the NPC into deep water, and then observing that the NPC is still alive after 30 minutes of waiting once it got onto land using the quick-wait function.

Additional context

I talked about this bug and how to fix it best with some of the developers on Discord, and from what I understood, what I have implemented here might fit decently what we discussed. This way of fixing it should also mean that implementing NPCs drowning in the future should be easier, since only a few lines should be removed, and those lines have a Github issue attached to it in the code. The issue #3266 has been created as a suggestion on implementing drowning and to help explain in the code why drowning is currently disabled for NPCs .

An NPC that was thrown into water would eternally drown, even after
getting back on land.

This also fixes that NPCs take damage from drowning normally while
being submerged in deep water after walking into it instead of being
thrown into it, which may not be a desirable change, since NPC
behaviour does not currently include a lot of deep water avoidance.
@github-actions github-actions bot added the src changes related to source code. label Sep 26, 2023
@scarf005 scarf005 self-requested a review September 26, 2023 09:25
@scarf005 scarf005 self-assigned this Sep 26, 2023
Copy link
Member

@scarf005 scarf005 left a comment

Choose a reason for hiding this comment

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

image

LGTM

@scarf005 scarf005 added this pull request to the merge queue Sep 27, 2023
Merged via the queue into cataclysmbnteam:upload with commit 0bbf82d Sep 27, 2023
@chaosvolt
Copy link
Member

chaosvolt commented Sep 27, 2023

So, debug killing NPCs does...this now.
image

EDIT: And trying to kill them the normal way does this:
image

@scarf005
Copy link
Member

scarf005 commented Sep 27, 2023

wha- but how

i think it's related to #3282

@cbnbugfix2
Copy link
Contributor Author

I just tried killing an NPC using "Kill NPCs" in the build I made with my fork, and that works fine. I also killed an NPC manually, and that also worked fine.

@chaosvolt
Copy link
Member

Yep it's fine, looks like it was the cache dead NPCs thing instead.

scarf005 pushed a commit to scarf005/Cataclysm-BN that referenced this pull request Sep 28, 2023
…team#3269)

* fix: NPCs no longer drown eternally after being thrown.

An NPC that was thrown into water would eternally drown, even after
getting back on land.

This also fixes that NPCs take damage from drowning normally while
being submerged in deep water after walking into it instead of being
thrown into it, which may not be a desirable change, since NPC
behaviour does not currently include a lot of deep water avoidance.

* fix: Disable NPCs taking damage from drowning.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
src changes related to source code.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants