search

defun-games / claylib

A Common Lisp 2D/3D game toolkit built on top of Raylib 4.5.
zlib License
71 stars 4 forks source link

Some enhancements, and review required #3

Closed mjkalyan closed 2 years ago

mjkalyan commented 2 years ago

See the "WIP: textures example 1" commit message for more detail about my texture issues.

The other changes should be good to go.

shelvick commented 2 years ago

BTW, the design intent is for the user to load assets once and then use those assets in the creation of objects. What's confusing in this case is that "texture" is both an asset and an object. Admittedly we don't have a very clear distinction there yet.

mjkalyan commented 2 years ago

I'd like to avoid pushing known-broken code to main

Me too, I was thinking I'd just fix up the issues before merging but it's smarter to make separate PRs. I'll work some git magic and resubmit.

sorry to send you on a wild goose chase

Don't be! I am more familiar with the code base now.

autowrap's wrapper objects are pointers (basically) [...] mind making that change?

Ah, gotcha, will do!

I had to use an absolute path to get the example to work

I had noted this mentally but forgot to switch to UIOP, will fix that too.

mjkalyan commented 2 years ago

Oh, and should we remove the ptr slot from game-asset altogether?

mjkalyan commented 2 years ago

Removed the WIP commit so this PR can be merged.

shelvick commented 2 years ago

Oh, and should we remove the ptr slot from game-asset altogether?

Double-check me here but I don't think it's being used in anything other than loading, and if not, then it can be removed.

Also, remember that we're also not setting c-struct to (autowrap:ptr foo) but just foo. (autowrap:ptr) just gets the raw pointer which AFAIK we will not need to use.

mjkalyan commented 2 years ago

ptr is used in some probably stale free methods (in claylib.lisp) and the macro definition for definitializer, which at first glance looks like it's specifying a finalizer to free the ptr.

Judging by the docstring, autowrap:free should work on the wrapper object (the c-struct) directly. If I'm right I should be able to replace all the freeing of ptr mentioned above with freeing of the c-struct and safely remove the ptr slot.

shelvick commented 2 years ago

ptr is used in some probably stale free methods (in claylib.lisp) and the macro definition for definitializer, which at first glance looks like it's specifying a finalizer to free the ptr.

Judging by the docstring, autowrap:free should work on the wrapper object (the c-struct) directly. If I'm right I should be able to replace all the freeing of ptr mentioned above with freeing of the c-struct and safely remove the ptr slot.

Actually, when it comes to finalizers, we do need to use ptr. When you first add the finalizer it creates a closure, but if the wrapper is included in that closure that means there's always a reference to it so it will never be freed. There's a section on this in the autowrap docs that is worth a read.