love2d / love

LÖVE is an awesome 2D game framework for Lua.
https://love2d.org
Other
5.04k stars 399 forks source link

Removing duplicate love.audio functions #488

Closed slime73 closed 8 years ago

slime73 commented 12 years ago

Original report by Anonymous.


love.audio.play(source) is a duplicate of source:play(), and so on.

I would assume that users who know about the Source methods would want to use source:play() etc., due to the concise syntax and consistency with other Source operations.

For users who don't know about the Source methods, I think that the duplicated functions in love.audio could be misleading. While it reduces the learning curve for some basic Source operations due to the familiar syntax, other common Source operations (such as looping) would require the user to learn about Source methods anyway, and if the user has controlled sources using the love.audio functions before, looking for Source methods might not be obvious.

If these functions were removed, I'd also suggest to consider appending "All" to the name of the appropriate love.audio functions (such as love.audio.pause becoming love.audio.pauseAll) to better describe their functionality.

slime73 commented 12 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


Edit: And I think this would actually be consistent with the rest of LÖVE, as (I think) any function which involves only one object and does not affect the global state is a method, and everything else is a function, and love.graphics.draw / drawq is an exception. (And love.graphics.draw / drawq being a function makes it consistent with everything else that draws to the screen.)

For example, File:write() is a thing, whereas love.filesystem.write(File) is not.

slime73 commented 12 years ago

Original comment by Enrique Garcia Cota (Bitbucket: kikito, GitHub: kikito).


I do think that the methods are redundant and should be removed. After all, Image doesn't have a :draw method (right?)

slime73 commented 11 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


Yeah, actually maybe it would be better to remove the methods instead, hehe.

slime73 commented 11 years ago

Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).


For consistency's sake, I think the methods should be removed and leave the module functions (or add them where they don't exist, a la love.filesystem.write(File) ) in place.

slime73 commented 11 years ago

Original comment by Alex Szpakowski (Bitbucket: slime73, GitHub: slime73).


love.graphics.draw is affected by outside state (current color, blend mode, etc. – and it also affects the screen state for further drawing), so in that way it makes sense to have Source:play but not Drawable:draw.

What happens when Source:play is called will only change based on the object in question, but the same is not true for love.graphics.draw.

slime73 commented 11 years ago

Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).


That sort of distinction is quite confusing, imo. Having a completely consistent API would lower the barrier of entry. I know that the first time I took a look at the audio API, I became quite confused with what I could do, and where, and what was an alias to which, etc.

Having one way of doing things would probably be best.

slime73 commented 11 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


Isn't playing a Source affected by outside state too? For example, the master volume and the listener position. And a playing sound also adds to the sound scape which affects the perception of other playing sounds.

slime73 commented 11 years ago

Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).


Well, the specific distinction made means that when you draw a drawable, not always the same thing happens (see canvases), when you play a source, a source still starts playing, but yes, there's some module state involved.

slime73 commented 11 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


So you mean, if you draw a canvas, it might have changed, or a ParticleSystem might have been updated or an Image might have been refreshed? But can't Sources also change in a sense, by their individual volume and position and velocity?

So, just to restate the issue because what I wrote originally was kind of weird:

love.audio.play(source) is the same as source:play(), etc. It's a different way of doing the exact same thing, and I don't think there is anything else in the API which conceptually does the exact same thing as something else. I don't think it's such good design because it's not much fun to learn ("oh, this is... the same thing...") or to teach ("and this thing does the same thing as that thing") or to use ("which one do I use?!?! I can't deciiiide!") So! I think it would be better to choose one way to do it. Does anyone disagree on this point?

As for which way... I used to think methods... now I kind of think functions... maybe...

In favour of methods:

In favour of functions:

slime73 commented 11 years ago

Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).


No, I mean if you call Drawable:draw it might or might not end up on the screen, depending on whether a canvas is set. It might be red, it might be blue, if might be pink, anything can happen! If you call Source:play, the source starts playing. Now, for the master volume and listener position, those are basically affecting the listener, and not the source, it's kind of an awkward distinction, though.

slime73 commented 11 years ago

Original comment by Landon “Karai” Manning (Bitbucket: karai17, GitHub: karai17).


Is a distinction necessary? Would a note in the wiki be an acceptable way to tell developers that X function may be affected by Y state(s)?

slime73 commented 11 years ago

Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).


I think the caveats for Drawable:draw go too far, but that's been discussed elsewhere.

slime73 commented 11 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


Woooooah. I think I realised why it's not obvious whether to have functions or methods for these things: these functions are unique in that they affect the state of the object, and also the external state.

Drawing a ParticleSystem does not change the state of the ParticleSystem. Starting a ParticleSystem does not change anything other than the ParticleSystem. Playing a Source changes the state of the Source and the external state of what's being played through the speakers.

