Design Question

[Josh]

According to the rules of smart pointers, the source will be immediately deleted at the end of the if function.  Should the user have to keep every sound source in scope to prevent it from being collected (and the sound stopped?)

shared_ptr<Sound> sound = Sound::Load("blablabla.ogg");
if (sound)
{
	shared_ptr<Source> source = Source::Create();
	source->SetSound(sound);
	source->Play();
}

 

 

[Rick]

I always thought the design of us having to load a sound then make a source and attach the sound was kind of unnecessary from our point of view. Why can't I as the user of the LE API just make a "source" where I pass in the wav/ogg path like I do sound and let the API manage things for me? If a sound is needed then you create it and keep it in scope. Don't put that on the user. I see no point in putting that on the user.

[Josh]

Okay, so we could do this:

void SetupSound()
{
	shared_ptr<Source> source = Source::Create("blablabla.ogg");
	source->Play();
}

But my question remains.  Should the source be deleted when it goes out of scope?

[Rick]

I think it's reasonable for the user to have to keep the pointer around. Most people will be using classes and so this pointer would be a class member variable so it would exist as long as the class exists and classes generally exist longer than functions so it's reasonable. So yes, when it goes out of scope having it clean itself up then opsould be nice.

[Rick]

As a side note, if we are meant to use these kinds of pointers all over the place you could typedef them with a common naming convention to market less typing for us. I can see that getting tedious. Also what are you doing for Lua? Automatically making all of these shared pointers? If shared pointers is the default then heck you could rename the classes themselves with and underscore and use the now class name as the typedef itself so nothing really changes on our side from today.

 

If you wanted you could probably even extend shared pointer to have addref and release functions that do nothing so current code would work. Making it backward compatible. Gives people a chance to clean up code slowly over time when they upgrade perhaps? Just a quick thought on that.

[Josh]

I will be using a new Lua binding system that supports smart pointers.  So Lua garbage collection will actually be useful now and the user never has to worry about AddRef / Release.

[Rick]

Is there a 95% use case for a specific type of pointer? Like are shared the majority of what a person would use? If there is then make that use case the default and easy vs the minority use case. ie if 95% of the time sources should use shared pointers then just make SourcePtr a typedef for shared_pointer<Source> to make things easier.

 

You're starting to see why languages like C# as loved so much. Don't have to deal with pointer **** directly and still get the feel of normal looking class.

[Ma-Shell]

I think, it's most reasonable to have the source maintain a smart pointer to itself. When the sound is finished playing or the user calls Stop(), this smart pointer is set to NULL.

This would mean:

- If the user keeps his own smart-pointer, there will be two smart pointers to the source and it will not be released until it is done playing and the user releases their own one.

- If the user does not keep his own smart-pointer, the source is deleted once it is finished playing.

[Rick]

6 minutes ago, Ma-Shell said:

I think, it's most reasonable to have the source maintain a smart pointer to itself. When the sound is finished playing or the user calls Stop(), this smart pointer is set to NULL.

This would mean:

- If the user keeps his own smart-pointer, there will be two smart pointers to the source and it will not be released until it is done playing and the user releases their own one.

- If the user does not keep his own smart-pointer, the source is deleted once it is finished playing.

Yeah I can see how one would get the best of both world there. At first it seems strange to be thst when a variable seems to goes out of scope it's still technically in scope, but I could see people liking that feature.

[Josh]

I was thinking along those lines.  The engine would maintain a list of all currently playing sources.  However, this would mean you had no way of stopping a source once you lost the handle (other than iterating through the global list, which seems kind of wrong to me).

[Rick]

Shell's way is convienent, the way I mentioned is reasonable (keep your references in scope for things you need), your way would require a lookup in a global storage container from some kind of key. The global list and lookup feels like it promotes some messy coding. It seems very GameCreators which I always thought was a strange way to mange resources and man did I see some horrible structures in some projects I joined because of it. When everything is available everywhere things get unruly quickly.

[Ma-Shell]

1 minute ago, Josh said:

I was thinking along those lines.  The engine would maintain a list of all currently playing sources.  However, this would mean you had no way of stopping a source once you lost the handle (other than iterating through the global list, which seems kind of wrong to me).

