Implement scene parameters

[mjkalyan]

Addresses: #40

Things to note:

1. I have yet to make this code not ugly. It's at a working state as of b5d699ae5778d510b647645406a9d56785e9d07b, so I wanted to open it up to review (try to avert your eyes when you get close to make-scene-pro though).
2. This also fixes an issue with re-computing identical forms in make-scene/make-scene-pro, as mentioned in the commit message of 756f318871d1445900b4f0bfd7162cdccc5a4cfa
3. I took an approach with billboards as part of a fix to (2.) which changes make-billboard to yield its camera argument when it's a future. This is totally fine but it means that whenever we have a case like billboards where the object could refer to a future, we have to add the "check and yield" to its make-* function.

The main thing I want to know is whether this approach is how you'd like to continue, now that you can see it in action.

[mjkalyan]

By "see it in action" I'm referring to the billboard example (https://github.com/defun-games/claylib/pull/46/commits/7256e43ee338ace87d4efc960dee9d51e8347670).

The object/asset/param bindings occur serially, in the order given to make-scene-pro.

[shelvick]

The overall approach is good, I'm in favor of continuing that. Does it work with multiples of the same declaration type in make-scene-pro, e.g. you have :parameters then :assets then :parameters again?

2. This also fixes an issue with re-computing identical forms in make-scene/make-scene-pro, as mentioned in the commit message of 756f318

I didn't completely follow your explanation on this. The original problem was... if you declared cam1 and cam2 as parameters, then created a billboard object referencing cam2 it would use cam1 instead?

[mjkalyan]

Does it work with multiples of the same declaration type

That would break it with the current approach. Should I support that? If not I'll at least try handle it gracefully.

The original problem was... if you declared cam1 and cam2 as parameters, then created a billboard object referencing cam2 it would use cam1 instead?

Not quite. It's easier to explain in your original code for make-scene:

...
(let* (,@assets ,@objects)                                              
  (declare (ignorable ,@(mapcar #'car (append assets objects))))        
  (progn                                                                
    ,@(loop for (binding val) in assets                                 
            collect `(setf (gethash ',binding (assets ,scene)) ,val))   
    ,@(loop for (binding val) in objects                                
            collect `(setf (gethash ',binding (objects ,scene)) ,val))))
...

At the top of this snippet we bind the assets and objects which will look something like (let* ((my-asset (some-form))) ...). Here, (some-form) is executed and bound to my-asset. The problem is those two loops go back and look at the assets and objects lists and extract the un-evaluated forms again (in val). So we end up executing (some-form) once in the initial binding of my-asset and again when we store the binding and value in the hashtable. This creates problems when we create an object (say, a billboard) which needs to reference a particular other object (a camera) and not an otherwise identical object. It wouldn't be a noticeable issue if you created a rectangle and referred to its width for the radius a circle because any rectangle with the same width would produce the same circle.

The simple fix is asking for the already-bound value of binding - so I changed ,val to ,binding in make-scene and make-scene-pro.

[shelvick]

That would break it with the current approach. Should I support that?

Ideally yes. It won't be long before we find, or somebody asks about, that exact situation. May as well figure out how to deal with it now (elegantly or not).

The simple fix is asking for the already-bound value of binding - so I changed ,val to ,binding in make-scene and make-scene-pro.

Oooh, good catch. That could've caused all sorts of subtle bugs in more advanced scenes.

[shelvick]

I think this is obsolete now. See d52e60e and 4f8d227.