beyond-all-reason / Beyond-All-Reason

Main game repository for Beyond All Reason.
https://www.beyondallreason.info/
Other
1.72k stars 288 forks source link

Flooded map, seaplanes underwater #495

Closed Teifion closed 2 years ago

Teifion commented 2 years ago

Greenfield with water depth of 600. Seaplanes fly underwater and are not targeted by AA but instead by Torpedoes.

Beherith commented 2 years ago

Regular planes are fine, and the smoothheightmesh is correct as well. So bombers and fighters (not vtol) seaplanes work fine, but vtol type like gunships and cons seem to follow the terrain underwater. Do vtols use a different heightmesh?

Beherith commented 2 years ago


static inline float AAMTGetGroundHeightAW(float x, float z) { return CGround::GetHeightAboveWater(x, z); }
static inline float AAMTGetGroundHeight  (float x, float z) { return CGround::GetHeightReal      (x, z); }
static inline float AAMTGetSmoothGroundHeightAW(float x, float z) { return smoothGround.GetHeightAboveWater(x, z); }
static inline float AAMTGetSmoothGroundHeight  (float x, float z) { return smoothGround.GetHeight          (x, z); }
static inline float HAMTGetMaxGroundHeight(float x, float z) { return std::max(smoothGround.GetHeight(x, z), CGround::GetApproximateHeight(x, z)); }
static inline float SAMTGetMaxGroundHeight(float x, float z) { return std::max(smoothGround.GetHeight(x, z), CGround::GetHeightAboveWater(x, z)); }

static inline void AAMTEmitEngineTrail(CUnit* owner, unsigned int) {
    projMemPool.alloc<CSmokeProjectile>(owner, owner->midPos, guRNG.NextVector() * 0.08f, (100.0f + guRNG.NextFloat() * 50.0f), 5.0f, 0.2f, 0.4f);
}
static inline void AAMTEmitCustomTrail(CUnit* owner, unsigned int id) {
    explGenHandler.GenExplosion(id, owner->midPos, owner->frontdir, 1.0f, 0.0f, 1.0f, owner, nullptr);
}

