Some `print-object` would be useful

[simendsjo]

Any tips on how they should be structured for inheritance? My small hacks below is very useful, but it isn't really suitable for the inheritance used by claylib. For instance path is a part of game-asset, but I print it from font-asset.

(defmethod print-object ((obj vector2) out)
  (print-unreadable-object (obj out :type t)
    (format out ":x ~A :y ~A" (x obj) (y obj))))

(defmethod print-object ((obj font-asset) out)
  (print-unreadable-object (obj out :type t)
    (format out ":path #P\"~A\" :size ~A" (path obj) (size obj))))

(defmethod print-object ((obj rl-font) out)
  (print-unreadable-object (obj out :type t)
    (format out ":size ~A :glyph-count ~A :glyph-padding ~A"
            (size obj) (glyph-count obj) (glyph-padding obj))))

(defmethod print-object ((obj text) out)
  (print-unreadable-object (obj out :type t)
    (format out
            ":text \"~A\" :font ~A :font-size ~A :color ~A :spacing ~A"
            (text obj) (font obj) (size obj) (color obj) (spacing obj))))

(defmethod print-object ((obj rl-color) out)
  (print-unreadable-object (obj out :type t)
    (format out ":r ~A :g ~A :b ~A :a ~A" (r obj) (g obj) (b obj) (a obj))))

[simendsjo]

Thinking about it, maybe the following is acceptable? But it doesn't print very nicely though

(defmethod print-object ((obj game-asset) out)
  (print-unreadable-object (obj out :type t)
    (format out ":path #P\"~A\"" (path obj))))

(defmethod print-object ((obj font-asset) out)
  (print-unreadable-object (obj out :type t)
    (call-next-method obj out)
    (format out ":size ~A" (size obj))))

#<FONT-ASSET #<FONT-ASSET :path #P"/home/simendsjo/.guix-profile/share/fonts/truetype/DejaVuSansMono.ttf">:size 16>

[simendsjo]

