Skip to content

/Cataclysm-DDA

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Map sound effects. [Ready] #12337

Merged
merged 8 commits into from May 17, 2015

Conversation

Reviewers
No reviews
Assignees

@kevingranade kevingranade

Labels
None yet
Projects
None yet
Milestone
No milestone
7 participants
@CIB @KrazyTheFox @Zireael07 @Lain- @BevapDin @Coolthulhu @kevingranade
@CIB
Copy link
Contributor

commented May 10, 2015

Adds functionality for map sound effects. The sound effect to be played and the volume is determined directly by the same call made to produce in-game noises. There is a documentation file containing an overview of all the sound effect IDs that are supported so far.

https://github.com/CIB/CDDA-Test-Soundpack/

@CIB CIB changed the title Added additional parameters to sound::sound() for playing sound effects. Map sound effects. [WIP] May 10, 2015

@KrazyTheFox

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 10, 2015

(why wouldn't it)

It's pretty easy to open and close doors without making any noise. I do this every time I open or close a door just by turning the handle first. I feel this is something a survivor in a post cataclysmic world would do. If it must be added, then might I suggest that it be dexterity based?

Additionally, opening a door should be much quieter than closing one.
@CIB

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 10, 2015

Ehh.. Yeah, sure, if we had a sneak system that would make sense. But right now, I have the option of assuming that people aren't sneaking around 100% of the time, or reducing atmosphere by making doors be totally quiet. I prefer the former. Can reduce the noise of door opening, yeah.
@Zireael07

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 10, 2015

Cool!

@CIB CIB force-pushed the CIB:sound branch 2 times, most recently from c1e5591 to b2a1efd May 12, 2015

@Lain-

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 12, 2015

Looking forward to this! This will make the cataclysm much more immersed for me.
src/sounds.cpp Outdated
@@ -39,9 +41,9 @@ void sounds::ambient_sound(int x, int y, int vol, std::string description)
sound( tripoint( x, y, g->u.posz() ), vol, description, true );
}

void sounds::sound(int x, int y, int vol, std::string description, bool ambient)
void sounds::sound(int x, int y, int vol, std::string description, bool ambient, std::string id, std::string variant)

This comment has been minimized.

Sign in to view
Copy link
@BevapDin

BevapDin May 12, 2015

Contributor

This (and the other sound function) should take the strings as const reference.
src/sounds.cpp Outdated
@@ -200,6 +202,8 @@ void sounds::process_sound_markers( player *p )

for( const auto &sound_event_pair : sounds_since_last_turn ) {
const int volume = sound_event_pair.second.volume * volume_multiplier;
const std::string sfx_id = sound_event_pair.second.id;

This comment has been minimized.

Sign in to view
Copy link
@BevapDin

BevapDin May 12, 2015

Contributor

And here is no need to copy the strings, just use const references to them:

const std::string &sfx_id = sound_event_pair.second.id

This comment has been minimized.

Sign in to view
Copy link
@CIB

CIB May 13, 2015

Author Contributor

Good point, I haven't had to write efficient code in too long. I hear you also made a string-int class like I'd always wanted to have one of, I might even look into using that. :)

@CIB CIB force-pushed the CIB:sound branch from b2a1efd to c3b091f May 14, 2015

@CIB CIB changed the title Map sound effects. [WIP] Map sound effects. [Ready] May 14, 2015

@CIB

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 14, 2015

This should be ready for merge now.
@BevapDin

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 15, 2015

It looks fine and it compiles fine and it works fine (the add_msg call when a sound is to be played is triggered), but my system does not do audible sound (but that is a problem of my SDL setup). Somebody else needs to merge it.

CIB added some commits May 10, 2015

@CIB
Added additional parameters to sound::sound() for playing sound effects. 
Additionally, opening and closing doors now makes noise, roughly a bit more than a normal foot step (why wouldn't it).

Added sound effects for:
- Foot steps (OK this is annoying so I turned the volume down until it can't be heard)
- Opening/closing doors (by specifying variants you can make this work for curtains and the like)
8dcbe58
@CIB
Added various smashing sounds.
ab8739b
@CIB
Reduced the noise of opening doors to regular foot step level.
dcd47e8
@CIB
Added explosion sound effect. 
Also a text file containing documentation on valid sound effect IDs and variants.
b6a098d
@CIB
tripoint conversion.
35809f7
@CIB
Added melee_hit sound effect.
f35b60e
@CIB
Added gun sound effects.
9f1514b
@CIB
Changed string arguments to constant string references.
Loading status checks…
2ac80d8

@CIB CIB force-pushed the CIB:sound branch from 4edaad0 to 2ac80d8 May 15, 2015

@CIB

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 15, 2015

Latest update was just rebasing against the sounds::sound() 3d refactor.

@Coolthulhu Coolthulhu self-assigned this May 15, 2015

@Coolthulhu

This comment has been minimized.

Sign in to view
Copy link
Contributor

commented May 15, 2015

It's a pain to get the sound build working on mingw (pkg-config, dependencies etc.). It's probably way easier on Linux.

Deassigning now. If no one else has the time to test it I may retry later, but I'm giving up now.

@Coolthulhu Coolthulhu removed their assignment May 15, 2015

@kevingranade kevingranade self-assigned this May 16, 2015

@kevingranade

This comment has been minimized.

Sign in to view
Copy link
Member

commented May 17, 2015

Code looks good and I'm looking at merging now, but some requests. (that you likely know are desirable already)
If the intended distribution channel is outside the cdda repository, the doc file needs instructions on getting the sound pack and where to put it. It took me several minutes to figure out where to put the sound pack, and I'm reading the source code.
Looks like it can only support one sound pack at a time right now, an option that enumerates the packs like we do with tilesets would be great.

@kevingranade kevingranade merged commit 2ac80d8 into CleverRaven:master May 17, 2015
1 check passed

1 check passed

default
Details
@CIB

This comment has been minimized.

Sign in to view
Copy link
Contributor Author

commented May 17, 2015

@kevingranade Yeah I'll look into improving the docs. Enumerating sound sets is a desired feature, but I don't see it as a priority until the community picks up on creating sound sets of their own, which likely first requires more support for various sound effects in the game. There's also another problem, I guess I'll create an issue on the tracker.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.