Re: [Flightgear-devel] apt.dat - add local version ?

[James T. <zak...@ma...>]

> On 17 Oct 2016, at 18:04, Florent Rougon <f.r...@fr...> wrote:
> 
> I don't like the conditional code either, but it is only 4 very small
> expressions or statements, and I don't want to lose the optimizations
> available with C++11. If you switch to C++11 soon in FG, I prefer just
> removing the conditional code when FG is ready, otherwise I can add
> macros as you said.

It’s fine, use your best judgement. 

> To be honest, I am a bit uncomfortable with this one. To me, it seems
> easier to understand/discover with everything in the same place (as long
> as it is not too big, of course). I think you're talking about these
> lines, mainly:
> 
> I'm not quite sure what you mean by "using a ‘d-ptr’ in the Qt sense",
> but I suspect you mean moving such things to a class similar to
> NavDataCachePrivate. If you think it's important, I can give it a try.
> But NavDataCachePrivate is referenced inside NavDataCache with:
> 
>  std::auto_ptr<NavDataCachePrivate> d;
> 
> which current gcc complains looooudly about, so with C++11 coming, I am
> not sure exactly about the pointer type to use. :)

Well this is a standard idiom (even in the design patterns book I think), there is no reason for GCC to be complaining about it? Being able to do a smart-pointer on a forward-declared classes’ pointer is 100% fine providing the otuer class has an out-of-line destructor implementation, is my understanding.
> 
> Well, it /miiight/ be that the unordered version is faster in some cases
> than the ordered one. No string splitting and joining, but more db
> insertions, and results on retrieval. Would have to be benchmarked to be
> quite sure... Also, the unordered version is slightly more general in
> that it doesn't have to sacrifice a character to use as separator; but
> since the separator can be freely chosen, this is most of the times not
> a problem.

Okay, fair enough.
> 
> Indeed, there is no enabling property, on purpose. For one, I consider
> that allowing several scenery paths (--fg-scenery & friends) but not
> additional apt.dat files was a bug---because it was impossible to ship
> new or improved airports correctly in the non-default scenery paths. My
> second argument is that I consider the code robust enough to be used all
> the time, and the third one is that having the feature optional would
> mean two different code paths, which would be harder to understand and
> maintain, and the one not enabled by default might have bugs without
> people reporting them, etc.

Hmm, yeah I can understand that argument, I still worry about it, but life is too short to worry about everything :/

> 
> On a second thought, if the enabling property was tested in
> NavDataCache::NavDataCachePrivate::getDatFilesPaths(), I think the
> maintainability and testing problems mentioned in the previous paragraph
> would be negligible, because only a tiny and simple part of the code
> would be conditional. However, I still think it would be some kind of a
> nuisance to ask users to set a particular property before they can
> correctly use custom sceneries that add or change airports. But that
> would be acceptable for me if it makes you happy. :)

Well again it comes down to if this is about making developer’s lives easier or users. Scenery developers should be contributing to the global scenery as quickly as possible, not trying to develop walled gardens of private scenery, or we will end up with a terrible situation.

If all the effort that people spent on custom sceneries went into making the global scenery better, it would have a much larger benefit for FG. Of course everyone wants to improve their back garden :(
> 
> Actually, it is the other way around: on page 10 of
> <http://developer.x-plane.com/wp-content/uploads/2015/11/XP-APT1000-Spec.pdf>,
> you can see that the apt.dat row code 14 corresponds to a "Viewpoint",
> and that it is apparently not used by X-Plane. It is FG that does
> cache->insert*Tower*() when it encounters such an apt.dat record.
> 
> It is possible to name a Viewpoint (last field of the record), but still
> according to this PDF, there is a “maximum of one viewpoint for each
> airport”. That probably justifies to systematically use it for "the
> control tower", so unless the apt.dat spec is changed (to allow several
> viewpoints per airport and make it easy to programmatically find which
> ones correspond to a control tower), I don't see much room for
> improvement…
Ah drat.

Really I want the tower at the Polderbaan to be another viewpoint at Schiphol….

Thanks for your work!

Kind regards,
James