Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initialising interdependent variables inside the defsketch macro #33

Closed
inconvergent opened this issue Nov 25, 2019 · 9 comments
Closed

Comments

@inconvergent
Copy link

inconvergent commented Nov 25, 2019

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 inconvergent changed the title passing variables to the sketch instance initialising interdependent variables inside the defsketch macro Nov 25, 2019
@inconvergent
Copy link
Author

inconvergent commented Nov 25, 2019

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
Copy link
Author

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
Copy link
Owner

vydd commented Nov 25, 2019

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

@inconvergent
Copy link
Author

inconvergent commented Nov 25, 2019

@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: inconvergent@8ceda09
But again, I'm unsure of what else i might be breaking here.

Best, A

@Inc0n
Copy link

Inc0n commented May 12, 2020

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
Copy link

Inc0n commented May 12, 2020

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
Copy link

death commented May 12, 2020

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
Copy link

Inc0n commented May 13, 2020

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants