Fix landing pads being buried on asteroids (better starport placement) #1503

Merged
merged 4 commits into from Sep 27, 2012

Projects

None yet

2 participants

@Ae-2222

A quick extension of the code that relocates starports if they are underwater.

  • 6 points around the starport center are checked to see if variation in height from the center exceeds a maximum limit.
  • If conditions are not met a 100 random locations are tried. The best fit is used for the new starport.
  • Does not guarantee a match will be found, and if planets are very spikey it might be impossible.
  • Does not check other buildings are buried, though it increases the likely hood the city is an a smooth area.

This sidesteps/mitigates the landing pads being buried mentioned in issue #7.

This doesn't change terrain under cities, and will eventually need to be replaced by an algorithm that does. It's an interim solution only.

Notes:

  • It is possible to increase the quality of matches by reducing the maximum height variation or increasing the number of tries. Given CPUs are capable of doing ~100k/sec of Height+Colour calls there is room for increasing the number of attempts each with 6 GetHeight calls.

Review:

  • There's a debug printout that's commented out.
  • Code for checking for underwater/spikey terrain doesn't really belong under Space:: but that's a wider restructuring issue.

Questions, To Do:
To configure the limits on height variation it helps to know:

  • what the distance from the center of the starport model to the outer edges of the landing pad are.
  • what the height of the landing pads are relative to the starport center.

Modellers?

As it is points at 20m distance are checked for height variation of 4m, and this solves the issue for Enki Catena.

Debug log:

Enki Catena: try no: 0, Match found: 0, best variation in previous results 10000000000.000000, variationMax this try: 7.653321, maxHeightVariation: 4.000000
Enki Catena: try no: 1, Match found: 0, best variation in previous results 7.653321, variationMax this try: 6.459278, maxHeightVariation: 4.000000
Enki Catena: try no: 2, Match found: 0, best variation in previous results 6.459278, variationMax this try: 5.423658, maxHeightVariation: 4.000000
Enki Catena: try no: 3, Match found: 0, best variation in previous results 5.423658, variationMax this try: 6.084463, maxHeightVariation: 4.000000
Enki Catena: try no: 4, Match found: 1, best variation in previous results 5.423658, variationMax this try: 1.145718, maxHeightVariation: 4.000000

Screenshots:
Enki catena relocated:

@Ae-2222 Ae-2222 Test if height variation will bury landing pads on planets where ther…
…e is strong variation in height relative to radius, and perform a short random search for a new suitable location. Use the best location found.
68cae05
@Ae-2222

I've manually checked all Sol starports on small moons and taking points over 20m works fine with the current slope.

There are two stations whose height variation fall just outside the criteria (4m) within 100 iterations, but since the criteria is conservative (a qualitative visual judgement), and the best result is chosen, there is no major issue. They are: Tomm's Sanctuary - 6.46m and Phobos Base - 6.2m.

This can be considered for merge as is, and if modellers can provide better data, that can be incorporated in a separate patch.

@Ae-2222 Ae-2222 Check for custom system starports which need to be relocated due to b…
…eing underwater and output to stderr.txt. Output relocation due to likelihood of custom system starports being buried as warnings.
3194b99
@johnbartholomew

It seems odd/incorrect to have two separate loops, one to check for water and then a second to check for terrain smoothness. What's to stop the second loop picking a location that's underwater?

What's the purpose of the BodyType == ASTEROID check for? You're checking maximum terrain height variation anyway, so the body type check seems unnecessary.

As you pointed out, this shouldn't really be in Space. I think it should be extracted into a separate function in Space.cpp, and then we can try to move it to a more sensible location later.

@Ae-2222

It seems odd/incorrect to have two separate loops, one to check for water and then a second to check for terrain smoothness

It would require conditions for the body of the loop as the heavy contents/logic are mutually exclusive (GetHeight calls are for a different set of points, only the create new random position bit is the same). The two conditions are extremely unlikely to overlap, but it can be fixed if needed.

What's the purpose of the BodyType == ASTEROID check for?

Terrain height variation based on maxheight is only an indication. What determines landing pads being buried is sudden variation, which might occur on even a fairly smooth planet. It's just there in case developed asteroid terrain will have a bias for high frequency fractals..


I've moved the relocation code out to it's own function pending some restructure in future.

@johnbartholomew

It would require conditions for the body of the loop as the heavy contents/logic are mutually exclusive (GetHeight calls are for a different set of points, only the create new random position bit is the same). The two conditions are extremely unlikely to overlap, but it can be fixed if needed.

It needs to be a single loop, otherwise a random location generated in the second loop can be underwater (the two conditions don't have to overlap for that to happen). The checks themselves can't be combined, but each candidate location needs to go through both tests to determine if it's valid.

Terrain height variation based on maxheight is only an indication.

It seems like it would be simpler and more robust to always run the local variation check. You've already made a good argument that it's not very expensive.

@Ae-2222

It seems like it would be simpler and more robust to always run the local variation check. You've already made a good argument that it's not very expensive.

The issue with this is that it might end up relocating a bunch of custom starports that have no problem on big smooth planets because the settings are conservative right now. Edit: Maybe custom starports should still use the current testing criteria.

It needs to be a single loop, otherwise a random location generated in the second loop can be underwater

I meant that water terrains are unlikely to occur (probably nothing is listed) on small bodies. I'll combine the two loops though..

@johnbartholomew

The issue with this is that it might end up relocating a bunch of custom starports that have no problem on big smooth planets because the settings are conservative right now.

Well, that's another reason not to relocate custom starports. Just emit a warning and let the human determine whether it's a genuine problem.

I meant that water terrains are unlikely to occur (probably nothing is listed) on small bodies. I'll combine the two loops though..

We get into problems with "unlikely". Partly because code can change and things that are currently unlikely might not always be unlikely. Partly because it's a big galaxy so unlikely things still happen. I'm prepared to accept that some things just can't be fixed easily and we have to rely on the probability being in our favour, but for things that are fairly simple to deal with robustly I'd like to do that. Putting it in one loop isn't particularly difficult, doesn't add any processing cost (it's just doing the same things in a different order after all), and it prevents something that logically could happen, so I think it's worth it.

@Ae-2222 Ae-2222 Combine underwater and variation checks and apply to all planets whil…
…e relocating on all planets except for custom starports on planets where finding flat ground will not be very hard.
b24d2e6
@Ae-2222

Well, that's another reason not to relocate custom starports. Just emit a warning and let the human determine whether it's a genuine problem.

The problem is on planets like Ganymede (Enki Catena) where the ground has a lot of variation everywhere. Trying out different locations might be time consuming. I've added automatic relocation code for underwater starports and asteroids/planets with high variation, the rest will receive a warning.

This might cause quite a few warnings in something like the Hometown mod, but if star ports have been placed on fairly flat ground I assume there won't be too many.

The region code should be ready to deal with starports on big worlds after a little more work, so it's more for the asteroids in the medium term.


The two loops have been combined using a lot of bools to make it followable and all starports are relocated except for custom starports on big fairly relatively flat worlds that can be relocated manually without issues.

@johnbartholomew johnbartholomew merged commit b24d2e6 into pioneerspacesim:master Sep 27, 2012
@johnbartholomew johnbartholomew added a commit that referenced this pull request Sep 27, 2012
@johnbartholomew johnbartholomew issue #1503 is supposed to fix #7 fa91a71
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment