widelands

Merge lp://staging/~widelands-dev/widelands/cleanup-soundhandler into lp://staging/widelands

  1. cleanup-soundhandler
  2. Merge into trunk
Proposed by GunChleoc
Status: Merged
Merged at revision: 9068
Proposed branch: lp://staging/~widelands-dev/widelands/cleanup-soundhandler
Merge into: lp://staging/widelands
Prerequisite: lp://staging/~widelands-dev/widelands/cleanup-rendertarget
Diff against target: 6413 lines (+1517/-1328)
170 files modified
data/campaigns/emp04.wmf/scripting/tribes/brewery1.lua (+1/-1)
data/campaigns/emp04.wmf/scripting/tribes/brewery2.lua (+1/-1)
data/campaigns/emp04.wmf/scripting/tribes/mill1.lua (+1/-1)
data/campaigns/emp04.wmf/scripting/tribes/mill2.lua (+1/-1)
data/tribes/buildings/productionsites/atlanteans/gold_spinning_mill/init.lua (+1/-1)
data/tribes/buildings/productionsites/atlanteans/horsefarm/init.lua (+1/-1)
data/tribes/buildings/productionsites/atlanteans/mill/init.lua (+2/-2)
data/tribes/buildings/productionsites/atlanteans/sawmill/init.lua (+1/-1)
data/tribes/buildings/productionsites/atlanteans/smelting_works/init.lua (+4/-4)
data/tribes/buildings/productionsites/atlanteans/toolsmithy/init.lua (+12/-12)
data/tribes/buildings/productionsites/atlanteans/weaponsmithy/init.lua (+10/-10)
data/tribes/buildings/productionsites/atlanteans/weaving_mill/init.lua (+3/-3)
data/tribes/buildings/productionsites/barbarians/ax_workshop/init.lua (+6/-6)
data/tribes/buildings/productionsites/barbarians/big_inn/init.lua (+3/-3)
data/tribes/buildings/productionsites/barbarians/cattlefarm/init.lua (+1/-1)
data/tribes/buildings/productionsites/barbarians/inn/init.lua (+2/-2)
data/tribes/buildings/productionsites/barbarians/lime_kiln/init.lua (+2/-2)
data/tribes/buildings/productionsites/barbarians/metal_workshop/init.lua (+10/-10)
data/tribes/buildings/productionsites/barbarians/smelting_works/init.lua (+4/-4)
data/tribes/buildings/productionsites/barbarians/tavern/init.lua (+1/-1)
data/tribes/buildings/productionsites/barbarians/warmill/init.lua (+12/-12)
data/tribes/buildings/productionsites/barbarians/weaving_mill/init.lua (+1/-1)
data/tribes/buildings/productionsites/barbarians/wood_hardener/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/brewery/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/donkeyfarm/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/inn/init.lua (+2/-2)
data/tribes/buildings/productionsites/empire/mill/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/piggery/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/sawmill/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/sheepfarm/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/smelting_works/init.lua (+4/-4)
data/tribes/buildings/productionsites/empire/stonemasons_house/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/tavern/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/toolsmithy/init.lua (+12/-12)
data/tribes/buildings/productionsites/empire/weaponsmithy/init.lua (+10/-10)
data/tribes/buildings/productionsites/empire/weaving_mill/init.lua (+1/-1)
data/tribes/buildings/productionsites/empire/winery/init.lua (+1/-1)
data/tribes/buildings/productionsites/frisians/armor_smithy_large/init.lua (+4/-4)
data/tribes/buildings/productionsites/frisians/armor_smithy_small/init.lua (+5/-5)
data/tribes/workers/atlanteans/builder/init.lua (+2/-2)
data/tribes/workers/atlanteans/farmer/init.lua (+1/-1)
data/tribes/workers/atlanteans/fisher/init.lua (+2/-2)
data/tribes/workers/atlanteans/geologist/init.lua (+1/-1)
data/tribes/workers/atlanteans/shipwright/init.lua (+3/-3)
data/tribes/workers/atlanteans/stonecutter/init.lua (+1/-1)
data/tribes/workers/atlanteans/woodcutter/init.lua (+2/-2)
data/tribes/workers/barbarians/builder/init.lua (+2/-2)
data/tribes/workers/barbarians/farmer/init.lua (+1/-1)
data/tribes/workers/barbarians/fisher/init.lua (+2/-2)
data/tribes/workers/barbarians/geologist/init.lua (+1/-1)
data/tribes/workers/barbarians/lumberjack/init.lua (+2/-2)
data/tribes/workers/barbarians/shipwright/init.lua (+3/-3)
data/tribes/workers/barbarians/stonemason/init.lua (+1/-1)
data/tribes/workers/empire/builder/init.lua (+2/-2)
data/tribes/workers/empire/farmer/init.lua (+1/-1)
data/tribes/workers/empire/fisher/init.lua (+2/-2)
data/tribes/workers/empire/geologist/init.lua (+1/-1)
data/tribes/workers/empire/lumberjack/init.lua (+3/-3)
data/tribes/workers/empire/shipwright/init.lua (+3/-3)
data/tribes/workers/empire/stonemason/init.lua (+2/-2)
data/tribes/workers/frisians/builder/init.lua (+2/-2)
data/tribes/workers/frisians/shipwright/init.lua (+2/-2)
data/world/critters/duck/init.lua (+1/-2)
data/world/critters/elk/init.lua (+1/-2)
data/world/critters/fox/init.lua (+1/-2)
data/world/critters/sheep/init.lua (+1/-2)
data/world/critters/stag/init.lua (+1/-2)
data/world/critters/wildboar/init.lua (+1/-2)
data/world/critters/wolf/init.lua (+2/-3)
data/world/immovables/grass1/init.lua (+1/-2)
data/world/immovables/grass2/init.lua (+1/-2)
data/world/immovables/grass3/init.lua (+1/-2)
data/world/immovables/trees/alder/init.lua (+1/-2)
data/world/immovables/trees/aspen/init.lua (+1/-2)
data/world/immovables/trees/beech/init.lua (+1/-2)
data/world/immovables/trees/birch/init.lua (+1/-2)
data/world/immovables/trees/larch/init.lua (+1/-2)
data/world/immovables/trees/maple/init.lua (+1/-2)
data/world/immovables/trees/oak/init.lua (+1/-2)
data/world/immovables/trees/palm_borassus/init.lua (+1/-2)
data/world/immovables/trees/palm_coconut/init.lua (+1/-2)
data/world/immovables/trees/palm_date/init.lua (+1/-2)
data/world/immovables/trees/palm_oil/init.lua (+1/-2)
data/world/immovables/trees/palm_roystonea/init.lua (+1/-2)
data/world/immovables/trees/rowan/init.lua (+1/-2)
data/world/immovables/trees/spruce/init.lua (+1/-2)
doc/sphinx/source/animations.rst (+7/-3)
doc/sphinx/source/productionsite_program.rst (+3/-3)
regression_test.py (+1/-2)
src/economy/flag.h (+1/-0)
src/economy/portdock.cc (+1/-1)
src/economy/portdock.h (+1/-1)
src/economy/road.h (+1/-2)
src/editor/editorinteractive.cc (+6/-2)
src/editor/editorinteractive.h (+1/-0)
src/graphic/CMakeLists.txt (+2/-0)
src/graphic/animation.cc (+26/-17)
src/graphic/animation.h (+7/-1)
src/graphic/game_renderer.cc (+2/-2)
src/graphic/rendertarget.cc (+7/-9)
src/graphic/rendertarget.h (+9/-5)
src/logic/CMakeLists.txt (+1/-0)
src/logic/editor_game_base.cc (+4/-0)
src/logic/game.cc (+3/-2)
src/logic/map_objects/bob.cc (+3/-2)
src/logic/map_objects/bob.h (+1/-1)
src/logic/map_objects/immovable.cc (+9/-8)
src/logic/map_objects/immovable.h (+3/-1)
src/logic/map_objects/immovable_program.h (+4/-5)
src/logic/map_objects/tribes/building.cc (+2/-1)
src/logic/map_objects/tribes/building.h (+1/-1)
src/logic/map_objects/tribes/constructionsite.cc (+16/-6)
src/logic/map_objects/tribes/constructionsite.h (+6/-2)
src/logic/map_objects/tribes/dismantlesite.cc (+13/-3)
src/logic/map_objects/tribes/dismantlesite.h (+5/-0)
src/logic/map_objects/tribes/partially_finished_building.cc (+2/-3)
src/logic/map_objects/tribes/production_program.cc (+10/-8)
src/logic/map_objects/tribes/production_program.h (+1/-1)
src/logic/map_objects/tribes/ship.cc (+4/-3)
src/logic/map_objects/tribes/ship.h (+1/-1)
src/logic/map_objects/tribes/soldier.cc (+3/-3)
src/logic/map_objects/tribes/soldier.h (+1/-0)
src/logic/map_objects/tribes/worker.cc (+8/-7)
src/logic/map_objects/tribes/worker.h (+2/-0)
src/logic/map_objects/tribes/worker_program.cc (+21/-21)
src/logic/message.h (+1/-1)
src/logic/player.cc (+20/-13)
src/logic/player.h (+6/-1)
src/sound/CMakeLists.txt (+8/-0)
src/sound/constants.cc (+1/-0)
src/sound/constants.h (+65/-0)
src/sound/fxset.cc (+76/-23)
src/sound/fxset.h (+32/-35)
src/sound/note_sound.h (+7/-12)
src/sound/songset.cc (+29/-19)
src/sound/songset.h (+5/-9)
src/sound/sound_handler.cc (+388/-443)
src/sound/sound_handler.h (+119/-131)
src/ui_basic/CMakeLists.txt (+1/-0)
src/ui_basic/panel.cc (+10/-7)
src/ui_basic/panel.h (+4/-2)
src/ui_basic/slider.cc (+2/-1)
src/ui_basic/slider.h (+1/-0)
src/ui_fsmenu/CMakeLists.txt (+2/-0)
src/ui_fsmenu/internet_lobby.cc (+4/-1)
src/ui_fsmenu/internet_lobby.h (+1/-0)
src/ui_fsmenu/options.cc (+12/-42)
src/ui_fsmenu/options.h (+4/-8)
src/website/CMakeLists.txt (+0/-1)
src/website/website_common.cc (+1/-7)
src/wlapplication.cc (+41/-13)
src/wlapplication_messages.cc (+0/-4)
src/wui/CMakeLists.txt (+13/-0)
src/wui/game_chat_panel.cc (+5/-2)
src/wui/game_chat_panel.h (+1/-0)
src/wui/game_options_menu.cc (+0/-1)
src/wui/game_options_sound_menu.cc (+19/-119)
src/wui/game_options_sound_menu.h (+5/-40)
src/wui/interactive_base.cc (+29/-23)
src/wui/interactive_base.h (+4/-4)
src/wui/interactive_player.cc (+18/-7)
src/wui/interactive_player.h (+2/-0)
src/wui/interactive_spectator.cc (+7/-2)
src/wui/interactive_spectator.h (+2/-1)
src/wui/itemwaresdisplay.cc (+1/-1)
src/wui/mapview.cc (+4/-3)
src/wui/mapview.h (+1/-1)
src/wui/sound_options.cc (+135/-0)
src/wui/sound_options.h (+32/-0)
src/wui/transport_draw.cc (+5/-5)
To merge this branch: bzr merge lp://staging/~widelands-dev/widelands/cleanup-soundhandler
Reviewer Review Type Date Requested Status
Klaus Halfmann reiew, compile, testplay Approve
Review via email: mp+365001@code.staging.launchpad.net

