pioneer
pioneer copied to clipboard
Load stars from the HYG Star Database
This PR closes #4045 - with many thanks to @ecraven for the initial implementation in Scheme!
This is a long PR with many changes along the way, considering it's been in development for half a year now (cough due to my own laziness cough). I'll summarize quickly here, but the commit descriptions are where the meat of the PR is:
- Moved some functionality to the
core/
module (compression, IniConfig, etc.) - Added external libraries to
contrib/
:csv-parser
to simplify reading the HYG database,nonstd::string_view
for an easier way to pass around strings without copying them,argh
to replace our ad-hoc and ugly command line parsing - Added some more performance information and restructured the way it's displayed
- Made the performance info widget available in the main menu
- Moved the modelcompiler run step into its own target -
make -C build build-data
invokes this as well as the HYG database fetch and parse steps
A couple of notes:
- Fetching the HYG database requires a
curl
instance to be available on the command line (sorry Windows users!) - About 1200 stars from the DB aren't imported into the game; this is probably an oversight on my part, if someone wants to add logic to bind the remaining spectral classes to Pioneer's star types, feel free!
- There's tons of data in the DB that we're not using; the absolute magnitude and star color temperature are two that I've added to the parser but don't have anything in-game to hook them up to.
- The new modelcompiler step isn't currently run in the
build-travis.sh
script (easy to fix). - It's not really feasible to remove
02_local_stars.lua
, as you wind up with an empty bubble about ~80ly around Sol. Whether this is intended behavior is up for discussion (as well as whether the galaxy should be a lot less dense), but for now it's staying. - System name generation is really bad. I did some cursory searching and within about 200ly we have 10+ duplicate systems all named "Solia". That's something to improve on next time though...
Quick first note: first commit aught to also add an entry to AUTHORS due to the csv import lib.
Last issue is already tracked in #4866
Will test later.
Thanks for catching that, I didn't even notice that I forgot to add entries to AUTHORS for csv-parser and string_view_lite!
Because I'm my own worst enemy, I pushed the logging changes to this branch as well. I'll split that off if people don't want to review both of them at the same time...
Have compiled, confirm that custom systems went from 500 100,000 in debug info, after make -C build build-data
1. (that needs to be documented in COMPILING), 2. Confed faction has moved so that needs fixing. i.e. now only the 3(?) custom CIW worlds are left, in a sea of Federation systems, + swarm of CIW systems very far away from CIW homeworld.
Nice progress!
I don't think we need to support procedural assignment of systems to factions. We decided some time ago to not have procedural factions, at which point the code generating them was removed, but only after its output had been dumped as +100 faction files in data/factions
.
This step was the first taken in the attempt to reduce factions from hundreads, down to only three: SolFed, CIW, and Haber, (plus "independent"). The idea is that factions should be hand written, with lore, illegal-goods rules, positions, wiki, ship manufacturers, etc.
In summary: I wouldn't mind if each faction explicitly listed which systems (or sectors) it controlled, and then that's what's used in sector view. Either way, currently with this PR it sort of breaks CIW. I'll hold off on further review until some dicision / action on how to fix that has been taken.
@impaktor as far as I know, nothing in this PR has touched the faction code; if loading custom systems is causing issues with faction alignment, the best solution I see is to temporarily disable the build-hyg-data
step until the faction code can be brought up to spec.
While it's certainly an issue that the factions are not showing up as intended, I think that's out of scope for this PR.
Also: starting positions are messed up. See for instance New Hope (in orbit), or Bernard Star (crashes the game)
Sigh... Looks like this needs some more work then. I'll split the QOL changes into its own PR and this can bitrot for a bit while I work on something else.
Sorry if I've come off as negative, I really like moving to using the HYG* star catalogue, so I really appreciate the work you've done. I agree that merging the code - but leaving it inactive - could be a good idea, to prevent bitrot.
As a side note, from the forum thread on using star catalogue, @Brianetta wrote:
I've processed star catalogues into custom systems before, and the one big thing that kept tripping devs up back then was the fact that star catalogues contain stars, not systems. There's a lot of work to be done on a star catalogue to combine binary, ternary and so forth systems from their component stars in the catalogue.
*EDIT: just reminding myself that HYG=Hipparcos Catalog + Yale Bright Star Catalog (5th Edition), + Gliese Catalog of Nearby Stars (3rd Edition)
I stand by every word. Alpha Centaurus is s great example; Toliman (as was; the stars' names have been moved around IRL since the Pioneer project began) is at such a threshold distance from Proxima that it is unclear whether to call it a separate system. In the galactic core distances of that order are the norm.
Incidentally, I'm no sunk cost fallacist. If something is better or more fun, use it, even if what it replaced cost a lot of time and effort.
Yeah, this PR was mostly an effort to finish up ecraven's work and get it going; I'm going to take a break from this and work on another section of the code instead while I figure out the best way to handle it.
From what I can see, there are a few major tasks that need to be done before this can be merged:
- [ ] Process star coordinates to match Pioneer's coordinate system
- [ ] Merge close (how close?) stars into binary / trinary stars
- [ ] Add an override / modify mode to CustomSystem to allow easier editing of imported stars
- [ ] Add a "star blacklist" to the parser / importer to filter out specific cases of incorrect stars