CaveShark

Hehe, glad to see you got FMOD working after all. Have you started working on a wrapper class (for reuse in future projects) or is it all directly integrated in your source for the time being? Not that either way is any better, once compiled, but it's worth the time to try to modularize code. Just some advice from an idiot who spent many projects copy-pasting bits and pieces from his Win32 GUI management routines in his younger years.

Err, it's a seperate class, but it does specifically what it needs to do for CaveShark. A wrapper class wouldn't be too hard though. It'd just take slight modification of my current class. It also should have the ERRCHECK with result as PART of the class. It's a seperate stand alone function right now.

class sounds
{
public:
/*creates and initialize the sound system
and the sound streams for weapon.wav
and item.wav */
void init();

void wpnSnd(); //Plays weapon sound
void itmSnd();  //Plays item sound
private:
FMOD::System     *system; //pointer to the sound system object
FMOD::Sound      *wpnwav; //pointer to weapon wav
FMOD::Sound      *itmwav; //pointer to item wav
FMOD_RESULT       result; //for ERRCHECK
};

I won't bother including the actual function definitions as the comments describe them.

stupid board ruined my indents.

 

Sounds quite good. Is there a reason why you create seperate functions for item and weapon sounds instead of passing a parameter to a "playSound()" function of some kind?

Unless you have a different format for either (in which case specialized code will be necessary, if only to normalize your audio data as some kind of raw format FMOD can play, most likely necessary if you load them directly from the executable) you could make that class into a nice little wrapper that would help you skip over the tedious lower-level junk (creating the FMOD system object and detecting device caps, etc...) for future projects, or other places where you might want to add sounds in your project.

Writing good, clean reusable code is a good habit. Of course it depends on the context you're in - chances are you wouldn't be reusing code that uses CS's own format for sounds, for instance, in other projects (unless you make another editor of some kind...)

 

Sounds quite good. Is there a reason why you create seperate functions for item and weapon sounds instead of passing a parameter to a "playSound()" function of some kind?

As I said, this wasn't coded to work elsewhere. I was coding it quick, without much thought of making it versatile. I suppose I really should make it a wrapper that loads ONE sound and plays it back, with init() as void init(char * file_name_or_path) instead. and since the object would then represent a single sound, I could name it appropriately when I declare it. better yet instead of init, use a constructor, that way it automatically does the init stuff when I declare the object.

sounds(char * file_name_or_path);

sounds wpnSnd = "sounds\weapon.wav";

since it's going to be a single sound and be versatile, I'll probably rename the class to like, snd or something. naming the class 'Sound' could cause problems and possible confusion, since Sound is a subclass of FMOD.

as for playSound(), that will simply be play(). playSound() is already used by FMOD, and also, this is being called from an object of class snd, so playSOUND would be ridundant. so 'wpnSnd.play();'. Not bad at all.

you could name the object wpn, but I've already used that in part of my code for the weapons.

EDIT: Changes done! Download:CaveShark 1.21

I created a wrapper class for sounds this time that I can use in any other code. Other than that, it's identical to 1.2 except that I fixed the item sound not playing when moving down across the list. all other directions played it.

 

Nice. Your code is getting more versatile.