Commit message

Complete overhaul of the sound handler

- Extensive refactoring of the SoundHandler, FXSet and SongSet classes.
  Make it Widelands-agnostic.
- Categorize sound effects and overhaul the sound options to have
  separate sliders for music, UI sounds, chat messages, game messages
  and ambient sounds. Available both in-game and in the Fullscreen menu
  options.
- Memory saver: Only load sound effects when they are played for the
  first time and then keep them in memory until their category is
  cleared. Clear the ambient sounds when EditorGameBase gets destroyed
- Implement stereo position and distance for sounds emitted by map
  objects. This includes in-game messaages to the player that have been
  triggered by map objects.
- Remove the now obsolete commandline options - we only support
  --nosound now.
- Fix slider style for FSMenu sliders
- Website binaries no longer instantiate the global sound handler.
- Switch to ingame music set in netsetup and internet lobby, because
  just 1 song can get annoying while waiting for other players.
- The register_fx function now only uses the file path for identifying
  the files.
- Randomize the time last played on creation of fxset, to prevent
  ambient sound barrage when starting a new game.
- Rename g_soundhandler to g_sh for consistency with other global
  objects.

Description of the change

This is for Build 21.

Enjoy your games with full stereo sound :)

Unfortunately, panning sound effects that are already playing while the map view is being moved does not seem to work with SDL_Mixer. We'll probably have to switch to a different backend for that (e.g. OpenAL), which is out of scope for this branch.

