Use Object::nextToDir in Room::updateTiles

[mspraggs]

I had a stab at using Object::nextToDir in updateTiles, as I get the impression nextToDir does what updateTiles does as well. It's not working quite right, in that the tiling doesn't work out properly.

How equivalent are the two bits of code in these functions?

[DivFord]

Had a brief look at Object::nextToDir. It's got some extra stuff with average dimensions and leeway. UpdateTiles used to just compare midpoints.

[mspraggs]

Ah right. I noticed the leeway thing before, but now I see the significance, I think. In any case, don't both bits of code try to compute the same thing, i.e. determine where the block is in relation to the current one?

[DivFord]

I assume so, but the new function isn't mine, so I'm not sure. The leeway has me a little confused, to be honest.

[mspraggs]

From the looks of it, the dimensions are averaged, then this is average is compared to the separation of the two objects. The leeway is just, I assume, to account for rounding errors in single precision.

[DivFord]

Left and right are swapped round in nextToDir.

I don't know what other code depends on it. Swapping the values in the enum is probably the safest way to correct it.

[mspraggs]

I just tried flipping the sign convention for the x difference in nextToDir, but this doesn't solve the problem. Perhaps we should see what Iain says, since he implemented nextToDir. Did you implement updateTiles?

[DivFord]

Originally, yes. What is the problem? Can you send a screenshot?

[Fyll]

nextToDir() is a bit funny in which way you call it. I can never remember, so I wrote a comment next to the function that tells you who should be calling and who should be passed in.

In order to get an accurate decision on whether or not two blocks were next to each other (as they may not be the same size) I felt that the average width method was the best. The leeway was then necessary as you say, to remove floating point errors. I've made it a parameter in case something ever wants a vaguer description.

Did you push the version with the signs swapped? I've just tested them, and the currently pushed version has all of the correct signs (I checked by changing the direction of the Player's gravity).

The only other bit of code (I think) that depends on nextToDir() is Room::checkFDG(). Again, are you sure you're calling it the right way round?

[DivFord]

Have you remembered to keep corners separate? If you're including those in the edges pass, that would explain the problem.

[Fyll]

The corners are described as if it is next to the object both horizontally and vertically.

[DivFord]

Yeah, but the original would only set a corner to true if the matching sides were false.

// Check top-right…
if ( (dir.y < 0 && dir.x > 0) && !(tilingIndex & 1) && !(tilingIndex & 2))
    tilingIndex |= 16;

So it had to do one pass to set all the edges for the tile, then a second to set the corners.

[Fyll]

So is that saying: If the block is up and right from this, and there isn't anything to the left or up from this?

Either way, would it not be easier to just check if there's a block in the space up and left from it?

[DivFord]

Nope.

Say we're setting the tiling for block A. We go through and compare it to every other block. Currently, we're comparing to block B. Block B is up and right from block A. However, we previously found that block C was right of block A. Since we can't have a corner that shares a side with an edge, the top-right corner doesn't get set.

 B
AC

So the way this has to work is we do two passes:

First, we look at every other block and flip the appropriate bit if we find any that are horizontally or vertically adjacent.

Then, we look at all the other blocks again, and flip the appropriate bit if we find any that are diagonally adjacent, so long as we haven't already flipped one of the corresponding edge bits (eg. top-right can only be set if neither top nor right is already set).

[Fyll]

Oh wait! I get it now. I thought setting the bits meant there was something in that direction. Whereas the bits actually mean there isn't something in that direction. Now it makes sense as to why you can't have a corner without an edge.

Wouldn't that still work, just instead of checking if the block is up and right:

(dir.x > 0 && dir.y < 0) && ...

, you'd just check if it's up-right from it:

(obj->nextToDir(check) == DIR_UP_RIGHT) && ...

(or possibly the other way around, as I said, I can never remember). Yeah it's a bit longer, but it's also clearer and uses the code we have.

[DivFord]

Yes, I think that's what we need to do.

[mspraggs]

I have managed to use the result of nextToDir in the first for loop in updateTiles (this is pushed). Now I just need to figure out the boolean logic to simplify the various if statements in the second for loop.

[DivFord]

You could have a function (let's call it ComponentSides(), or something like that) that returned the two sides for the corner, bitwise OR-ed together. So if you pass it DIR_UP_RIGHT, it returns DIR_UP | DIR_RIGHT.

Then the second loop would be:

if ( !(tileIndex & ComponentSides(obj->nextToDir(check)))
    tilingIndex |= obj->nextToDir(check);

[mspraggs]

Ok, but then why not just make DIR_UP_RIGHT = DIR_UP | DIR_RIGHT. (I think this has been asked before, I just can't remember why we shouldn't do it.

[DivFord]

Because having two edges isn't the same as having the corner between those two edges.

Compare top and right edges:

To top-right corner:

[mspraggs]

Right, got it. What I'll do is add a std::map to map the diagonal directions onto the ORed separate directions (or vice versa).

[mspraggs]

Ok this is done and pushed into the main repo