search

pd-rs / crankstart

A barely functional, wildly incomplete and basically undocumented Rust crate whose aim is to let you write Games for the Playdate handheld gaming system in Rust.
MIT License
235 stars 24 forks source link

Menu item API #79

Closed tehsmeely closed 10 months ago

tehsmeely commented 10 months ago

Exposes an api in System for adding (etc) menu items.

Slightly painful since it involves passing callbacks into C but it's handled by double-boxing dynamic Fn() trait objects.

There's some degree of code around handling the possible kinds of MenuItem values (as MenuItemKind). Potentially one could go a step further so users do not have the directly deal with the index value and can get its typed (bool or string) accordingly.

Adds a new example that adds one of each menu item

boozook commented 10 months ago

@tehsmeely, thank you for your attempt to make the better implementation! 👍

Just one question. Have you seen another attempt? Could you please compare yours with that?

One of the advantages that I see here is that there are no generics, which makes it easier to use. However, the UserData type is lost here, which is a disadvantage.

tehsmeely commented 10 months ago

@tehsmeely, thank you for your attempt to make the better implementation! 👍

Just one question. Have you seen another attempt? Could you please compare yours with that?

One of the advantages that I see here is that there are no generics, which makes it easier to use. However, the UserData type is lost here, which is a disadvantage.

As far as I'm aware there's little to no need for userdata handling here as that's primarily the C handling for generic callback functions. Since we're using rust closures in this implementation, we can include any relevant userdata into that thunk and continue from there.

tehsmeely commented 10 months ago

I just pushed a fix for a soundness issue where one could drop a MenuItem and result in the item still being visible in the menu, at which point pressing would result in reading of unmanaged memory.

Worked around this by forcing the "removeMenuItem" call in the drop impl for MenuItem instead. The remove_menu_item method still exists but it's just an explicit drop.

removeAllMenuItems is gone because it doesn't make sense as it would leave MenuItems with dangling pointers. This is not a problem from the API since it's easier to just drop MenuItems