[PATCH] Implement the Anchor tag

[sanderd17]

An implementation of the anchor tag. As few calculations as I possibly could, to ensure a minimal slowdown.

For some (big) units, it might be worth to average the tiles they cover, up to now, only one tile is used to calculate the normal, but results are pretty good.

I also added a few <Anchor> tags where they were missing.

[historic_bruno]

When would we need "roll"?

[sanderd17]

For a segway? :D

I tried it for wall alignment: ​https://lh5.googleusercontent.com/wcN1B-hhTbFIoAC2syz1-LKlGUhSzOnOBaLJ1wywC2o6=w1246-h768-no

But it didn't work out. As it seems to mirror code a bit, I thought to leave it in, but it isn't used. It can just as well be removed.

[sanderd17]

It could be used for wooden barriers as these: http://www.wildfiregames.com/forum/index.php?showtopic=16959&st=20#entry262485

But since they're eyecandy, it would be a job for the map designer to get them on rather flat terrain.

[historic_bruno]

I guess it's possible we'll find a use for roll, so it can stay.

Comments after a more thorough review:

• Visually it looks very good. There are some glitches in extreme cases where for example, a cavalry unit can walk to the edge of a cliff and the horses back legs appear to be sticking into the air. But that's not really worse than without the patch.
• You shouldn't store the CTerrain* in CCmpPosition or access it directly, it should only be accessed through CCmpTerrain interface (this will also prevent breakages of tests where no terrain is defined)
• m_InterpolatedRotY isn't network synchronized and it's a float, so it can't be used in the calculations of m_RotX and m_RotZ (similarly it's not safe to call UpdateXZRotation() from the interpolation code, that's definitely not synchronized)
• widthInTiles, depthInTiles are unused, so they should be removed
• I get a compiler warning about not handling the ROLL case in the serialize switch statement. It should also follow the init logic making UPRIGHT the last case and falling through with default.
• The switch style is unconventional, see our coding conventions
• I think all quadrupedal fauna should also have pitch anchor, agreed? (horses, goats, etc.) If so, please add it to their templates.
• Not your patch but: some cleanup could be done, the mine templates redefine their anchors instead of using inheritance.

Think you can fix those and we could get it into A14? :)

[sanderd17]

I fixed the things you mentioned. But it's not that visually pleasing as the previous.

Because now the X/Z rotation can't depend on the interpolated positions, sometimes, a ram can rotate with the wheels in the hill. It's still better than without the patch (where the wheels are constantly in the hill), but not as pleasing as with the original patch.

Next to that, it doesn't work in Atlas anymore. You may notice the incosistence in the code (jumpTo sets a flag, while moveTo immediately calculates the new rotation). This is, when the map is loaded, there is a certain point where cmpTerrain exists but seems to be not completely loaded. Asking the normal results in a segfault. Since those methods only set a flag, the rotation is only updated on a turn (thus only when simulating the game).

[historic_bruno]

Would checking CmpTerrain::IsLoaded() fix the crash and the Atlas problems?

What about making the anchoring a visual only effect? It wouldn't actually change m_RotX,m_RotZ. That way you could use floats and update it during interpolation and it should have the same appearance as the original patch. (It would also avoid a lot of flags / special cases for updating the rotations?)

Another thing I was thinking: we need to be clear about how anchoring affects the XZ rotations set by the simulation. Should they be ignored, or should they be combined with the anchoring? The patches seem to overwrite the existing rotations, which isn't ideal.

[sanderd17]

• I'll try
• I also thought about making it a visual effect only. But I wonder if rotation changes something to the bounding box, which could be used in damage calculation (right now, only the footprint I used I think). So I didn't change that synchronising
• If you set the Anchor to something different than "upright", X/Z rotation will be overwritten. If the Anchor is set to "upright", you can manually change X/Z rotation.

[sanderd17]

CmpTerrain::IsLoaded() did the trick.

