Page MenuHomeWildfire Games

Add support for hotloading of sound-related files
Needs ReviewPublic

Authored by snokis on May 5 2020, 4:37 PM.
This revision needs review, but there are no reviewers specified.

Details

Reviewers
None
Summary

Handle changes of .ogg and .xml files in the audio/ directory. Files
will be reloaded from the SoundManagerWorker thread and not at any random time.
This will deal with changes to sound groups, standalone oggs and oggs used
by sound groups. Tell me if I've missed anything.

Test Plan

I've tested manually be changing parameters in sound groups and replacing
currently playing music and ambient sounds. Not sure how to automate it.

Event Timeline

snokis created this revision.May 5 2020, 4:37 PM
Vulcan added a comment.May 5 2020, 4:39 PM

Build failure - The Moirai have given mortals hearts that can endure.

Link to build: https://jenkins.wildfiregames.com/job/macos-differential/661/display/redirect

Vulcan added a comment.May 5 2020, 4:49 PM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Source...

source/soundmanager/ISoundManager.h
|  27| class·ISoundManager
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classISoundManager{' is invalid C code. Use --std or --language to configure the language.

source/soundmanager/SoundManager.h
|  27| #include·"items/ISoundItem.h"
|    | [MAJOR] CPPCheckBear (syntaxError):
|    | Code 'classISoundManager{' is invalid C code. Use --std or --language to configure the language.
Executing section JS...
Executing section cli...

Link to build: https://jenkins.wildfiregames.com/job/docker-differential/2076/display/redirect

Stan added a subscriber: Stan.EditedMay 5 2020, 4:59 PM

Hey, thanks for the patch! Here are some style comments.

You can try to add test_ files, changing them and making sure they are hotloaded see how it's done in others areas :)
You should also try with mods. In both the source (binaries/data/mods/) and ~/.local/share/0ad/ https://trac.wildfiregames.com/wiki/GameDataPaths It should work everywhere, no matter what is loaded, as long as the file is not overriden by another mod :)

You might want to pass const refs instead of pointers where possible too.

Btw, if feel free to make other patches, simple or not :) You can make as many as you want

source/soundmanager/ISoundManager.h
53

Is that name used for other classes doing hotloading?

source/soundmanager/SoundManager.cpp
124

Comments on top. Ends with periods :)

246

Not sure if we use the XML counterpart XMB (Binary xml files)

260

Is .ogg defined somewhere?

268

I guss you can use a range based loop here :)

540

nullptr :)

647

nullptr :)

656

nullptr :)

670

nullptr :)

798

Use nullptr instead.

954

Not sure it's worth the function call, I guess it won't be called often.
item maybe? could be a const method

Angen added a subscriber: Angen.May 5 2020, 5:02 PM
Angen added inline comments.
source/soundmanager/SoundManager.cpp
232

could you please take a look into wait notify for example? http://www.cplusplus.com/reference/condition_variable/condition_variable/

active waiting is not good idea

Angen added a comment.May 5 2020, 5:34 PM

thank you for working on this,
sorry my battery is low and i do not have cabel

source/soundmanager/SoundManager.cpp
236

You can omit { and } when only one line is part of the branch, for loop, while loop etc

255

remove return

533

nullptr

545

nullptr

645

Second parameter is super redundant, please remove it.
Also deleting not owned pointer is not safe.

647

nuke

649

Locale variable

Stan added a comment.Wed, Jul 29, 1:17 AM

Hey, any news?