Feature/sprite #45
Feature/sprite #45
Conversation
SHiLLySiT
left a comment
SHiLLySiT
left a comment
There was a problem hiding this comment.
Heya, thank you for your contribution, and sorry it took my so long to review this!
I did a initial review by testing with the Asheteroids sample from the Playdate SDK. Although it wasn't until the end that I realized it wasn't going to run anyways since Playbit doesn't have the geom API yet 🤦
I'll circle back at a later date to test with a different sample, but figured I'd leave the review comments I have now for you to work on.
Thanks for helping improve Playbit!
|
|
||
| -- Unclear if the drawing of the images of a sprite needs to be called here | ||
| -- or would happend later through the love.graphics.draw(playdate.graphics._canvas, ... ) | ||
| playdate.graphics.sprite.drawAll() |
There was a problem hiding this comment.
These sprite methods shouldn't be called here as not all games will need the sprite system.
I'd recommend sticking these inside of sprite.update() so that they are invoked when the sprite system is in use.
| local hasSpr = sprite == nil and 'no sprite' or 'has sprite' | ||
|
|
||
| print("sprite " .. hasSpr) | ||
| -- printTable(sprite) | ||
|
|
||
| local hasImg = imageOrTilemap == nil and 'no image' or 'has image' | ||
|
|
||
|
|
||
| print("imageOrTilemap " .. hasImg) | ||
| -- printTable(imageOrTilemap) |
There was a problem hiding this comment.
Please remove this block of debugging code.
| local hasSpr = sprite == nil and 'no sprite' or 'has sprite' | |
| print("sprite " .. hasSpr) | |
| -- printTable(sprite) | |
| local hasImg = imageOrTilemap == nil and 'no image' or 'has image' | |
| print("imageOrTilemap " .. hasImg) | |
| -- printTable(imageOrTilemap) | |
| local hasSpr = sprite == nil and 'no sprite' or 'has sprite' | |
| print("sprite " .. hasSpr) | |
| -- printTable(sprite) | |
| local hasImg = imageOrTilemap == nil and 'no image' or 'has image' | |
| print("imageOrTilemap " .. hasImg) | |
| -- printTable(imageOrTilemap) |
There was a problem hiding this comment.
The playdate SDK file name uses the plural form sprites. If we don't follow this, normal imports of the sprite system one work i.e. import "CoreLibs/sprites"
| @@ -0,0 +1,468 @@ | |||
| local bit = require("bit") -- LuaJIT's bitwise operations | |||
|
|
|||
| local module = {} | |||
There was a problem hiding this comment.
Sprites should have include support for object.
Noting that I don't think I had objects implemented when you opened this PR.
|
|
||
| local allSprites = {} | ||
|
|
||
| function module.new(imageOrTilemap) |
There was a problem hiding this comment.
The native sprite is a bit odd and supports both sprite:new() and sprite.new(). Yours only supports the former.
| end | ||
|
|
||
|
|
||
| function module.updateAll() |
There was a problem hiding this comment.
Was this intended to be playdate.graphics.sprite.update()?
2 participants
Iusse #22
Added basisc sprite implementation for rendering and collision, not all functionalities are implemented yet.
I'm right now unclear if it better would be integrated into the playdate.graphics or if it makes sense to call in the header.lua.
@SHiLLySiT Please review