I tried making it a visual effect only, and it works quite good I think. Only the GetRotation() method has become a bit strange. Although it still returns the rotation if that's set be the SetXZRotation method.

Right now, setting the XZ rotation overwrites the Anchor, and will always have preference. Setting it back to zero gives preference to the anchor. We could also make a flag.

[historic_bruno]

The latest patch does look good most of the time, but there's one strange error. Around the cliffs and ramps on Acropolis I, I was able to get units to "oscillate" while standing still, they pitch back and forth quickly. I notice you compare two floats directly for rotY, shouldn't you be checking a range instead?

You can't tell from this screenshot but it does show the area of the map where the bug occurs, and it's not too hard to reproduce (see the cavalry). Also it appears the ram's anchor rotation wasn't updated properly when loading the map:

[sanderd17]

Sorry about the oscillation, it happened when in the time of one turn, you jumped to a next vertex, in that case, the previous XZ rotations weren't updated, so it kept interpolating between the old and the new position every frame. Now it should work as the previous XZ rotations are always updated, even if the XZ rotation isn't recalculated.

And for the not setting of the Anchor on loading, that's the same issue with the terrain not loaded. In that case the method would just return, but as long as you didn't change the position, it wouldn't update the rotation. Now I've added an extra check, as long as the terrain isn't loaded, it will try to recalculate the XZ rotation every interpolation. So by the time you see it, you should have all entities rotated correctly.

[sanderd17]

So, after the IRC discussions, I added some hybrid solution, with floats for the visual part, and fixed numbers for the engine interaction.

[sanderd17]

As it was discussed that the only_visual patch would fit best, I cleaned it up a bit according to IRC comments. The main change is that there are far less float-fixed conversions.

[ben]

In 13565:

Implements entity terrain anchoring as a visual effect, based on patch by sanderd17, fixes #1988.
Sets cavalry and quadrupedal animals to 'pitch' anchoring. Cleans up mine templates.

[historic_bruno]

Thanks for all the work on this!

I made some changes to the final patch before committing:

• in MSVC debug builds there was a bug due to uninitialized floats, causing all entities to appear rotated by 180 degrees, the interpolated XZ rots needed to be initialized to 0.
• we should check m_InWorld before calling UpdateXZRotation, since it uses XZ positions, they could be garbage if the entity isn't in world (plus it makes no since to calculate the rotations for an invisible entity)
• movement on hills was jerky, actually it was jerky at other times but on hills it was most noticeable since Y varies so much, I found that calling UpdateXZRotation with the interpolated Y value at the start of turns solved that (it was using the actual Y rot, so it jumped at the start of every turn or ~5 times per second)
• simplified UpdateXZRotation by always using m_X/Z and interpolated Y, I didn't notice any problems from this.
• replaced CTerrain* with CmpTerrain as discussed previously; also a CVector3D can be directly constructed from a fixed vector. I didn't notice any obvious visual difference between the fixed and exact normal calculations.

The other changes were minor code convention issues. Hopefully nothing was inadvertently broken.

[sanderd17]

I preferred CalcExactNormal for two reasons:

• CalcExactNormal uses the normal of the triangle the unit is on, while CalcNormal takes the normal of the closest vertex. If that vertex is on the edge of a cliff, the unit will have an unnatural XZ rotation.
• CalcExactNormal (contrary to what it's name makes you believe) is a lot cheaper in calculation than CalcNormal. CalcExactNormal only uses a single sqrt, while CalcNormal uses 4 sqrts.

So the difference is only visible on the edge of cliffs (hind legs sticking out as you said).

If the interface through CmpTerrain is preferred, CmpTerrain should have an aditional method that calls the CalcExactNormal function.

For the other things, you're correct, and thanks for fixing them.

[historic_bruno]

Ok, that's an easy fix and I'll change it now.

[ben]

In 13571:

Changes terrain anchoring to use faster CalcExactNormal instead of CalcNormalFixed, refs #1988, #2039