AAirMoveType::GetGroundHeightFunc amtGetGroundHeightFuncs[6] = {
    AAMTGetGroundHeightAW,       // canSubmerge=0 useSmoothMesh=0
    AAMTGetGroundHeight  ,       // canSubmerge=1 useSmoothMesh=0
    AAMTGetSmoothGroundHeightAW, // canSubmerge=0 useSmoothMesh=1
    AAMTGetSmoothGroundHeight,   // canSubmerge=1 useSmoothMesh=1
    HAMTGetMaxGroundHeight,      // HoverAirMoveType::UpdateFlying
    SAMTGetMaxGroundHeight,      // StrafeAirMoveType::UpdateFlying
};```
This seems to indicate that hoverairmovetypes movetypes dont read the heightmap correctly for some reason
lhog commented 2 years ago

Woah nice ticket. Should dedicate some time to trace it.

Beherith commented 2 years ago

Useful info: bug only present if modoption waterlevel_map = 300 or some silly method of lowering map is done. The best unit to test with is armcsa (arm construction seaplane), which has HoverAirMoveType, and its corresponding HAMTGetMaxGroundHeight is likely the culprit

marcushutchings commented 2 years ago

ith water depth of 600. Seaplanes fly under

So how does one replicate the issue? I don't I see that map in the list.

Teifion commented 2 years ago

Any map with a sufficiently raised water level would likely produce the same results.

marcushutchings commented 2 years ago

Which maps do that? Sorry, I'm not as familiar with the maps.

Teifion commented 2 years ago

It's a setting (Under "Other" I think) which allows you to change the waterlevel. Sorry, I realise how what I said made it sound like it was the map itself.

lhog commented 2 years ago

any map, you can also try in the game /luarules waterlevel 800 (after /cheat)

marcushutchings commented 2 years ago

Okay, so if the waterlevel is changed after the map is loaded, then you can get underwater aircraft. Okay, that makes sense because I don't believe the smooth airmesh is ever updated after initial load.

marcushutchings commented 2 years ago

Did this just apply to sea planes?

marcushutchings commented 2 years ago

As a further observation: all trees and rocks are placed ontop of the water if you start the map with a raised waterlevel and they would thave otherwise started on land.

Teifion commented 2 years ago

Did this just apply to sea planes?

We flooded the entire map so were only able to test with airplanes. I've just finished work so am happy to assist in testing if you need.

marcushutchings commented 2 years ago

I think it may be working now, by virtue of the changes that save/load brought about. Working at least if you set the waterlevel in the lobby.

marcushutchings commented 2 years ago

Okay, Sea planes are wokring slightly differently. I see.

marcushutchings commented 2 years ago

Doesn't look like Seaplanes use the smooth airmesh.

marcushutchings commented 2 years ago

Stupid question on my part: is the seaplane aircraft at rest/grounded allowed to submerge into the water, or is it supposed to sit on top?

marcushutchings commented 2 years ago

Okay, I have a solution. Because the unit is allowed to submerge, it tracks the ground height to determine the correct altitude. If we agree that it makes no sense that a HoverAirMoveType should be travelling underwater (landing and takeoff are fine) then I can place a special check to ignore the submergable property with the unit is trying to fly (it will still be able to land submerged)

marcushutchings commented 2 years ago

@Beherith does this sound acceptable to you? Note that this will be a change of behaviour of existing code and thus there is a chance that Zero-K or a mod for it may be impacted (if, for some strange reason, they want HoverAirMoveType units to fly underwater.)

Teifion commented 2 years ago

It makes sense to me at least :)

lhog commented 2 years ago

Because the unit is allowed to submerge, it tracks the ground height to determine the correct altitude.

Curious why it fails to gather the correct altitude in such case? Waters are too deep?

Beherith commented 2 years ago

I still feel that the issue arises from hoverairmovetype getting stale data in: static inline float HAMTGetMaxGroundHeight(float x, float z) { return std::max(smoothGround.GetHeight(x, z), CGround::GetApproximateHeight(x, z)); }

Only @googlefrog would know if ZK is impactec by this.

I still feel that this entire fuckery with luarules setting ground heights is the wrong way of setting this: The heightmap data for a map is stored as floats [0;1]. Then, the mapoptions.lua defines the smf fields minheight and maxheight. Then when the games loads the map, the [0;1] floats are scaled to be between minheight and maxheight. Some newer maps have mapoptions (similar to modoptions) that override this minheight and maxheight, resulting in correct behavior for everyone.

Now when a game does it by setting it in any other way, it always results in stupid bugs like this. So imo the best option would be to have an engine-level mapoptions_post.lua, which has two factors (linear bias and gain) that get applied after the maps' mapoptions.lua is loaded, and act as if all maps had this mapoption present.

The engine, as far as I know, doesnt know anything which script tag is a map or a mod option, its up to the games/map to parse them.

marcushutchings commented 2 years ago

Well, believe it or not, it determined the altitude correctly (just not the same correctly you are thinking Ivand :) .) There are several methods to track the ground height, but generally they all track the ground heightmap to some degree; however non-submersible units will treat ground that drops below the waterline to be at height zero (i.e. won't go below the water level.) submersible units continue to track the ground under the water. Typically the aircraft try to keep a fix height above the ground. The seaplane constructor 'armcsa' unit (which is submersible) tries to keep about 52 elmos above the ground. So if it flies over the sea where the ground is say (-100), then the unit with aim to fly at (-48) - that puts it underwater, but it is still 52 elmos above the ground.

Beherith commented 2 years ago

That would imply that on the map Cape Violet V1, where the water is -130 elmos deep, that armcsa would attempt to fly at ~80, which it certainly does not, it flies above the water.

image

marcushutchings commented 2 years ago

This is interesting. I see what you mean. That is strange. From my readings, the seaplane can't see the ground height while over the water until it decides to land.

marcushutchings commented 2 years ago

Okay. The problem here is that most of the engine assumes smooth ground does just that, but it doesn't. It smooths ground, but no further than the waterline. As you can imagine, that causes a few problems. In this case AAMTGetSmoothGroundHeight behaves the same as AAMTGetSmoothGroundHeightAW (unless you mess around with waterlevel ;) ) - and interestingly sea/vtol planes seems to rely on this and only appears to work because the logic in the engine is wrong. I have a working fix where I have corrected smooth ground, so that it will follow the terrain underwater (this fixes issues cause by waterlevel changes) and I have also made sure that AAMTGetSmoothGroundHeight keeps its original, incorrect behaviour (which allows sea/vtol to work correctly.)

marcushutchings commented 2 years ago

Thanks for your insight about cape violet Beherith. That was very helpful!

marcushutchings commented 2 years ago

Fix provided in spring engine. https://github.com/beyond-all-reason/spring/commit/ef491dd7d3e3c7980b68032d5dac55e4507d2701

GoogleFrog commented 2 years ago

Zero-K does not have submersible aircraft or use smoothmesh. I think changing ground height via lua makes perfect sense and should function properly. Hacking in a special case via a separate method for the only application BAR wants seems a bit dodgy. I was told (ages ago) that lua was responsible for updating smoothmesh when heights are changed.

Beherith commented 2 years ago

much <3 TK!