Correct, if the user wishes to stop the source, they have to keep a pointer to it themselves. If the user only wants to play the source once, without planning on stopping/resuming it, I see no reason, why you would want to force them to keep their own pointer around and continually check, whether the sound is done playing, so they can release it. The way I described it above, the user can choose whether to keep a pointer to it or not. Basically: Whenever the sound is playing, the engine makes sure that it is not deleted, while whenever the sound is not playing, it can be deleted.

If you go down the other route of always forcing the user to keep the pointer, I can already see a new thread coming up every other week complaining about their sounds not playing. I'm pretty sure that for most users, playing a sound is a "fire-and-forget" kind-of-thing.

 

PS.: What I forgot to mention above is, that if the user calls Play() after they called Stop(), the pointer needs to be populated again, of course

[Josh]

Also, I considered making a lot of funny typedefs for all the classes, but decided transparency was the best approach.  I don't want the engine to be plagued for years by legacy stuff where we think "Oh yeah, that is from the days before smart pointers".

[Rick]

You wouldn't have to check if the sound is playing in your case. You'd just have to be more careful with your scoping. The example presented in this thread should almost never happen in my view. Load your assets in a loading area, then play them via thier reference. That's a common design.

8 minutes ago, Ma-Shell said:

If you go down the other route of always forcing the user to keep the pointer, I can already see a new thread coming up every other week complaining about their sounds not playing. I'm pretty sure that for most users, playing a sound is a "fire-and-forget" kind-of-thing.

 

Who isnt doing this already today? In Lua people are making class/script variables to hold thier sources because you almost always need to play your sound more than once.

[Ma-Shell]

14 minutes ago, Rick said:

Who isnt doing this already today? In Lua people are making class variables to hold thier sounds because you almost always need to play your sound more than once.

Yeah, holding the sound is one thing but holding the sources played from that sound is another thing: If you have a gun, which you can fire quite fast, you will play the sound everytime you pull the trigger. However, you would expect there to be sometimes 3-4 sources of the same sound playing at the same time if you're trigger-happy. This would mean, you'll have to keep a list of all sources emitted around and periodically check, whether you can release the individual sources.

So: It's reasonable to always have a reference to the Sound, but not to all the Sources.

To sum up the design I would prefer with a clear differentiation between Sound and Source:

- Source has a smart-pointer to the Sound, which gets nullified when the source gets deleted. 

- Engine has a list of smart-pointers to all currently playing Sources. As soon as a Source is Stopped/Paused, this smart-pointer is nullified. The reasoning behind this, is that if the user plans to Resume/Restart the Source, they need to have their own smart-pointer anyway (otherwise they have no means of calling Resume()/Play()), which by itself prevents it from getting deleted. (This replaces what I previously stated as the source having a smart-pointer to itself)

 

Bonus: The Sound::Play()-function should return a smart-pointer to the emitted Source-object!

[Rick]

I'm pretty sure sources can play on top of each other. That is I can have one reference to a source that uses a sound and call play on that source while it's still playing from another play call and it works to give that rapid fire audio. Maybe I'm missing something around that? I'm sure I've done that before. So not sure what you mean keeping a list of sources.

[Ma-Shell]

You're right, at the moment this is working but what Josh is asking is about the future with smart-pointers. The question is, if the user should have to maintain these source-references for them to keep playing or if the engine should do that for you. (At least that's how I interpreted the question)

[Rick]

1 hour ago, Ma-Shell said:

You're right, at the moment this is working but what Josh is asking is about the future with smart-pointers.

To me he wasn't asking if this behaviour of being able to play the same source on top of each other with one reference should change. That will stay. He's simply asking if a source's reference goes out of scope but it's playing should it be deleted and therefore stop playing. To me I think that makes sense and it's what people do today anyway. That way you can play your audio over and over again via this reference.

To let it keep playing while the variable is out of scope now means you have no means to play it again, unless you add the functionality that you're talking about. I'm not really sure what the point of the engine storing these pointers though. The use case listed in the original post is a situation that should pretty much never happen. It doesn't seem to be what people are really doing today. From memory it's not what Josh does in his FPScript and at most it saves a person from having to make a variable at the correct scope for the rare instance they only want to play a sound once in the entire app. Doesn't seem worth the effort on Josh's side to me but I will say that it allows for both styles to work so it's no biggie if he wants to spend the time to manage them internally as well. I just don't see the benefit.