Sound Manager Cleanup [Updated]

[Draygera]

Today, I'm starting to check into this and am planning on getting most of it finished as soon as I can. Seems simple enough since most of the task is removing certain variables and their references.

--Brandon

[Zini]

For reference, we are talking about this task here: Sound Manager Cleanup

I still don't see you on the tracker, so I can't officially assign it to you yet.

Edit: And while you are at it you could also bring the SoundManager in line with our naming conventions (see wiki).

[Draygera]

Now I'm registered on the bug tracker. Sorry about that.

[Draygera]

I removed the boost::timer variable from the SoundManager and replaced it with an incrementing integer and noticed that OpenMW loads and shuts down (seemingly almost instantly) faster with this gone. Did not notice any changes in sound quality or any other issues. I would like for someone else to test this out. Please and thank you.

[swick]

I don't notice any speed differnce. I only got an unused var (go) in openmw/mwsound/soundmanager.cpp:579:38

[Draygera]

Sorry. No speed difference. Didn't realize I was using the launcher instead of the executable. The launcher after closing for some reason hangs on the command line until you hit Enter. Really weird.

Swick, I checked that warning out and from what I can see:

Code: Select all

if(total == 0){
             while (!(soundIter == test.soundList.end()))
             {
                        ESM::NAME32 go = soundIter->sound;
                                int chance = (int) soundIter->chance;
                                //std::cout << "Sound: " << go.name <<" Chance:"
 <<  chance << "\n";
                                soundIter++;
                                total += chance;
                                }
                        }
                               

go looks like it was originally just for debugging purposes and it doesn't show up anywhere else in the code. If I comment out this entire group of code, except for some esmtools warnings:

Code: Select all

/home/draygera/Source/Engines/OpenMW/openmw-git/apps/esmtool/esmtool_cmd.c: In function 'update_arg':
/home/draygera/Source/Engines/OpenMW/openmw-git/apps/esmtool/esmtool_cmd.c:930:15: warning: variable 'val' set but not used [-Wunused-but-set-variable]
/home/draygera/Source/Engines/OpenMW/openmw-git/apps/esmtool/esmtool_cmd.c:929:9: warning: variable 'stop_char' set but not used [-Wunused-but-set-variable]
/home/draygera/Source/Engines/OpenMW/openmw-git/apps/esmtool/esmtool_cmd.c: In function 'cmdline_parser_internal':
/home/draygera/Source/Engines/OpenMW/openmw-git/apps/esmtool/esmtool_cmd.c:999:7: warning: variable 'check_required' set but not used [-Wunused-but-set-variable]

my code compiles fine and everything works as intended, so far.

[Zini]

Sorry, integer won't do. You would end up with inconsistent behaviour when the framerate is not constant. Please use the frame duration as described in the bug tracker.

Also, a few hints about procedures. We don't usually work with patches and since you have pushed the changes to your repository the patch you uploaded is redundant anyway.

Furthermore I would like to direct you to the developer documentation on the wiki again, especially the page about branching policies. You can continue to use your branch for now, since it is not a big deal, but please stick to our policies for future tasks.

[Draygera]

Yes, sir. I will do that. I'm going to do some research now and I'll try to have it corrected by tonight or tomorrow.