search

samdze / playdate-nim

Nim bindings with extra features for the Playdate SDK
66 stars 3 forks source link

ADD SoundSamplePlayer rate #35

Closed ninovanhooff closed 11 months ago

ninovanhooff commented 11 months ago

Needed this so I added this to my own fork.

Tested to work.

Created this PR to check whether I didn't miss anything

samdze commented 11 months ago

Looks good. Only one thing: we should choose whether to use the setRate and getRate methods convention or the property-style getter and setter. I think in this case the getter and setter approach (rate and rate=) is more fitting.

ninovanhooff commented 11 months ago

Agree the property style is best. Was wondering why you did both for some other methods?

On Fri, 5 Jan 2024, 23:00 Samuele Zolfanelli, @.***> wrote:

Looks good. Only one thing: we should choose whether to use the setRate and getRate methods convention or the property-style getter and setter. I think in this case the getter and setter approach (rate and rate=) is more fitting.

— Reply to this email directly, view it on GitHub https://github.com/samdze/playdate-nim/pull/35#issuecomment-1879304641, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACO7DH7BLCFADYDB4D3IEU3YNBZWFAVCNFSM6AAAAABBO7ATYGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNZZGMYDINRUGE . You are receiving this because you authored the thread.Message ID: @.***>

samdze commented 11 months ago

Mhh, looking at volume for example, I remember adding setVolume too because it's a setter that has two parameters. There may be others that I added by mistake, not sure.

ninovanhooff commented 11 months ago

Allright, removed the getter and setter, kept property style accessors

samdze commented 11 months ago

Thank you!