.. the following is a bit better, but the hierarchy leaks a bit. game-asset suddenly knows there are derived classes (and what if there isn't?), and font-asset knows game-asset will print without adding a blank character at the end:

(defmethod print-object ((obj game-asset) out)
  (format out ":path #P\"~A\"" (path obj)))

(defmethod print-object ((obj font-asset) out)
  (print-unreadable-object (obj out :type t)
    (call-next-method obj out)
    (format out " :size ~A" (size obj))))

[shelvick]

Agreed, this would be a nice enhancement. Not too difficult to add but the class hierarchy could be confusing to someone who's never worked with it before. Lots of slots-that-aren't-really-slots and objects containing other objects. The approach I would take is to comb through the src/ files and add a method in the same place for the objects/fields you want to print. I'd add a blank line at the end of each format to avoid having to care about whitespace so much. Don't feel pressured to PR this yourself unless you really want to.

[simendsjo]

If you can guide me on how you'd like it, I can do some grunt work. For a class c which derives from p and embeds e, would something like the following be acceptable: #<c #<c :parent-slot 1> :slot1 1 :slot2 #<e > :slot3 3>?

I think it would be better if the actual parent name was shown, and maybe something like :base-classes (#<p>) to support multiple inheritance?

What does other libraries do?

[shelvick]

would something like the following be acceptable: #<c #<c :parent-slot 1> :slot1 1 :slot2 #<e > :slot3 3>

Personally I like to see each value on a separate line, but if you're writing the code then you get to decide what looks prettiest (just be consistent). I think that's a good start at least.

maybe something like :base-classes (#<p>) to support multiple inheritance

That shouldn't be necessary if every method contains call-next-method. It will call print-object for the first superclass, then the next.

Since this is not performance-sensitive code, another approach would be to use the MOP, which will let you eschew some of the boilerplate. texture is a good example here since it has a bit of everything. This will get you all of the methods for texture and its superclasses:

(mapcan #'(lambda (c)
            (copy-list (closer-mop:specializer-direct-methods c)))
        (cdddr (reverse (closer-mop:compute-class-precedence-list (find-class 'texture))))

Filtering that list is an exercise to the reader. We only care about the standard-reader-methods and any standard-methods that take one argument, with a few exceptions: draw-object, initialize-instance, probably others. Note that you would not use call-next-method in this case.

That's what I would do: Use the MOP, figure out what we actually want to print, wrap that all up in a macro that spits out the defmethod. That's probably a bit much if you're fairly new to CL -- nothing wrong with the straightforward approach either.

[simendsjo]

EDIT: Never mind, I have an idea I'd like to try

---

That shouldn't be necessary if every method contains call-next-method. It will call print-object for the first superclass, then the next.

My problem was when to include print-unreadable-object. Doing so for each class looked quite awful, and it printed the most specific class name rather than the actual class which got matched in the method (not sure about the terminology here).

We only care about the standard-reader-methods and any standard-methods that take one argument, with a few exceptions: draw-object, initialize-instance, probably others.

Not sure if I understand, but it sounds dangerous. Do you mean I should call the standard-methods, printing it's result? Like seeing the GLYPH-COUNT method, and printing :glyph-count (glyph-count obj)? Printing could easily lead to side-effects if I use use an exclude-list for methods. For font-asset, I would need to exclude load-asset too, and as more methods is added, this list would quickly become out-of-date.

[simendsjo]

Here's an example. Per class, we list the slots/methods which should be added. The topmost class will use print-unreadable-object, while superclasses will only print directly.

A font-asset renders like the following (there's something strange with rl-font, an extra print-object somewhere..?). So :PATH is added from game-asset without creating any additional structure.

#<FONT-ASSET :SIZE 0 :ASSET #<RL-FONT :GLYPH-COUNT 0 :GLYPH-PADDING 0 :SIZE 0 #<RL-FONT {10038272A3}>> :PATH ~/.guix-profile/share/fonts/truetype/Cormorant-Bold.ttf >

The API is as follows:

(define-claylib-print-object rl-font (glyph-count glyph-padding size))
(define-claylib-print-object game-asset (path))
(define-claylib-print-object font-asset (size asset))

And the implementation:

(defun print-it (obj methods out)
  (dolist (method methods)
    (format out ":~A ~A " method (funcall (symbol-function method) obj))))

(defmacro define-claylib-print-object (type methods)
  `(defmethod print-object ((obj ,type) out)
     (let* ((class (find-class ',type))
            (top (car (closer-mop:compute-class-precedence-list (find-class (type-of obj))))))
       (if (equal top class)
           (print-unreadable-object (obj out :type t)
             (print-it obj ',methods out)
             (call-next-method))
           (print-it obj ',methods out)))))

What do you think about something like this?

[shelvick]

Yeah, you figured out what I should have said. :sweat_smile: This is good; we're already following a similar pattern in several places. Don't think it needs claylib in the name as this will be internal but otherwise I don't see any obvious problems.

[simendsjo]

Not sure if such initialized errors are bugs or if I need to handle unbound slots in my printer:

(defmethod print-object ((obj rectangle) out)
  (format out "~A" (gradient-style obj)))

(print-object (make-rectangle 1 2 3 4 +red+) nil)

;; The slot CLAYLIB::%GRADIENT-STYLE is unbound in an instance of CLAYLIB:RECTANGLE.

EDIT: By printing a special value for this case, it looks like the following for a rectangle

#<RECTANGLE :ORIGIN #<VECTOR2 :X 0.0 :Y 0.0 #<VECTOR2 {100CB89633}>> :ROT 0.0 :THICKNESS 1.0 :GRADIENT-STYLE #<unbound-slot> :X 1.0 :Y 2.0 :WIDTH 3.0 :HEIGHT 4.0 :COLOR2 #<unbound-slot> :COLOR #<RL-COLOR :R 230 :G 41 :B 55 :A 255 #<RL-COLOR {100CB89643}>> :FILLED T :POS #<VECTOR2 :X 0.0 :Y 0.0 #<VECTOR2 {100CB89633}>> #<RECTANGLE {100C9304F3}>>

It's quite verbose, and I'm pretty sure I print way too much data in same cases, so it's important to go through the PR in detail.