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

Fragmentation armor fix #24836

Merged
merged 8 commits into from Aug 24, 2018

Conversation

Projects
None yet
5 participants
@kevingranade
Copy link
Member

commented Aug 14, 2018

Fixes #24796
Fixes #24863
Drops fragment damage a bit overall, and also decreases fragment size.
The result is there are about 10x as many fragments flying around, but they deal about 1/20th the damage they did previously, so they dont defeat armor.

  • cant damage power armor. (Even light)
  • minimal damage to vehicles.
  • minimal damage to buildings.
  • avoid log spam from huge numbers of fragments.

This is built on top of the perf fix.

@kevingranade kevingranade force-pushed the kevingranade:fragmentation-armor-fix branch Aug 15, 2018

@kevingranade

This comment has been minimized.

Copy link
Member Author

commented Aug 15, 2018

Well that was super not fun, it was quite a pain to generate these messages.
Even better, I'm not 100% that they're at the right level of detail, let me know what you think.

@kevingranade kevingranade force-pushed the kevingranade:fragmentation-armor-fix branch Aug 16, 2018

src/explosion.cpp Outdated
// Target, Number of impacts, total amount of damage, proportion of deflected fragments.
std::map<int, std::string> impact_count_descriptions = {
{ 1, _( "a" ) }, { 2, _( "several" ) }, { 5, _( "many" ) },
{ 20, _( "a large number" ) }, { 100, _( "a huge number" ) }

This comment has been minimized.

Copy link
@narc0tiq

narc0tiq Aug 16, 2018

Contributor

As this will be used in, e.g., "[You are/NPC is/The Monster is] hit by %s bomb fragments", I think you want "a [large/huge] number of".

@kevingranade kevingranade force-pushed the kevingranade:fragmentation-armor-fix branch 2 times, most recently Aug 17, 2018

@kevingranade

This comment has been minimized.

Copy link
Member Author

commented Aug 17, 2018

Next up is building some scenarios and testing them.
Hand grenade near car, hand grenade near shopping cart.
Near is 5 squares.
Expected result is nothing more than cosmetic damage to shopping cart.
Ok for car to lose tires and windows, nothing else should break.

Similarly, I don't expect a hand grenade to take down a wall, even adjacent.

Let me know if you have any other tests that need to happen.

@kevingranade kevingranade changed the title [WIP] Fragmentation armor fix Fragmentation armor fix Aug 17, 2018

@ZhilkinSerg ZhilkinSerg self-assigned this Aug 23, 2018

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Aug 23, 2018

Game hangs when you explode grenade in your inventory while wearing decent armor (full winter survivor suit with hood, gloves, boots, and mask).

@kevingranade

This comment has been minimized.

Copy link
Member Author

commented Aug 23, 2018

hrm, that is a rather large number of projectiles to handle, I wonder if I can shortcut that by comparing the potential damage vs the armor and skipping all the special effects if it has no possibility of dealing damage.

Also I'll just profile it, should be pretty straightforward to reproduce, there might be something dumb happening in there... ooh, like an animation delay.

@kevingranade kevingranade force-pushed the kevingranade:fragmentation-armor-fix branch to f34f2b2 Aug 24, 2018

@kevingranade

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2018

Final commit should adress the hang, player::deal_damage was drawing thousands of hitboxes for all the non-damaging fragment hits, and waiting for thousands of animation delay-length ticks.

@ZhilkinSerg

This comment has been minimized.

Copy link
Contributor

commented Aug 24, 2018

Performance is much better now - still noticeable "Hang on a bit" message sometimes, but game doesn't hang.

Grenade explosions seems to be a little underpowered as I couldn't shatter car windshields from a single explosion, but it looks the same in master branch.

I guess it is good to be merged now.

@ZhilkinSerg ZhilkinSerg merged commit bea5321 into CleverRaven:master Aug 24, 2018

4 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.1%) to 24.197%
Details
gorgon-ghprb Build finished.
Details

@ZhilkinSerg ZhilkinSerg removed their assignment Aug 24, 2018

@kevingranade

This comment has been minimized.

Copy link
Member Author

commented Aug 24, 2018

m.bash( target, damage / 10, true );
add_msg( ngettext( "The %s is hit by %s bomb fragment, %s.",
"The %s is hit by %s bomb fragments, %s.", total_hits ),
critter->disp_name().c_str(), impact_count, damage_description );

This comment has been minimized.

Copy link
@narc0tiq

narc0tiq Aug 24, 2018

Contributor

I just noticed this creates lines like "The the zombie is hit..." and "The the crawling zombie....". I think you wanted critter->get_name() instead?

@KurzedMetal

This comment has been minimized.

Copy link
Contributor

commented Aug 25, 2018

Shouldn't this merge close #24748 and probably #24861 too?

@CoroNaut

This comment has been minimized.

Copy link

commented Aug 27, 2018

I ran over a landmine by accident going 60kph in my bike and all it did was damage my stethoscope i was wearing by 1 point, didn't even damage the vehicle any. Is this intentional or did I get extremely lucky to not have 2 bikes with 1 wheel each?

@kevingranade kevingranade deleted the kevingranade:fragmentation-armor-fix branch Aug 28, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.