A word of advice: don't do ANYTHING in the constructor. Initialize your variables, but don't give them content. This not only goes against what a constructor should do (ie, "construct" the object for the programmer's eventual use) but makes it much less versatile (you'll have to create an object per sound and can't reuse them.) Furthermore, you might eventually find yourself wanting to do more than just play a sound (say, creating a DSP filter to give it echo) and may have to do some operations before loading and playing it.

Other than that, nice work.

 

Nice. Your code is getting more versatile.

A word of advice: don't do ANYTHING in the constructor. Initialize your variables, but don't give them content. This not only goes against what a constructor should do (ie, "construct" the object for the programmer's eventual use) but makes it much less versatile (you'll have to create an object per sound and can't reuse them.) Furthermore, you might eventually find yourself wanting to do more than just play a sound (say, creating a DSP filter to give it echo) and may have to do some operations before loading and playing it.

Other than that, nice work.

That's why you can use function overloading on constructors

say, for class snd:

snd();
snd(char * file);

later, declaring variables:

snd theSnd; // construct the object with snd(), but leave it alone for later use
snd wpnSnd = "sounds\\weapon.wav"; //construct the object with snd(char * file), initializing and preloading it with specified file, for immediate use.

for the case of the constuctor that leaves the object alone, you would add functions to the class that can call the initialization processes that you want for your specific fmod object. But I haven't found the need to do that yet.

If I created a game or program that used 20 sounds, for example, I DEFINITELY would have the need, however. one object that using functions can load and play all 20 of those sounds when needed would be easier than declaring 20 snd objects, obviously.

EDIT: with a bit of modification, I could even make a constructor that took some int x as a parameter and made a snd object with an array of x number of sound files. But then it could also get really hard to remember which number in the array is which sound. You could use enum to get around that, but I never really could get myself to LIKE enum.

 

What I meant was that having your constructor do work other than initialising variables to a safe value (ie, generally NULL) is an improper use of constructors. It's less "clean" and standard than creating a seperate loader function. For instance...

CObject* myObj = new CObject("filetoload.dat");
myObj.parse();
myObj.loadFile("file2.dat");
myObj.parse();

...is less standard and far less clean than...

CObject myObj;
myObj.loadFile("fileA.dat");
myObj.parse();
myObj.loadFile("fileB.dat");
myObj.parse();

...or even...

CObject myObj;
myObj.loadAndParse("fileA.dat");
myObj.loadAndParse("fileB.dat");

The purpose of a constructor is really just to set the object up so that it won't be created in a state that could cause a crash. It's like creating private members - you don't actually NEED them, but suppose you write a class which handles window and control creation and management. You don't want someone messing with the handle since, well, let's face it, there isn't any reason why you'd want to do that. So you'd make it private and create a method to retreive it if necessary (for instance, to call something the class doesn't implement.)

Just like you could leave your internal-use members public, you can have your constructor do anything you want. It can even load, play, and remove the song from memory without having to call any methods. But this behavior might end up confusing another user who uses the class, or you in a few months when you'll come back to it and decide to implant it elsewhere ("What the... why are my sounds playing during window creation?")

 

...

Just like you could leave your internal-use members public, you can have your constructor do anything you want. It can even load, play, and remove the song from memory without having to call any methods. But this behavior might end up confusing another user who uses the class, or you in a few months when you'll come back to it and decide to implant it elsewhere ("What the... why are my sounds playing during window creation?")

LOL. Yeah, I'm not stupid enough to make a constructor go as far as PLAY the sound.

But it'd be clean enough to at least have the constructor call System_Create() and MAYBE system->init(), right? If you don't, it seems kinda silly to create a wrapper, other than to make the function calls shorter to type.

Especially since you pretty much HAVE to call those to set up an FMOD object.

 

Wow. I'll have to take a look at this now that you've
released a version of this. It sounds neat. =)

also, does this use Cave Story sound effects?

I need CS sounds for my RPGM2k3 CS game. =D;

 

When I have time, I'll work on another update. A friend of mine used it and immediately pointed out that I have the Maximum exp values for some or all of the weapons all messed up. I coulda sworn I entered those into my code correctly.

So when I have time, I'll have a look and see what the blazes I did wrong.

 

Usually, if they're operations that will only be done once and must be done to properly use the class, putting it in the constructor is almost certainly going to be a good idea. For instance you need to set up FMOD before you can use it to do anything, and you're only going to do it once anyhow.

Consider this though: suppose you have multiple sounds playing at once. What happens then? You have one instance of FMOD created per sound playing. That's a bit of a problem, and the danger with doing too much in the constructor. You could add a public function that takes a pointer to an instance of FMOD's system object that sets a private member of the class representing the system object it must use to play sound, for instance ("setSystem(...)")

This might seem like unecessary work, and it probably is if you just have one sound playing and doing nothing else. But someday your wrapper might allow, say, the inclusion of DSP filters, and you'll be glad to just have to type "fmodWrapper->addEcho()" instead of setting up the filter, its parameters, and binding it to a channel.

If you really don't want to create your system object seperately, there's no harm in creating multiple objects, but if you're striving for clean code it just won't do.

Good luck!

Edit: Or better yet, your wrapper could allow for the dynamic inclusion of additional channels/sounds and encapsulate the entire FMOD library instead of just sounds. THEN you can create your system object in the constructor and not have to worry about having multiple instances of the system class at once.

 

...

Edit: Or better yet, your wrapper could allow for the dynamic inclusion of additional channels/sounds and encapsulate the entire FMOD library instead of just sounds. THEN you can create your system object in the constructor and not have to worry about having multiple instances of the system class at once.

Yeah, that's kinda what I was getting at. basically like, y'know, the class being a wrapper FMOD, but basically it being an embodyment in object form of the FMOD -system- object, that's more within the programmer's reach and can have everything set up by the programmer AFTER construction. Just with the main System stuff out of the way. Have the contructor just create and initialize the system, as you pretty much have to do that every time you want to use FMOD.

Say I rename the class as...err...System is probably already taken by quite a few classes I'm sure. So how about FMODSystem. If you need to initialize the system with a specific parameters, then, using overloading on the constructor, you could call have defaults for each of the different parts if you WANT to use defaults. Allowing for FMODSystem(), FMODSystem(int maxchannels), FMODSystem(FMOD_INIT_FLAGS flags), FMODSystem(void * extradriverdata), FMODSystem(int maxchannels, FMOD_INIT_FLAGS flags, void * extradriverdata), or even any combination of just TWO of the variables normally required by system->init().