love
love copied to clipboard
love2d
Renaming Source/Video:seek/tell
Original report by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
There are few issues with seek and tell I think.
They set and get a property of the Source, rather than do an "action" on it, so I'd expect a "set/get" prefix. Also, even though the names are good for what they are, they have an obvious setter/getter relationship, and the name "tell" isn't super obvious, at least to me.
How about set/getLocation? I know it's kinda similar to set/getPosition though, but here's hoping that wouldn't be too confusing.
Original comment by Minh Ngo (Bitbucket: mngo, GitHub: mngo).
I'm ok with set/getPosition
EDIT: Guess that's already taken.
Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).
These are well-known names, consistent with Files, and used in just about every other piece of software. (See fseek, ftell.) I'm not sure this is more obvious, and it feels a bit like being different for the sake of being different.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
All else being equal it would nice if Sources and Files had the same names, but I think the consistency between Sources and Files isn't as important as the consistency between Sources and LÖVE's naming convention for functions which set/get state.
I took a look at some other library-framework-engines to see what names they were using (accuracy not guaranteed).
- SFML:
get/setPlayingOffset - Allegro:
al_get/set_voice_position - Slick2D:
get/setPosition - FlashPunk:
positionproperty - Polycode:
get/setOffset(samples),getPlaybackTime(time),seekTo(time) - openFrameworks:
get/setPositionMS,get/setPosition(normalised) - jMonkeyEngine:
get/setTimeOffset(but I think this might be to set the sample that playback starts from) - Moai:
get/setPosition - Unity:
timeandtimeSamplesvariables
I'm now thinking that get/setPlaybackPosition may be best names for LÖVE.
Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).
That is not something I would like, I mean, I use one file with many sounds and use a combination of source:tell, source:seek, and love.update(dt) to see wich sound is playing.
Writing tell/seek is easy and not confusing at all, even if they don't match the other functions.
Also they are short, just 3 or 4 letters long, and you want to change it for a function with a name like get/setPlaybackPosition that has almost 20 letters... That definetly doesn't match love standards and I wouldn't write a function that long
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
Writing tell/seek is easy and not confusing at all, even if they don't match the other functions.
If audio recording (#6) is added to LÖVE, what should a function which returns the current position/offset of the recording in samples or seconds be called?
Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).
I guess that recording wont be handled with sources, but instead return a source that you can save or play, so I guess that the API for that would be external to the source API.
Was that what you mean? I don't really see a relationship with that sorry
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
It would return a SoundData.
I mentioned it because it's functionality that achieves the exact same thing as Source:tell does right now, but for recording instead of playback. I don't really see any option other than naming it to something like love.audio.getRecordPosition or something similar, and renaming Source:tell and Source:seek to be consistent.
Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).
I'm guessing but you could use something like source:length to get the total length of the audio source. It could also be used for other things too
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
To clarify, an audio recording API will probably not return a Source. It will likely have a way to get all currently recorded audio as a SoundData, but it should also have a function for telling you how long you've been recording for (or how much time or samples will be in the SoundData if you wanted it now), hence love.audio.getRecordPosition or similar.
Basically something like this: http://docs.unity3d.com/Documentation/ScriptReference/Microphone.GetPosition.html
Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).
Sorry I didn't wrote that right. What I mean is that you are getting the whole length of the data, it's quite different to seek or tell, so a source:length would match that function in the recorder.
love.audio.recordedLength() would match source:length() and they do the same thing. But not the same thing as source:tell() (Althought source:tell() would return the same as source:length() after the source finished playing, and that would be useful to know if the source has already finished)
Also I don't think that the recorder should be part of the audio module since they do different things but whatever
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
What I mean is that you are getting the whole length of the data
But you aren't, you're getting the duration you've been recording for. The API will likely have a fixed maximum recording duration you set when you start recording, which would be the whole length (or use SoundData:getDuration).
Also I don't think that the recorder should be part of the audio module since they do different things but whatever
They don't. love.audio only deals with audio output right now because audio input hasn't been added to LÖVE at all yet, but it's all under the umbrella of interacting with audio hardware on the system (and the underlying libraries that LÖVE uses - OpenAL in this case - works that way too.)
Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).
I don't like that implementation, I should be able to record indefinetly until I say stop, And length would return the length of the SoundData at that time or if used with a preexisting source, the whole length of the source
If i want to set the duration of the recording then I would do:
if love.recorder.length() > duration then
sourcedata = love.recorder.stop()
end
Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).
Side note: Source:length would do the same as Source:getDuration but is shorter and can be easily understanded (getDuration can be easily understanded too).
But this doesn't matter, I just want to point out that length/getDuration and tell do different things
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
I don't like that implementation, I should be able to record indefinetly until I say stop
There are some technical and performance reasons why the implementation I described is the most likely one.
If i want to set the duration of the recording then I would do:
In the likely implementation I'm describing, you can still stop the recording early like that.
Regardless, that's very off-topic for the actual issue - and the code you posted still gives the duration you've been recording for, just like Source:tell gives the duration the source has been playing for.
Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).
The duration of the recorder is the same as the duration of the whole SourceData that you have until that time, If you stopped recording just after you got the duration of the recording then the duration of the recording would be equal to the duration of the whole SourceData, and that differs from what tell does.
Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).
This are the functions I would love to have:
Source:seek()
Source:tell()
Source:length()
love.recorder.setDuration()
love.recorder.start()
love.recorder.getLength()
love.recorder.stop()
Original comment by Sasha Szpakowski (Bitbucket: slime73, GitHub: slime73).
The duration of the recorder is the same as the duration of the whole
SourceDatathat you have until that time, If you stopped recording just after you got the duration of the recording then the duration of the recording would be equal to the duration of the wholeSourceData, and that differs from whattelldoes.
There's no such thing as a SourceData, but disregarding that, you can flip it around and say the same thing about Sources (except they go in the other direction, from data into hardware instead of from hardware into data):
The value returned by Source:tell is the same as the duration of the whole buffer that you have until that time. If you stopped playback just after you call Source:tell, then the duration of the sound that played back would be equal to the duration of what was used to play the audio.
As an aside, Source:getDuration doesn't exist - mostly because (IIRC) it's not really possible to be completely accurate with it for streaming Sources.
Original comment by Pablo Mayobre (Bitbucket: PabloMayobre, GitHub: PabloMayobre).
Good points, but I think that seek and tell should stay like that, even if they are not consistant with the recorder API. Thanks for the explanation It made lots of sense
Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).
Since most of this discussion has focused on recording, let me deal with the other frameworks name instead:
- offset: Yes, it's an offset, it wouldn't be the name I'd be looking for, though.
- position: We already have source positions, and they have nothing to do with time. In case you're thinking of renaming get/setPosition, they actually describe a physical position.
- time: Possible, but getTime and setTime just feel awkward.
- seek/tell: Matches up with Files, seeking is something found in media players everywhere, tell, well, not so much, but is usually seen as the opposite.
I'm not saying seek/tell are perfect, but in my opinion they do qualify for the least bad.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
@bartbes I agree with your judgements of those names, hence why my last suggestion was get/setPlaybackPosition. :)
Original comment by Bart van Strien (Bitbucket: bartbes, GitHub: bartbes).
Of all these suggestions I like playback position the most, but then you'd probably have to disambiguate get/setPosition too.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
@slime73 I know. :( But, it's only one character more than get/setBackgroundColor. :P
I think the verbosity makes it more obvious what it does though: Source:tell vs. Source:getPlaybackPosition... I think with the latter it's easier to tell (tee hee) what's going on.
And plus, other than seek/tell in Files, I think this may be the only place in the API where clear getter/setter functions aren't named get/setState.
@bartbes Maybe... Would the problem be that someone could mistake set/getPosition for methods which change the playback position? There's nothing stopping this misunderstanding currently, but I guess if that person had read about set/getPlaybackPosition they might be more prone to misremember. Disambiguation doesn't strike me as a huge concern, but they are similar names.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
I think I've found a good name! Maybe.
Source:set/getPlaybackProgress
(Also Video:set/getPlaybackProgress too now.)
Just to reiterate the points I still think are valid about this:
- The naming of
seek/tellfor Sources and Videos is conspicuously inconsistent with other functions in the API. -
seekwould be a fine name if it wasn't for this inconsistency,tellless so in my opinion. - I don't think the verbosity of
set/getPlaybackProgressis a problem considering other function names in the API and how often these functions would be used.
Original comment by Mi 28 (Bitbucket: rcoaxil, ).
As far as data string navigation goes, seek/tell are the industry standard and are used in virutally every single existing framework and library that does this. Maybe there are better names (which have not been presented this far, none of them are good replacement), but these were around since DOS and are extremely ubiquitous, so even if you found a good replacement, changing them for any reason whould achieve nothing but confuse people.
It's as if you renamed shaders to "pixel effects". That will just accomplish nothing.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
To be clear, I'm not suggesting changing the seek/tell data string navigation function names (i.e. the File object methods), only for Sources and Videos.
Original comment by Gabe Stilez (Bitbucket: z0r8, ).
I'm against that change for the simple reason that it's clear. If it's not, the wiki helps you out.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
@z0r8, I agree that seek is clear, I don't know if tell is but it is analogous to the File method.
Just for the fun of discussion, I'll share some ideas I have on LOVE's naming which are relevant to this particular issue, all just my current opinion of course:
-
If the name is descriptive enough there may be less reliance on docs. I think it's useful if a function's name reads similarly to its description. (I could even imagine alternative docs where descriptions aren't always needed if the function is simple in purpose and its name is clear enough. Maybe!)
-
All else being equal, I prefer shorter names because they are easier to read and write and are cuter.
-
With consistency, in this case setters/getters prefixed by "set/get" (although LOVE also has boolean-returning getters prefixed "is" and "has"), you can get benefits which may be unintended. For example, see this section of the SLAM library, and how it makes use of how the functions share prefixes. Also see this API reference that I made, and how the function navigation is simplified by the shared prefixes.
-
Also, when setters and getters differ only by prefix you only have to remember one "thing", i.e. the name of the state which it relates to ("playback progress" in this case). If you ever see
setPlaybackProgressin code or otherwise remember it, you can assume thatgetPlaybackProgressexists too. -
I think that consistency is a sign of quality. Inconsistency is kind of like a spelling mistake, it may still be obvious what is meant, but it's not confidence inspiring.
Original comment by hahawoo (Bitbucket: hahawoo, GitHub: hahawoo).
Now I think I prefer get/setPlaybackPosition, and renaming the listener functions to disambiguate get/setPosition and also because I think they might be better names (maybe), i.e. get/setListenerPosition, get/setListenerOrientation and get/setListenerVelocity.
The reason I think they might be better is that the playback functions (love.audio.play etc.) are named the same as Source methods and operate on Sources, whereas the listener functions are also named the same as Source methods but operate on the listener, i.e. there is no indication in their naming of what they operate on.
(Not that the playback functions would make sense for the listener of course, but both Sources and the listener have positional state.)
I would also suggest that the names are more self-descriptive, i.e. setListenerPosition "sets the listener position".
I don't think get/setDistanceModel and get/setDopplerScale would need to be changed because I think that these are about the relationship between Sources and the listener, and neither would get/setVolume because this is about the relationship between LÖVE's audio world and the computer's audio system, to the best of my understanding.