This... this may be obvious but I just thought of this just now.

slime73 commented 11 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


Okay... I think I may have realised the Ultimate Way of Playing Sources.

And that is...

Drum roll...

Methods.

Here is how I see it right now:

Whether the global state affects or is effected is irrelevant.

The ultimate distinction between methods and functions may be: methods change or return the object's state and/or properties.

love.graphics.draw, love.graphics.setCanvas and love.graphics.setFont don't affect the objects they're passed, the objects are used to do something else.

Image:getWidth and Font:getHeight return "properties" of the objects.

Canvas:renderTo and ImageData:setPixel change the object directly, I'm not sure if the term "state" still applies here but it's conceptually similar.

Font:getWrap... I guess returns the state with a function performed on it, like, to make a ridiculous example, Image:getWidthMultipliedBy(4) would.

Source controls absolutely affect the state of the objects, and therefore should be methods!

I think this is actually a pretty clean conceptual difference, maybe! And, in addition, the interface for controlling Sources with methods is nicer.

getSourceCount almost breaks the distinction, but maybe not really, because it's getting the state of the global state, rather than of a particular Source (and perhaps it might be possible to have more Sources in a "playing" state than are actually playing in the global state, if there are a lot of them playing at the same time). The functions which affect all Sources might actually break the distinction slightly, but maybe that could be handwaved, and I also have other crazy ideas related to that: #647

slime73 commented 11 years ago

