FGData Merge Request #212: Boost the UFO and add slightly increased resolution for the moon texture (merged)

This patch has two commits:

-one boosts the UFO by adding two more available speed: 500 km/s and
1000 km/s. Moreover, the displays now show speed in km/s. It allows you to go to the Moon in more or less five minutes, but be careful, watch your throttle while on Earth: that is FAST!

-the other is a slightly increased resolution for the moon texture (1024x512), made from public domain images of LROC and GPLv3 code from chris_blues and myself (https://github.com/eatdust/LROC2FGearthview). The previous one has been renamed moon_old.png, and the previous previous one moon_very_old.png

[James Turner]

I think we can delete the old files, that's what version control is for :) Unless there is some active code path which uses them?

About the UFO, I'm not sure it's wise to add such fast values, if you really need to be that careful? Seems like something you could just mod locally when testing the moon code, or do you expect other people to need it?

Km/s is a weird unit for a flight simulator as well: I'm wondering if you need a 'spaceship' mode for the UFO which adds these features, which I guess are very useful for space-sim work only?

[eatdirt]

Hi James,
nothing of all these is "necessary", just improvements in my opinion, I am just proposing. Feel free to reject that merge if you think they're not adequate! (only the FGData)

For the UFO, it is not dangerous at all. First of all, it is like if I've added two gears on top of an already existing ladder of gears. When the UFO is loaded, it defaults to a reasonable max speed. So, if you push the throttle to full, you'll move as before, reasonably fast, but not crazily fast.

To engage the additional "gears", as before, you have to voluntarily press "]" to go up to the next level of the ladder, which only changes the maximal throttle speed. There is no way someone would do this in an involuntary manner (well, maybe my 2 years old kid could!). My adds, are at the end of that ladder. Instead of topping to 100000 m/s, the additional gears add 5x more and 10x more, i.e. 500000 and 1000000 m/s.
Now, I think you understand why I switched the unit to km/s... Even right now, you cannot understand the maximal speed when displayed, because there are too much zeros, which, honestly, is kind of ridiculous. If the UFO can fly at 100000 m/s, it is natural to display it like 100 km/s. If the SI unit is a problem, we could switch it to kft/s, or, more naturally, for Earth, to knm/s, but the k has a purpose to allow you eyes and brain to get the speed without using your finger to count the number of zeros on the screen :)

I've tried ground texture loading at 1000km/s, it works fine, although you're faster than the loading time even with textures on SSD. That is a good stress test I suspect.

if you need a 'spaceship' mode for the UFO

I am not sure if I get that, the UFO is a spaceship per se no?

For the Moon, the improvements are not really to go there, but to get better screenshots while zooming. The only concern may be extra graphics load, but it is a mild increase. Of course, that is a bit subjective. It used to be rendered with 15x15 points, and I increased it to 40x20 to make it less hexagonal, the texture is changed from 512x512 to 1024x512. The new map contains shadows and supposedly accurate albedo (so, it cannot not be used with a normalmap to render shadows, but we don't do that anyway).

I read on the forum that Thorsten updated the last moon.png, so maybe he would have some objections/comments for replacing/removing the old texture?

[James Turner]

If you can ask Thorsten about the moon texture, that would be good.

About the UFO speed, what I'd recommend is we keep using m/s until the value gets to large, i.e add an if/else clause to the popup text computation? My concern is we don't want to display 0.01km/s at 'slower' speeds, that's not good either. So keep the current display for the low gears, and use km/s for the new top settings?

[eatdirt]

Yes, good idea for the if/then. I'll ping Thorsten about the map!

[Thorsten Renk]

I'm (of course) fine with changing the moon map. The last ufo change also seems reasonable to me.

May I however suggest for the future to not mix up completely unrelated merge requests? It makes reviews very difficult, one can be of the opinion that changing moon visuals is a good idea but changing the ufo is not - or vice versa.

Anyway - if James has no further objections I'll put my vote in for pressing the merge button on that one.

[eatdirt]

Yep, I'll narrow down my future merges then! Sorry!

[James Turner]

Sorry, yes I should have pointe dout: a comit shoudl solve one problem, not two unrelated ones. So please have:

• one commit for the moon texture
• one for the UFO speed range additions.

Then we should be good to go.

[eatdirt]

Sorry for the naive question, when you say "commit", do you refer to as "merge request"?

Because, I had exactly two commits on that merge request, one for the Moon, one for the UFO, the UFO became two commits due to the latest fix you suggested.

Or if you really refer to as "commits", you mean that I should merge into one the last UFO fix such that it appears as only one commit?

Thank you!

[James Turner]

For the future, I woudl say: one MR for the moon, one MR for the UFo change. A MR cna contain multiple related commits, but I nearly always squash them into one when applying; for the overal history, there's no value in the two UFO commits remaining seperate. The reason for the two MRs would be that they are unrelated (one might be approved, the other rejected).

Anyway, no big deal, I'll get them merged shortly.

[eatdirt]

Ok, great. I'll pay attention! Thanks.

[James Turner]

Merged, with some changes:

• squashed the two UFO commits together
• removed the old and very_old Moon textures, that's what Git is for :)