Skip to content
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

TNT explosion custom sound #1823

Closed
wants to merge 4 commits into from
Closed

TNT explosion custom sound #1823

wants to merge 4 commits into from

Conversation

tenplus1
Copy link
Contributor

@tenplus1 tenplus1 commented Jul 8, 2017

This adds the ability to customise explosion sound when using tnt.boom() which can be used when creating custom tnt or when using other mods directly, it also fixes the 128 node max_hear_distance always being used by calculating the sound distance using the explosion radius.

This adds the ability to customise explosion sound when using tnt.boom()
@SmallJoker
Copy link
Member

add tnt sound info
@paramat
Copy link
Contributor

paramat commented Jul 9, 2017

Why? None of our other registered items have this ability, and an explosion sound works for all explosions.

@SmallJoker
Copy link
Member

Someone might want different explosion sounds - may it be for fun or to make it more realistic.

@tenplus1
Copy link
Contributor Author

tenplus1 commented Jul 9, 2017

The ability to add a custom sound is easily done here, give modders the option.

@paramat
Copy link
Contributor

paramat commented Jul 9, 2017

No objection.

@tenplus1
Copy link
Contributor Author

@paramat - would it be find if I added the sound distance fix in here also, to save a new pull ?

@paramat
Copy link
Contributor

paramat commented Jul 19, 2017

What needs fixing about the sound distance? And yes fine as a separate commit.

This fixes max_hear_distance so it's not always set to 128, instead it uses radius * 20 with a max of 128 in total.
added missing bracket
@sofar
Copy link
Contributor

sofar commented Aug 11, 2017

👍 but either this should be squashed to 1 or to 2 commits, now it's 4 commits.

@paramat
Copy link
Contributor

paramat commented Aug 12, 2017

Squash to 1 i think on merge.

@paramat
Copy link
Contributor

paramat commented Aug 18, 2017

bb08429

@paramat paramat closed this Aug 18, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants