Add get_sprite_count, remove_sprite, get_userdata and set_userdata

[lyonbeckers]

I wound up needing access to more of the sprite API for my game. I found this worked for me, but I'm not a wiz with smart pointers so I may have done some questionable things.

I might want to mark get_userdata as unsafe, just because it is dependent on T matching what was passed to set_userdata which can get dicey

[parasyte]

Some thoughts on designing a safe interface for this.

[parasyte]

I don't think you can do this safely, since the type is not Rc<Box<dyn Userdata>>, it's Rc<Box<T>>.

The right way to solve the problem with the T generic is moving it into a parameter of SpriteInner:

pub struct SpriteInner<T> {
    pub raw_sprite: *mut crankstart_sys::LCDSprite,
    playdate_sprite: *const playdate_sprite,
    image: Option<Bitmap>,
    _userdata: PhantomData<Rc<Box<T>>>,
}

But this may not be easy to consolidate with the Sprite type that owns it.

(As a bonus, making the SpriteInner hold an Option<Rc<Box<T>> would eliminate the need to add this extra drop logic.)

I don't really know of any other way to do this. Type erasure can't work because the get/set API takes Rc. It would have to take full ownership (not just shared ownership) of the type to safely erase the type.

But maybe that is good enough? The caller can always get a userdata pointer after moving it to the sprite. It just depends on if this is acceptable for your use case, e.g. you don't require the Rc to exist before the userdata is set on the sprite. An example where this ownership model breaks down is that sprites will only be able to share userdata with double-indirection.

[lyonbeckers]

I don't think you can do this safely, since the type is not Rc<Box<dyn Userdata>>, it's Rc<Box<T>>.

Yeah I wasn't 100% certain about this one, because I figured it would be aligned to the box size, not specifically T. If I'm wrong about that, if I'm understanding correctly, I think it could become a problem if replacing the userdata with something else that has the trait but is not the same T. On that note though, I'm realizing that I should probably check the userdata ptr in set_userdata and drop the reference before setting it again if it's not null (same cleanup logic as in the drop).

Yeah this is a bit of a weird one because for my use case, the userdata exists as a component in an ECS model, the sprite just needs to reference it in the draw call. I could avoid using userdata completely by querying my world in the draw call based on the sprite's tag, but this saves a bit of work there since I can set the userdata when setting up the sprite itself.

Similarly in most cases, I'd expect that most people would find safer ways of handling something that would otherwise be userdata through the Game state object itself, bypassing Playdate's C API entirely.

I also considered SpriteInner<T>, but I figured it might be a pain to have to specify it where the majority of cases might call for no userdata at all. Sprite would have to be Sprite<T>, and you'd need to call SpriteManager::new_sprite::<T>() (Possibly just using () for when it's not needed, but still). With adding the Option<Rc<Box<T>> like you said you could just bypass the C API too and just return the owned data. Maybe that could be more universally useful to others? It'd be a breaking change for sure though

[parasyte]

There's a thing called object safety which requires that constructing a trait object must always be possible for every T: Trait. And because that isn't the case here (there is no trait bound on T), I think this implementation violates at least object safety.

The bigger problem, though, is that nothing implements the UserData trait. So I don't see how the compiler can correctly drop the type behind the Rc.

I actually do like the idea of type erasing the userdata safely. I think it can be done with core::any::Any to downcast out of get_userdata. But if it's behind a smart pointer and Any, I don't really know what benefit you are getting out of the FFI calls, sending a raw pointer to the SDK. You can just do the userdata in pure safe Rust at this point. And you don't need the type parameter on Sprite for it, either.

Similarly in most cases, I'd expect that most people would find safer ways of handling something that would otherwise be userdata through the Game state object itself, bypassing Playdate's C API entirely.

Yep! Basically this! 😂 I'm using an ECS, too. I would expect the Sprite to be a component as well. The problem might be that it's !Send and !Sync. Which ... basically prevents adding it to any ECS that exists, at least as a component. What I ended up doing, even though I am not using Sprite, is putting my Playdate resources into a shipyard::Unique, which can be !Send and !Sync. And they can just be put into a BTreeMap with a &'static str, String, or u32 key... Some type that can trivially be put into a component.

[lyonbeckers]

The problem might be that it's !Send and !Sync

I wound up forking edict which is supposed to be no_std, but modifying it a lot 😂. I've basically just been stripping out any Send + Sync trait bounds where I can.

I'll close this because I'm sold on the possibility of doing this safely. I'm going to look into a safe implementation that uses Any, because like you said, we could avoid the type parameter which prevents any breaking changes, and I think it's just as good to sort of mimic the functionality of the C API without having to directly make the ffi call

[lyonbeckers]

Closing this to work on a safe approach to userdata