Add equator and ecliptic overlays (#109)#111
Conversation
alexchandel
changed the title
|
@da-luce can I get a workflow approval? |
Codecov Report❌ Patch coverage is
🚀 New features to boost your workflow:
|
|
Just checked this out--awesome! I'll try and take a closer look soon to better understand the code and then merge this in |
|
Thx! I can add some more test coverage if you need. |
This is exactly how I've tried approach |
|
Expanded the tests, think I got everything worth covering. Can you rerun the workflow? @codecov will you do that yourself, bot? There might be some inspiration in |
|
@da-luce needs workflow approval again apparently |
|
There is also a bikeshedding question of |
|
I honestly don't have a huge preference, especially because I'll reorganize the API in 2.0. Stuff has been busy and I haven't had time to really look through this but I'll get it merged in the next few weeks |
|
No rush. There are a couple corner case "partials" in the codecov still too, lmk if you need those to be covered. I've been using this build locally for a while, haven't seen any issues. I've noticed that if it's left running for like a full day, astroterm appears behind when I check in on it. Not sure if that's because of OS sleeping, or drift in the "game loop" logic, but worth noting. |
|
Did you want me to add test for those yellows in |
| update_planet_positions(planet_table, julian_date, config.latitude, config.longitude); | ||
| update_moon_position(&moon_object, julian_date, config.latitude, config.longitude); | ||
| update_moon_phase(&moon_object, julian_date, config.latitude); | ||
| config.julian_date = julian_date; |
There was a problem hiding this comment.
Eh kind of a mess, forgot to ask about that.
render_reference_circles() rotates the celestial circles for the observer's sky before stereographic projection. Other renders get passed updated object positions, updated right above L175 (where main passes in its static variable), but the rendered celestial circles are just reference circles sampled at render based on the time & loc.
- One simple option is to pass
julian_dateto therender_reference_circles(), tho that deviates from the(WINDOW *win, const struct Conf *config, ... /* objects */)signature of renderers. - Another option is to add it to
Conf; it's kind of conceptually similar to observer lat & observer long. However, other non-ObjectBase things (basically the constellations) still have their own object-like info passed in, but OTOH constellation lines don't require independent calculation fromjulian_date. - Cleanest option is probably to pass some trivial object to
render_reference_circles(), with maybe just a single value,julian_date.
May want to refactor observer lat + long + date into an "observer" object, which is different from the (otherwise timeless spaceless) config.
|
Tests look beautiful. Just a few questions/comments |
| int samples = MAX(height, width) * 4; | ||
| if (samples < 180) | ||
| { | ||
| samples = 180; | ||
| } |
There was a problem hiding this comment.
This empirically gives a descent resolution to me, but is there any specific reason for the * 4 specifically. I think having a magic number number here is fine, as there's no formula to determine the perfect resolution for given terminal dimensions.
However, having a comment explaining that this bit is sort of a guess would be nice. I might experiment with a rasterized approach sometime (though that would probably be horribly over-engineered).
There was a problem hiding this comment.
No, no exact terminal-resolution formula used there, just an empirical oversampling factor to avoid obvious chord gaps / jaggedness. I can comment that.
One might be able to (in a feat of overengineering) derive some slightly lower bound, but I don't think this is the bottleneck.
2 participants
Summary
--equatorand--eclipticoptions for the polar projection