Variants and renderer system

[dormieriancitizen]

Description of the changes

This is a successor to #636 that I think is a better fit for BD. Resolves #275, supersedes #275.

It introduces two ideas: Renderer and Variant.

A Variant is kinda like a special but specific to a given Ball. In it you can specify a different collection_card than normal for the rendering of a specific BallInstance, and also set a renderer.

A renderer is just a function that takes a BallInstance and outputs an image. image_gen.py gets a dict of renderers, which have string keys mapping to renderers.

Renderers can be set in four places: in a Variant (applies to BallInstance), in a Special, in a Ball, and the default renderer (in descending order of precedence).

For this PR, I've written 3 (also mildly refactored the existing renderer): default, blackscreen (for testing), and fullart (names are hopefully self-explanatory).

Some examples

Here is Chewytapioca's full card art China with the fullart renderer

Here is another China ball again rendering with the default one.

Here is another China ball, with the default renderer, but I poorly drew a birthday hat on it.

All of these examples are BallInstance specific and require no intervention after setting the Variant on the panel.

Extensibility

If a package wants to add a renderer, all it has to do is append to the RENDERERS dict in image_gen.py, and everything should work out.

Were the changes in this PR tested?

Yes, though not for performance (presumably there's some overhead)
There could be some polishing done on the admin panel probably.

[Caylies]

Bringing my suggestions from the developer server, a small simple framework should be implemented as the current implementation is muddled. A Renderer class should be implemented that provides default methods and a clear concise structure to improve card creation.

Additionally, a component-based system could be introduced to allow easy modification of card elements. Components could be declared using a @component(...) decorator (a decorator was chosen as its intuitive and idiomatic).

@component(name: str)
def new_component(self):
    pass

Renders could have the following methods:

• extend_components() - Adds all components from the base class renderer.
• add_component(name: str, order: int) - Adds a registered component.
• remove_component(name: str) - Removes a component; stops it from rendering.

Proposed FullArt example renderer syntax:

class FullArt(Renderer):
    """
    Handles full card art.
    """

    def render(self):
        self.extend_components()  # Adds the components from `Renderer`
        self.remove_component("artwork")

        self.add_component(name="rarity", order=1)

    @component("rarity")
    def rarity_text(self):
        self.draw.text(
            (520, 1670),
            str(self.ball_instance.rarity),
            font=stats_font,
            fill=(255, 255, 255),
            stroke_width=1,
            stroke_fill=(0, 0, 0, 255),
        )

[dormieriancitizen]

I'm not really a fan of a class-based system, since a) whatever draws the card art really should be stateless (sans caching), and b) classes add a decent bit of overhead.

I don't really think that individually-overrideable components are simpler, and adding a whole bunch of decorators and components adds a whole lot of OOP complexity that I really don't think is needed. There'd also be a bunch of maintenance burden involved and mental modeling with trying to keep track with what is overriden and what isn't, so I think it's better just to have each renderer be a function, and if someone wants to duplicate code from the function, they can just duplicate the code, there's not really a need for premature abstraction here.

[Caylies]

I'm not really a fan of a class-based system, since a) whatever draws the card art really should be stateless (sans caching), and b) classes add a decent bit of overhead.

I don't really think that individually-overrideable components are simpler, and adding a whole bunch of decorators and components adds a whole lot of OOP complexity that I really don't think is needed. There'd also be a bunch of maintenance burden involved and mental modeling with trying to keep track with what is overriden and what isn't, so I think it's better just to have each renderer be a function, and if someone wants to duplicate code from the function, they can just duplicate the code, there's not really a need for premature abstraction here.

Your points are good, but there are a few flaws and misconceptions. I'll also include extra points I didn't state in my previous comment.

1. "Just duplicate the code" falls flat when you have renderers that build off of other renderers. If FullArt extended off of the base renderer, and there was another renderer that extended off of FullArt, a single coordinate change would have to be tracked and corrected across many derived copies.

2. I'm not focusing on theoretical purity, but rather developer experience. The proposed implementation is unorganized and can't have renders inherit from each other without duplicated code (which isn't recommended). I didn't suggest decorators and components for abstraction; they're there to make the renderer's intent immediately readable. A developer seeing remove_component(name="artwork") and add_component(name="rarity", order=1) will understand what that renderer does differently in two lines, without reading a full implementation.

3. Class instantiation adds nanoseconds of overhead to an operation that already involves heavy image processing. Rejecting the class model due to the few nanoseconds it allocates is a premature optimization.

4. In terms of 'mental modeling', with freestanding functions, a developer has to manually diff two full implementations to understand what one renderer does differently from another. With the class model, that diff is written out in the render method. The override surface is smaller and self-documenting (due to its simplistic naming), compared to the current approach.

5. put_card_info currently handles title, capacity, stats, rarity, credits, and economy icon all in a single 60-line function. With the component-based approach, each responsibility becomes its own named component. Renderers that don't need a component can simply omit it.

Overall, I still believe a class-based approach would fit better, similar to the class-based approach that spawn managers have.

[dormieriancitizen]

Just to be clear, I'm talking mostly about implementation complexity, not usage complexity. This is not intended to be a perfect DSL on top of python with a full component system, it's intended as a minimal viable system to replace the if soup that BD currently uses for things like frames and whatnot.

I don't really see a need to add a vast amount of implementation complexity for what appears to me as extremely small advantage over composing functions. If a third-party-developer wants to implement the kind of thing that you're describing above, it's very possible to do on top of the system in the PR here inside of a package, but I don't think there's much value in putting that kind of system into core BD.

I agree that put_card_info could be split up, but I've left it as is for now just because I don't currently want to fully re-implement all the idiosyncrasies of the card generator in a better system.