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.

Sign up for GitHub

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

Jump to bottom

Fire - Fix animals screaming like humans #8625

Merged
merged 2 commits into from
Nov 9, 2021
Merged

Fire - Fix animals screaming like humans #8625

merged 2 commits into from
Nov 9, 2021

Conversation

Whigital
Copy link
Contributor

When merged this pull request will:

  • Only play screams if unit on fire is a human

Though hilarious, not too immersive and in line the realism ACE strives for ....

@PabstMirror PabstMirror added this to the 3.14.1 milestone Oct 25, 2021
@PabstMirror PabstMirror added the kind/bug-fix Release Notes: **FIXED:** label Oct 25, 2021
@veteran29
Copy link
Member

Conflicts with #8578

@Whigital
Copy link
Contributor Author

Doh, sorry, missed that PR.

Close, wait until merged and resolve, or incorporate into #8578 ?

@veteran29 veteran29 mentioned this pull request Oct 26, 2021
Merged
@veteran29
Copy link
Member

Doh, sorry, missed that PR.

Close, wait until merged and resolve, or incorporate into #8578 ?

I've rebased your PR on master branch after conflicting PR was merged. It also made me aware that there are 2 places in the code where screams are played and only one of them is in dedicated function.

The whole fnc_burn feels bit big and spaghetti ;/

@veteran29
Copy link
Member

Maybe it would be worth to add another param to burnReaction... something like _throwWeapon and utilise this function in both places.

jonpas
jonpas approved these changes Nov 7, 2021
View reviewed changes
Copy link
Member

@jonpas jonpas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@veteran29 Can you make a new issue for that? I'd like to see that too, but it doesn't have to block this PR.

@veteran29
Copy link
Member

@veteran29 Can you make a new issue for that? I'd like to see that too, but it doesn't have to block this PR.

I can fix it in this PR if the author (@Whigital is fine with this).

@Whigital
Copy link
Contributor Author

Whigital commented Nov 8, 2021

@veteran29 No worries, go ahead 👍

@PabstMirror PabstMirror merged commit d73f0f3 into acemod:master Nov 9, 2021
@Whigital Whigital deleted the fire_screams branch November 9, 2021 08:59
AndreasBrostrom pushed a commit to AndreasBrostrom/ACE3 that referenced this pull request Dec 3, 2021
@Whigital @veteran29 @AndreasBrostrom
Fire - Fix animals screaming like humans (acemod#8625)
04cbe09 
* Fire - Only human screams

* Use fnc_burnReaction for screams

Co-authored-by: Filip Maciejewski <veteran29@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PabstMirror PabstMirror PabstMirror approved these changes

@jonpas jonpas Awaiting requested review from jonpas

Assignees
No one assigned
Labels
kind/bug-fix Release Notes: **FIXED:**
Projects
None yet
Milestone
3.14.1
Development

Successfully merging this pull request may close these issues.
4 participants
@Whigital @veteran29 @jonpas @PabstMirror