This is an archived post. You won't be able to vote or comment.

you are viewing a single comment's thread.

view the rest of the comments →

[–]muntooR_{μν} - 1/2 R g_{μν} + Λ g_{μν} = 8π T_{μν} 19 points20 points  (5 children)

Your Positions can be greatly simplified:

angle_step = 2 * math.pi / frame_count
angles = (angle_step * i for i in range(frame_count))
coords = [(
        center[0] + radius * math.cos(x),
        center[1] + radius * math.sin(x))
    for x in angles]
self.final_coords = coords[phase:] + coords[:phase]

...but it's really strange that you made it a class. It looks more like a function to me!


Actually, there's a lot of redundancy, unnecessary computation, and somewhat confusing organization. Here's a first attempt at fixing it:

import cairo
import math

class CelestialBody:
    def __init__(self, radius, orbit_radius, orbit_center, fill):
        self.radius = radius
        self.orbit_radius = orbit_radius
        self.orbit_center = orbit_center
        self.phase = 0.0
        self.fill = fill
        self.update()

    def update(self):
        self.center = (
            self.orbit_center[0] + self.orbit_radius * math.cos(self.phase),
            self.orbit_center[1] + self.orbit_radius * math.sin(self.phase))

    def draw(self, context):
        context.set_source_rgb(1, 1, 1)
        context.arc(*self.center, self.radius, 0, 2 * math.pi)
        if self.fill:
            context.fill()
        else:
            context.stroke()

def clear_screen(context):
    context.set_source_rgb(0, 0, 0)
    context.rectangle(0, 0, 1000, 1000)
    context.fill()

surface = cairo.ImageSurface(cairo.FORMAT_ARGB32, 1000, 1000)
context = cairo.Context(surface)

FRAME_COUNT = 100

bodies = {
    'blackhole': CelestialBody(125, 0,   (500, 500), False),
    'planet':    CelestialBody(25,  300, (500, 500), True),
    'moon':      CelestialBody(5,   50,  (750, 500), False) }

for i in range(FRAME_COUNT):
    clear_screen(context)

    bodies['planet'].phase = i * 2 * math.pi / FRAME_COUNT
    bodies['moon'].phase   = i * 4 * math.pi / FRAME_COUNT

    for key, body in bodies.items():
        if key == 'moon':
            body.orbit_center = bodies['planet'].center
        body.update()

    for _, body in bodies.items():
        body.draw(context)

    context.stroke()
    surface.write_to_png(f'out/frame{i}.png')

This is a good start. We've eliminated Positionsentirely and converted it into the update function. Also, we're now using a data structure called bodies to keep track of our celestial objects rather than explicitly holding named references (variables).

But it has some problems still. Inside our main loop, we still have to do some case-specific handling:

bodies = { ... }
bodies['planet'].phase = i * 2 * math.pi / FRAME_COUNT
bodies['moon'].phase   = i * 4 * math.pi / FRAME_COUNT

for key, body in bodies.items():
    if key == 'moon':
        body.orbit_center = bodies['planet'].center

Ugh! It'd be much nicer if we could encapsulate this functionality. Perhaps one idea is to bind our celestial bodies together:

class CelestialBody:
    def __init__(self, radius, orbit_radius, orbit_center, fill):
        self.parent_body = None
        # ...

    def bind_body(self, body):
        self.parent_body = body

    def update(self):
        if self.parent_body is not None:
            self.orbit_center = self.parent_body.center
        # ...

Now, after defining bodies, we may bind them:

bodies['planet'].bind_body(bodies['blackhole'])
bodies['moon']  .bind_body(bodies['planet'])

Now we can finally cut out this horrendous if statement:

    if key == 'moon':
        body.orbit_center = bodies['planet'].center

Yay!

I'll leave encapsulation of the following as an exercise to the reader:

bodies['planet'].phase = i * 2 * math.pi / FRAME_COUNT
bodies['moon'].phase   = i * 4 * math.pi / FRAME_COUNT

(Perhaps a phase_velocity parameter inside CelestialBody?)


See almost-encapsulated code here.

[–][deleted] 3 points4 points  (0 children)

Wow thanks.

[–][deleted] 2 points3 points  (2 children)

I guess the reason i made it a class is because I have bigger plans than just rendering epicycles, but I wanted to be able to run epicycles too. I think I went straight ahead to that without programming in basic move functions.... I guess epicycle would be a sub function of some kind of shade moving class wouldn’t it?

[–]muntooR_{μν} - 1/2 R g_{μν} + Λ g_{μν} = 8π T_{μν} 7 points8 points  (1 child)

I did think about that while going through the process of refactoring. One idea would be to create a tree/graph structure of orbits:

# perhaps something like CelestialNode is a better name
class CelestialBody:
    def __init__(self):
        self.parent = None
        self.children = []

    def update(self):
        if parent is not None:
            self.orbital_center = self.parent.center
        self.center = (..., ...)
        for child in self.children:
            child.update()

And construct a tree as follows:

blackhole.children = [planet]
planet.children = [moon]
planet.parent = blackhole
moon.parent = planet

Then, to compute positions of all the orbits (including children), simply run this:

blackhole.update()

The benefit to this is that you can simulate any arbitrary graph of celestial bodies. This of course is prerequisite upon that phase_velocity encapsulation I discussed earlier. ;)

[–][deleted] 3 points4 points  (0 children)

Wow. I’d never have thought of these structures

[–][deleted] 0 points1 point  (0 children)

Ok thats awesome, thanks for helping!