To post a comment you must log in.
Revision history for this message
GunChleoc (gunchleoc) wrote :

The new UI classes and changes to WLApplication are missing from the diff, because it is too long :(

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4628. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/510328711.
Appveyor build 4415. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cleanup_soundhandler-4415.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4648. State: failed. Details: https://travis-ci.org/widelands/widelands/builds/511299575.
Appveyor build 4435. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cleanup_soundhandler-4435.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

I played the first 3 Tutorial on this branhc and everyhting was fine.

When hanging around in the config I get
> Songset: Loaded song "music/menu_00.ogg"
 again and again, is this intended, or should this happen only once

I will try to do a code review. But this seems to be a _bigger_ task :-)

Revision history for this message
GunChleoc (gunchleoc) wrote :

That message should come up every time that the song finishes playing and starts again.

Yep, definitely a big code review. Probably best do it with a diff program like Meld and add your remarks as NOCOM comments to the code. I'll merge trunk now to make the diff smaller.

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

Review, found so far:
* restructered the sound directory wihht more subdirectories
* Help now shows --nosound Starts the game with sound disabled.
* Using FXset one could have different soundscapes for testing or scenarions, interesting
* I assuem the random must be a _local_ random, as sounds are per player.
* More coommetns please, see inline comments

For the wishlist: a helpscreen explainng the sounds :-)

