vydd / sketch

A Common Lisp framework for the creation of electronic art, visual design, game prototyping, game making, computer graphics, exploration of human-computer interaction, and more.
MIT License
1.4k stars 67 forks source link

initialising interdependent variables inside the defsketch macro #33

Closed inconvergent closed 2 years ago

inconvergent commented 4 years ago

Hi

I have a problem when it comes to creating objects with their own state when using the defsketch macro.

Here is simple (and quite pointless) example:

#!/usr/bin/sbcl --script

(load "~/quicklisp/setup.lisp")
(ql:quickload "sketch")

(in-package :sketch)

(defun change-state (a)
  (setf (gethash 'v a) 1)
  a)

(defsketch nier ((title "weir")
                 (width 1000)
                 (a (make-hash-table :test #'equal))
                 (b (change-state a))
                 (height 1000))

  ; expect this to print (:A 1 :B 1)
  ; but it prints (:A NIL :B 1), so a has not been altered.
  (print (list :a (gethash 'v a) :b (gethash 'v b)))

  (background (gray 0.1)))

(defun main ()
  ; can i pass variables here somehow instead?
  ; i've tried eg. (make-instance 'nier :a 3), but this does not 
  ; affect the value of a inside defsketch
  (make-instance 'nier)
  (sleep 100))

(main)

So I'm wondering if this is expected behaviour? (I sort of assume it is.) And what is the preferred way of initialising multiple interdependent things inside defsketch, where those things have an internal state?

As a sidenote, evaluating the following code has the behaviour i'm expecting

(let* ((a (make-hash-table :test #'equal))
       (b (change-state a)))
 (print (list :a (gethash 'v a) :b (gethash 'v b))))

Setup

I see that there is a setup method in sketch, but i haven't found any documentation or examples anywhere. Perhaps this is the correct approach? If so, could someone provide an example, please?

Thank you for this library either way, it is precisely what i need! Best Anders

inconvergent commented 4 years ago

I've noticed that if i print the assignment of b inside the macro

(b (print (change-state a)))

this will happen twice. Which i assume is the explanation for this behaviour i'm seeing.

inconvergent commented 4 years ago

For anyone else who ends up here. Perhaps this is an example of the way setup should be used: https://github.com/vydd/qelt/blob/master/qelt.lisp#L275 (I will try this, and report back)

vydd commented 4 years ago

@inconvergent Weird. Thanks for reporting this. I'll try to find some time to look at it during the weekend.

inconvergent commented 4 years ago

@vydd no problem. I had a look at the expanded code from the macro. I'm not too sure about how classes/methods work, but i noticed that i could make the following change to get the behaviour i need (expanded with pg's mac-macro):

(PROGN
 (DEFCLASS NIER (SKETCH)
           ((A :INITARG :A :ACCESSOR NIER-A) (B :INITARG :B :ACCESSOR NIER-B)))
 (DEFMETHOD PREPARE PROGN
            ((INSTANCE NIER) &REST INITARGS &KEY &ALLOW-OTHER-KEYS)
   (DECLARE (IGNORABLE INITARGS))
   (LET* ((FULLSCREEN (SLOT-VALUE INSTANCE 'FULLSCREEN))
          (COPY-PIXELS (SLOT-VALUE INSTANCE 'COPY-PIXELS))
          (Y-AXIS (SLOT-VALUE INSTANCE 'Y-AXIS))
          (TITLE
           (IF (GETF INITARGS :TITLE)
               (SLOT-VALUE INSTANCE 'TITLE)
               "weir"))
          (WIDTH
           (IF (GETF INITARGS :WIDTH)
               (SLOT-VALUE INSTANCE 'WIDTH)
               1000))
          (A (OR (GETF INITARGS :A) (MAKE-HASH-TABLE :TEST #'EQUAL)))
          (B (OR (GETF INITARGS :B) (CHANGE-STATE A)))
          (HEIGHT
           (IF (GETF INITARGS :HEIGHT)
               (SLOT-VALUE INSTANCE 'HEIGHT)
               1000)))
     (DECLARE
      (IGNORABLE TITLE WIDTH HEIGHT FULLSCREEN COPY-PIXELS Y-AXIS TITLE WIDTH A
       B HEIGHT))
     (SETF (SKETCH-TITLE INSTANCE) TITLE
           (SKETCH-WIDTH INSTANCE) WIDTH
           (SKETCH-HEIGHT INSTANCE) HEIGHT
           (SKETCH-FULLSCREEN INSTANCE) FULLSCREEN
           (SKETCH-COPY-PIXELS INSTANCE) COPY-PIXELS
           (SKETCH-Y-AXIS INSTANCE) Y-AXIS)
     ; replace this ------------------------------------
     ;(SETF (SLOT-VALUE INSTANCE 'A) (MAKE-HASH-TABLE :TEST #'EQUAL)
     ;      (SLOT-VALUE INSTANCE 'B) (CHANGE-STATE A))
     ; with this? ----------------------------------------
     (SETF (SLOT-VALUE INSTANCE 'A) A
           (SLOT-VALUE INSTANCE 'B) B))
   (SETF (ENV-Y-AXIS-SGN (SLOT-VALUE INSTANCE '%ENV))
           (IF (EQ (SLOT-VALUE INSTANCE 'Y-AXIS) :DOWN)
               1
               -1)))
 (DEFMETHOD DRAW ((INSTANCE NIER) &KEY &ALLOW-OTHER-KEYS)
   (WITH-ACCESSORS ((TITLE SKETCH-TITLE) (WIDTH SKETCH-WIDTH)
                    (HEIGHT SKETCH-HEIGHT) (FULLSCREEN SKETCH-FULLSCREEN)
                    (COPY-PIXELS SKETCH-COPY-PIXELS) (Y-AXIS SKETCH-Y-AXIS))
       INSTANCE
     (WITH-SLOTS (TITLE WIDTH A B HEIGHT)
         INSTANCE
       (PRINT (LIST :A (GETHASH 'V A) :B (GETHASH 'V B)))
       (BACKGROUND (GRAY 0.1)))))
 (MAKE-INSTANCES-OBSOLETE 'NIER)
 (FIND-CLASS 'NIER))

A naive fix that addresses this could look something like this: https://github.com/inconvergent/sketch/commit/8ceda098fe37323b83438ca0b097dffd243aeed6 But again, I'm unsure of what else i might be breaking here.

Best, A

Inc0n commented 4 years ago

Yes this is still the problem til this day, I think the mentioned commit would resolve the problem, if not at least it's at the right direction.

Inc0n commented 4 years ago

Also, may i suggest a much more elegant fix, in defclass, we could use initform for each of the slots. instead of manually determine to initialise the slots with set default value or the pass in args in make-instances

death commented 4 years ago

Interestingly, I had a patch that's similar to inconvergent's but using the car of binding. See https://gist.github.com/death/73f001b54d23c7ba83f03c9ade7159ae

Inc0n commented 4 years ago

No yh it seems with cdr it produces

(SETF (SLOT-VALUE SKETCH::INSTANCE 'CELL-SIZE) (10)
           (SLOT-VALUE SKETCH::INSTANCE 'POS-VEC)
             ((MAKE-VEC2 :X (/ WIDTH 2) :Y (/ HEIGHT 2)))
           (SLOT-VALUE SKETCH::INSTANCE 'RUNNING) (T))

when we defsketch with (cell-size pos-vec running) these slots, and car actually uses the let binding variables and avoids re-evaluation.

But still, I think using initform removes much of these unneeded code.

inconvergent commented 2 years ago

the double evaluation seems to be fixed when using new defsketch macro