Rework gibbing, refactor damage.h

[Coolthulhu]

"NOGIB" flag was passed around in a weird way. Unlike all other effects, it was a damage_instance member. It was also the only usage of said member. Made it use ammo effects instead, since it's much cleaner and more standard.

Critter gibbing was happening in 2 places: monster::explode and mondeath::normal. This allowed weirdness like monster gibbing and yet leaving a corpse. Now monster can only gib if it leaves no corpse.

Finally, refactored damage.h. It is included all over the place and in headers and will be included even more, so it should be as tiny as possible. Moved projectile-related stuff into projectile.h/.cpp instead.
This was actually the primary reason for this PR, with gib fixes only being necessary because of the NOGIB weirdness.

[illi-kun]

Please check Jently bot error:

src/player.cpp:6432:20: error: unused variable 'dealt' [-Werror,-Wunused-variable]
        const auto dealt = deal_damage( nullptr, bp, damage_instance( DT_HEAT, rng( intense, intense * 2 ) ) );

[Coolthulhu]

May possibly fix #17188 since it deals with one possible source of the bug.

[mugling]

why?

[Coolthulhu]

It requires an extra header. It was cleaner to break it up than to include a header just for 3 way max.

[mugling]

which one out of curiosity?

[Coolthulhu]

I think it was <algorithm>. I assume it uses accumulate internally.
http://www.cplusplus.com/reference/algorithm/max/ says max requires algorithm, but they only use the 2 value one. We may have a forward declaration of the 2 argument max somewhere, possibly in the compiler itself.

[mugling]

I'm torn on that. It's not a lightweight header but I suspect we have a build error waiting to happen there given the right combination of platform and compiler.

[mugling]

Why this as opposed to the default destructor?

[BevapDin]

This could indeed be defaulted, as in projectile::~projectile() = default;. But it has to stay in the cpp file, not the header. If it was in the header, the header would have to include "item.h" as well to allow destroying the item inside of the unique_ptr. The current code allows using this class without having to include "item.h".

[mugling]

Should be specified in the header as default values

[BevapDin]

Why? It's just as good here and will result in the same code anyway. And putting it in the header exposes an implementation detail - it's nobodies business what the initial value of private members is.

[Coolthulhu]

Pretty sure pushing it to header would make it necessary to expose the item part of the unique_ptr<item>.
Since damage.h is all over the place, it shouldn't require item.h.

[mugling]

Why?

Reduction in complexity

That said it's impossible to do so as set out in @Coolthulhu's argument

[mugling]

could (and probably will) be inlined if moved to the header

[Coolthulhu]

The operator= operates on an item&, meaning it couldn't be pulled up to the header. Inlining a constructor that will then still expand into a function won't help with performance, especially if the compiler realizes the operator= is in the same compilation unit.

[mugling]

src/iuse_actor.cpp:1944:25: error: no member named 'accumulate' in namespace 'std'

[mugling]

#include <numeric> fixes that

[mugling]

Same in vehicle.cpp

[mugling]

Also turret.cpp

[mugling]

and damage.cpp

[mugling]

Compiles correctly after those inclusions

[mugling]

Fixes #17188

[Coolthulhu]

Didn't complain here (clang 3.6.0) or in Jenkins' version of GCC.

Which compiler doesn't like the lack of <numeric>?

[mugling]

FreeBSD clang version 3.4.1 (tags/RELEASE_34/dot1-final 208032) 20140512
Target: x86_64-unknown-freebsd10.2