Original comment by Some Guy (Bitbucket: [Somepotato NA](https://bitbucket.org/Somepotato NA), ).


Let's not turn Lua into C, I believe instead of removing them there should be more metafunctions added IMO.

slime73 commented 10 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


What I wasn't taking into consideration is that due to the threaded nature of audio, I don't think that sources could be started/stopped at exactly the same time using methods. (Right?)

For example, with this code:

#!lua

source1:stop()
source2:stop()

I assume there could be a small amount of time when source2 is still playing but source1 has stopped.

By using functions instead, perhaps changing the playback state of multiple sources may be able to done "atomically" (Not sure if I'm using that word correctly).

#!lua

love.audio.stop(source1, source2)
slime73 commented 10 years ago

Original comment by Alex Szpakowski (Bitbucket: slime73, GitHub: slime73).


perhaps changing the playback state of multiple sources may be able to done "atomically"

That would be possible to implement, yes, although it would require a bit of reworking of that aspect of love.audio's internals.

slime73 commented 9 years ago

Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).


#!lua

love.audio.stop(source1, source2)

Wont that end up being like this?

#!lua
source1:stop()
source2:stop()

I'm up for methods... love.audio should be used for the master things while love.sound and Sources should handle the sounds states

slime73 commented 9 years ago

Original comment by Alex Szpakowski (Bitbucket: slime73, GitHub: slime73).


I think the idea is that the first version would have a more atomic internal implementation (so LÖVE would tell OpenAL to stop both source1 and source2 at exactly the same time.)

i.e. it would use these:

The functions are also available as a vector variant, which guarantees synchronized operation on a set of sources.

void alSourcePlayv (ALsizei n, const ALuint * sNames);

void alSourcePausev (ALsizei n, cost ALuint *sNames);

void alSourceStopv (ALsizei n, const ALuint *sNames);

void alSourceRewindv (ALsizei n, const ALuint *sNames);

Although there would be a bit more to it than that with LÖVE's implementation, because streaming sources are treated a bit differently by LÖVE.

slime73 commented 8 years ago

Original comment by Alex Szpakowski (Bitbucket: slime73, GitHub: slime73).


love.audio.play, love.audio.pause, and love.audio.stop can now take a list of Sources that it will atomically do the operation on.

slime73 commented 8 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


Very cool. :)

I still think that Source:play/pause/stop could be removed now.

slime73 commented 8 years ago

Original comment by Gabe Stilez (Bitbucket: z0r8, ).


And i don't. :3 Seriously, what's the negative impact of having them as Object methods? Furthermore, since #723 happened, implementing stop as rewind+pause, and rewind as seek(0) to me would suggest a love.audio.seek(Source1, Source2, ...) to exist as well, since the function of it is analoguous to the other three. ("stop", play and pause, where the first already does this) Then again, i know that seeking to anywhere else but 0 is not guaranteed to work with multiple Sources, so... love.audio.rewind? after rewinding got killed out? Yeah.

slime73 commented 8 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


The negative impact of having them as object methods, in my opinion:

  1. The programmer has to make an arbitrary decision. Do I use source:play(), or love.audio.play(source)? I don't particularly like having to make arbitrary decisions.

  2. It's unprecedented API design, in the sense that I don't think any other two functions/methods do the exact same thing. (Or maybe you could say a complete superset or something, since love.audio.play does more than the play method.) There are shortcut functions that do things that could be done using other functions, but they're not used in the exact same way.

  3. It's little bit of additional complexity. More to learn, more to document, etc.

I think a positive impact is that it's quicker to type the object methods.

I agree that love.audio.seek is analogous to the other three.

I didn't know that seeking to anywhere but 0 isn't guaranteed to work with multiple Sources, interesting.

Do you know of any use cases for love.audio.rewind?

slime73 commented 8 years ago

Original comment by Gabe Stilez (Bitbucket: z0r8, ).


The same use-case would suffice for the use-cases of atomic play/pause/stop, and in slime's post, he did mention an atomic rewind method as well: "void alSourceRewindv (ALsizei n, const ALuint *sNames);" Seeking to 0 is now basically rewind that doesn't modify any playback state for a source (and stop is seek(0)+pause state), that's evident, but sources can be of differing lengths, so a vector variant of seeking to an arbitrary sample could try to seek to a position in a source that's after it's end; how would that be handled? wraparound? wouldn't be atomic. Still, now that love.audio's non-object methods have an extra use-case that the object methods don't have, so it's not that arbitrary. If one wants to manipulate one Source at a time, use object methods, else if you want to handle multiple at the same time, use love.audio's.

slime73 commented 8 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


Re: use case, I mean what "real world example" is there for rewinding multiple sources, or even for a single source?

For example, love.audio.stop could stop all of the sound at the end of a level, or love.audio.play could use multiple sources to create a sound effect composed of multiple sounds playing together. But I can't think of a reason for wanting to rewind sources, which is why I ask, maybe I'm missing something obvious. I'd suggest that if there isn't a use case, it wouldn't be a good thing to have.

Re: seeking multiple sources at the same time, I'm not certain this is a good idea, because again I can't really think of any use cases. Wraparound seeking doesn't seem too useful, I guess trying to seek beyond the end of a source should just stop the source?

Re: arbitrariness, that's a reasonable rule you mentioned (one source: use method, multiple sources: use function), but it's still an arbitrary rule. :P

There was once a function named love.graphics.triangle, and you passed it three sets of x and y, and it drew a triangle. Calling this function was exactly the same as same as calling love.graphics.polygon, except it could only draw triangles. Now, one could say "if a polygon has three points, call love.graphics.triangle, for more than three points call love.graphics.polygon". But I doubt anyone objected to love.graphics.triangle being removed, because it was a straight subset of love.graphics.polygon.

This is how I feel about the source methods, except the source methods have the benefit of having less characters to type.

slime73 commented 8 years ago

Original comment by Gabe Stilez (Bitbucket: z0r8, ).


I agree with the lg.polygon superset function making any subset function unnecessary.

In the case of the Source methods, in any paradigm that one would use (OO or ECS), it would be more "local" to call a Source's methods than call out to love.audio. To me, this sounds reasonable. You wouldn't do love.graphics.getImageDimensions(Image), would you?

For love.audio.rewind/seek use-cases, the question is "What would need a function that would make one or more Sources seek (atomically) to 0, and keep their current state?", and i can think of one. Not rewind, but seek-specific. If a game would have its bgms separated to some degree for whatever aesthetic reason (like the world being able to shift between two "states", light or dark, for example), and you'd want to use loop points on those tracks (that are the same, and the track lengths are the same), then love.audio.seek(looppoint, Source1, Source2) would work.

slime73 commented 8 years ago

Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).


Re: methods being more "local" than functions, do you mean that, for example you wouldn't have love.graphics.getImageDimensions(Image) because the dimensions are specific to that image object and don't have anything to do with anything "global"? I would agree with this. But I guess sources aren't quite that simple because of the whole playing sounds in a different thread thing, which makes the love.audio functions useful. If it wasn't for that, I would say that methods would make more sense than methods, because they're all about changing a specific objects state.

Re: use case, that is a really cool use case in theory. The issue is that you'll probably be using the seek function in love.update, right? But love.update is called way less than the sample rate of the music, so it doesn't really seem possible for the music to loop perfectly. If the API had something which could support loop points, that would be neat, but it doesn't seem possible using seek.

slime73 commented 8 years ago

Original comment by Gabe Stilez (Bitbucket: z0r8, ).


Bottom line is, since the user handles sources just like how they handle images, they do the bookkeeping, and the objects have their own methods, i'd want to keep using methods that affect only those sources in the objects themselves.

Two (both already tested) solutions for the latter, either handle audio in a separate thread that's not limited by vsync nor love.timer.sleep, or force vsync off and modify love.run; in any case, it's doable, but it's an user concern. Even then, one shouldn't worry about sub-milisecond precision when seeking; if current > loopend then seek(loopstart+(current-loopend)) or even just seek(loopstart).