generalize node damage-per-second function

[FaceDeer]

I got an interesting bug report on my magma_conduits mod that said that the moment my mod was enabled all wildlife instantly fell over and died.

Turns out it's because my mod uses an alias to eliminate mapgen lava by turning it into air, causing this mod to treat air as lava. Which is understandably painful for most animals. :)

This pull request modifies wildlife to overcome this problem and in the process generalizes the detect-node-dps mechanism to cover all nodes that might cause harm (or, if someone adds a mod with a node that magically heals via negative dps, this should handle that too). Animals are no longer fireproof, for example.

I feel like a bit of a monster given the testing I needed to do to confirm my fix was working. Chasing down deer and setting the forest on fire around them, dumping lava on them to watch them die. The sacrifices I make for Minetest. :)

[TheTermos]

Thanks, my bad, I must have missed that node param.
Will merge, but first I'd like to shorten it a bit.

[FaceDeer]

Okay. Make sure to keep algorithmic complexity in mind when shortening, though - this is code that's called every tick so I advise doing everything you can to reduce the number of things the CPU needs to do in it.

[TheTermos]

Don't worry about that, also it is called inside a 1 second timer block, which is probably once every 30 steps on average.

Actually I'll just go ahead and ask why you used find_nodes_in_area instead of get_nodes_in_area.
The latter gets you node references as keys, so you can get node property directly at runtime and save yourself the hassle of translating strings to nodes back and forth and saving node name list after load.

I also don't like the idea of adding up damage based on the number of adjacent nodes of same kind.

[FaceDeer]

find_nodes_in_area is part of the engine API, so it's inherently faster than anything implemented in Lua. It gave me exactly the data I wanted - what nodes this animal is touching and how many of each - and it does all its work engine-side, in compiled C++ code with direct access to the game's data.

Saving the nodename->dps mapping is a one-off operation done when the server starts up, it's not a significant hassle. Using that cache means I can skip the step of looking up the node definition each time I want to know how much the node hurts.

Basically, my coding philosophy is "don't repeat yourself". Don't do the same thing over and over again when you can do it once and save the result. There are very few node types in most Minetest games that do damage per second, so the table takes up a tiny amount of memory. The DPS values never change once the game is loaded so the table never needs to be regenerated.

Of course I'm not a fundamentalist about that philosophy. There are always going to be exceptions, coding standards shouldn't be a straightjacket. But when dealing with code that's called very frequently it's good to squeeze as much performance out of it as you can without making the code hard to follow so this seemed like a good opportunity.

If you don't like multiplying the damage by the number of nodes a creature is touching I've got no issue there - hit points are a very abstract thing and interpretation of what they mean will differ. I figured if you've got twice as much lava pouring over you it's going to hurt twice as badly. Would you rather it be just "take the damage that the most painful node touching me does"? That way if an animal is touching both lava and fire it'd just take lava damage, on the assumption that the fire is not the poor creature's major concern right then. That'd be pretty straightforward to code.

Want me to have another go at rewriting this?

[TheTermos]

Yes, please.

The primary function of this mod is being a mobkit usage example, so it only makes sense that a mobkit function is used.

Yes, chances are find_nodes_in_area is faster itself, but I'm not that sure about the whole thing.
However I'm pretty sure any differences will be below the measurement error margin.

I figured if you've got twice as much lava pouring over you it's going to hurt twice as badly.

Consider a cube of size less than 1.0 fully submerged in lava. Now depending on where it is relative to the grid it intersects with 1 to 8 nodes which would be from 1x to 8x the damage at random. That's inconsistent and unnecessary.

Would you rather it be just "take the damage that the most painful node touching me does"? That way if an animal is touching both lava and fire it'd just take lava damage, on the assumption that the fire is not the poor creature's major concern right then. That'd be pretty straightforward to code.

I was thinking if it touches any node A it takes one node A damage, if it touches any node B it additionally takes one B damage, and so on.
That'd be the most straightforward to code, and probably made the most sense.

[FaceDeer]

Anything that gets called lots of times will eventually add up. But I agree that it's probably pretty minor in this case, so if this is meant to showcase the mobkit API it's probably fine.

A cube smaller than 1.0 would only be able to overlap at most 4 unit cubes, and a cube slightly larger than 1.0 would overlap between 4 and 8 cubes. (Sorry, suffering from Khan's Disease there and not thinking three-dimensionally. The next line still applies though:) I figure it averages out as they move around, and there'd be less variance for much larger animals, but I'm fine with whatever since hitpoints are abstract anyway.

One problem I see with the one of node A and one of node B approach is that default:lava_source and default:lava_flowing are different node types, so an animal wading around in the fringes of a pool of lava is likely to be overlapping both. If you're okay with that level of variance I can implement that, for the time being I just uploaded a change that adds the "max damage" approach I mentioned above (and uses the mobkit API).

[TheTermos]

One problem I see with the one of node A and one of node B approach is that default:lava_source and default:lava_flowing are different node types

Right, good catch. Yes, max damage approach will be fine.
Had a look at the PR, just lose the unused minetest.after block and the global tables, and we should be good to go.

[FaceDeer]

Whoops. :)

[TheTermos]

Tested, works, mergin.
Thanks!