Checekd upto "TODO(GunChleoc): Compatibility code to avoid getting bug reports about unread sections. Remove after Build 21." sorry can nott do more tonight

Revision history for this message
Klaus Halfmann (klaus-halfmann) wrote :

I played some internet game with this brnahc and it was ok, felt a bit slow, though.

Completed the review, code is fine, please check some nits:
 * some functions should be inlined.
 * Once this is in r21 I would like to do a performace review
 * Mabye we could make more of this run in a seperate thread / cpu?

This can go in in r21 anytime now.

review: Approve (reiew, compile, testplay)
Revision history for this message
GunChleoc (gunchleoc) wrote :

Thanks a lot for the review!

SDL_Mixer already uses threading for playing the sounds.

The only other spot where we could use some threading would be when registering sounds, but then the threads would fight over hard disk access and probably make things slower rather than faster. I already have another branch in the works that will speed up finding the files for animations and sounds by not using regular expressions, which will greatly speed up loading the tribes and world.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4736. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/522063139.
Appveyor build 4521. State: failed. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cleanup_soundhandler-4521.

Revision history for this message
GunChleoc (gunchleoc) wrote :

@bunnybot merge

Revision history for this message
bunnybot (widelandsofficial) wrote :

Continuous integration builds have changed state:

Travis build 4770. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/523599764.
Appveyor build 4554. State: success. Details: https://ci.appveyor.com/project/widelands-dev/widelands/build/_widelands_dev_widelands_cleanup_soundhandler-4554.

Revision history for this message
bunnybot (widelandsofficial) wrote :

Refusing to merge, since Travis is not green. Use @bunnybot merge force for merging anyways.

Travis build 4770. State: errored. Details: https://travis-ci.org/widelands/widelands/builds/523599764.

Revision history for this message
GunChleoc (gunchleoc) wrote :

Transient failure on Travis

@bunnybot merge force

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: