Ability to add additional superclasses to a sketch.

[Kevinpgalligan]

trafficstars

First draft. Adds a defsketchx macro that allows specifying additional superclasses for a sketch, which means that behaviour can be shared between sketches and it's easier to develop extensions for sketch.

(As per: https://github.com/vydd/sketch/issues/142).

Usage example:

(defclass black-background () ())
(defmethod draw :before ((instance black-background) &key &allow-other-keys)
      (background +black+))
(defmethod setup :before ((instance black-background) &key &allow-other-keys)
      (format t "Setting up black background!"))
(defmethod initialize-instance :before ((instance black-background) &key &allow-other-keys)
      (format t "Initializing black background!"))

(defsketchx my-sketch (black-background)
        ((width 200)
         (height 200))
      (circle 100 100 50))

I've also tested out sharing state between a sketch and its superclass, e.g. accessing a superclass slot from the sketch class, and it seems to work as expected, though this requires the user to be know that they can do (slot-value *sketch* 'slot-name) in the sketch body. I don't think that's documented, and I don't think it would work outside the sketch package because *sketch* is not exported.

[vydd]

Interesting! Have you considered making it backwards compatible by creating something like

(defsketch foo ((mixins (a b c)))
  (do-stuff))

?

[Kevinpgalligan]

I have considered that, might be nicer than having an entirely separate interface. Another possibility:

(defsketch foo (:mixins (a b c)
                :other-option (x y))
   ...)

The only tricky part would be testing whether theres an options list, I guess it should be backward compatible to test whether the first item in the body is a plist.

[vydd]

But that's still not backwards compatible, is it? I mean defsketch is, but arg list isn't - so if you want to add mixins to an old sketch, you also need to add the old options under ... :bindings?

I think just using mixins with a list works ok

[Gleefre]

What about

(defsketch foo ((:mixins (a b c))
                (some-variable 'hello))
  (do-stuff))

or maybe just

(defsketch foo ((:mixins a b c)
                (some-variable 'hello))
  (do-stuff))

By using a keyword name we show that this parameter is different from usual slots; but the API is kept backward compatible and uniform.

[vydd]

@Gleefre what are the cons of using the non-keyword version? is it about the need to figure out what happens when you change it from the code?

[Kevinpgalligan]

I might be wrong, but I think my version is backward compatible? The bindings would still be there...

(defsketch my-sketch (:mixins (a b c) :some-future-option 42)
    ((width 400)
     (height 400))
  (circle 100 100 50))

And the code-generating logic would check whether the first item after the name is a plist. If so, treat it as the options plist and the first element of the body becomes the bindings. Otherwise, there are no options, and we treat the item after the name as the bindings. Like so:

 (defun make-sketch-class-def (name options-or-bindings body)
   (if (is-plist-p options-or-bindings)
       (let ((options (parse-options options-or-bindings))
             (bindings (parse-bindings (car body)))
             (body (cdr body)))
        (make-def name options bindings body))
       (let ((options nil)
             (bindings options-or-bindings))
         (make-def name options bindings body)))

I'm also open to making the configuration options part of the bindings list, of course. Depends on which option ye think is neater.

And yeah, I think the point of using keyword args is that they're easily distinguishable from regular bindings and should be backward compatible in case someone happens to have a slot called mixins.

[Kevinpgalligan]

I went with your first suggestion, @Gleefre ! Now options like :mixins can be passed at the start of the bindings form.

My example usage is unchanged, except it now looks like:

(defsketch my-sketch
        ((:mixins (black-background))
         (width 200)
         (height 200))
      (circle 100 100 50))

As expected, it generates the following code:

(DEFCLASS MY-SKETCH (SKETCH BLACK-BACKGROUND) NIL)

Oh, I forgot to update the documentation.

[Gleefre]

what are the cons of using the non-keyword version? is it about the need to figure out what happens when you change it from the code?

It is an interesting question what should happen if mixin list is changed... Dunno.

My point of using keyword is to make it more readable -- it hints that this is not a slot (since you can't have variables named by keywords); but an additional option to the macro itself.