Stop absorbing scent from closed doors. This creates a loop that will

[BrianLefler]

increase smell infinitely on the opposite side of a door. Ironically, a
player in the room resets their square to their own smell and prevents the
infinite loop. But anyway. This is bad. We will now correctly ignore
closed doors and therefore calculate squares_used accurately. Note: Smell
still lingers forever, it just doesn't increase.

Fixes one of the issues being discussed in pull/4704

[swwu]

We talked about this in IRC and this is almost certainly the main cause for the "wacky scent" divergence bugs. So @phaethonfire I think this is probably the isolated fix you wanted, but it still might be good to add other things like damping later on? Assuming they conform to performance standards etc.

[phaethonfire]

I can't reproduce this. Scent was getting trapped in walls when going up or down stairs, and the walls were definitely causing this bug. There is a section of the code that says:

    if( move_cost == 0 && is_bashable) {
        if( m.has_flag("REDUCE_SCENT", x, y)) {
            grscent[x][y] /= 12;
        } else {
            grscent[x][y] /= 4;
        }

So any bashable door will reduce scent. I guess the non-bashable ones are triggering the problem you've seen. In the proposed code, that internal if statement will get traversed 4,800 times per turn, which is why I was trying to avoid coding it up like that, but it should fix the bug. After looking more closely, I don't think there are enough terrain keywords to get scent movement exactly right, and I opened a github issue to see if anyone has suggestions. I'm okay with merging this for now, but I'd like to remove this extra loop later if we can.

[freezerbunny]

@phaethonfire Really, you're holding the game back. Converting the loop to three if statements is easy. Converting it to one if-statement is also easy. Adding an additional flag to terrain objects and multiplying the it by the scent amount, removing all if-statements entirely, is also easy.

But.

Do we really care that much in a turn based game that's already running pretty fast.

I think not.

Stable game > fast game.

[BrianLefler]

@phaethonfire I gave it some thought, I should be able to replace the if
check with a multiplication, and it'd be fun to write a macro that unfurls
the loop for you transparently. I think that'll improve performance but
I'll need to measure it. I'll put you on the PR when it happens.

BTW, the easiest repro is to open an impassible lab door to a small room,
step in then out, and close the door. That will give ~5000 scent to an
unbashable space and it'll escalate quickly from there.

@freezerbunny Please keep a respectful tone and don't try to dictate how
contributors spend their efforts. You two can agree to disagree about how
important performance here is.

[swwu]

@phaethonfire, I do have to agree that worrying about an if block being traversed 4800 times is a little bit of a premature optimization. I haven't run an actual test in C++ but I imagine that a modern computer can easily handle tens of millions of if-block traversals like that one every second in a compiled language without much trouble, depending on what the functions do.

Either way, I think it's probably better to not worry about this kind of performance until it actually becomes an issue, as highly-optimized code is much more difficult to maintain, and increases the likelihood of somebody "rewriting" it to be "cleaner" at a later date.

[BrianLefler]

This code takes ~20ms to complete a single iteration, so that truly is visible to a player if they're trying to do something like walk in a straight direction (if everything else was instant, this code alone would limit us to 50 fps). More improvement here wouldn't hurt.

That said, I tried replacing it with an unwound solution that avoided both if checks and for loops, and the result actually looked 5-10% slower (as measured over 100 iterations). So I am not smarter than the compiler in this case. Here's the ugly alternative that I attempted, which would have replaced the if check with multiplying by 1 or 0. I'm at a loss for other good ideas, so leaving this code as is.

int can_diffuse = m.has_flag("BASHABLE", x, y - 1) || (m.move_cost_ter_furn(x, y - 1) > 0);
sum_3_squares_y[x][y] += grscent[x][y - 1] * can_diffuse;
squares_used_y[x][y] += can_diffuse;
can_diffuse = m.has_flag("BASHABLE", x, y) || (m.move_cost_ter_furn(x, y) > 0);
sum_3_squares_y[x][y] += grscent[x][y] * can_diffuse;
squares_used_y[x][y] += can_diffuse;
can_diffuse = m.has_flag("BASHABLE", x, y + 1) || (m.move_cost_ter_furn(x, y + 1) > 0);
sum_3_squares_y[x][y] += grscent[x][y + 1] * can_diffuse;
squares_used_y[x][y] += can_diffuse;