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

Added a visual explosion to the flame thrower #14107

Merged
merged 1 commit into from Nov 12, 2017

Conversation

Projects
None yet
7 participants
@abc013
Contributor

abc013 commented Oct 3, 2017

closes #12233.

@Smittytron

The large explosion and sound are a bit jarring for an infantry-sized unit (Same with the gren but I plan to address that later.) For a cleaner fit, I went with:

VisualExplode:	
	Warhead@2Eff: CreateEffect
		Explosions: small_napalm
		ImpactSounds: firebl3.aud

I'd also remove the 50% explosion chance but that's just my pet peeve against RNG.

@abc013

This comment has been minimized.

Show comment
Hide comment
@abc013

abc013 Oct 4, 2017

Contributor

Your explosion and the sound fits for me, so updated.

Contributor

abc013 commented Oct 4, 2017

Your explosion and the sound fits for me, so updated.

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Oct 14, 2017

Member

The explosion plays at the unit's feet, which looks a bit unpolished. Can you move it up a couple of pixels to line up with the torso / fuel tank?

Member

pchote commented Oct 14, 2017

The explosion plays at the unit's feet, which looks a bit unpolished. Can you move it up a couple of pixels to line up with the torso / fuel tank?

@abc013

This comment has been minimized.

Show comment
Hide comment
@abc013

abc013 Oct 14, 2017

Contributor

@pchote i dont know where exactly i should adjust the position, could you give me a tip?

Contributor

abc013 commented Oct 14, 2017

@pchote i dont know where exactly i should adjust the position, could you give me a tip?

@pchote

This comment has been minimized.

Show comment
Hide comment
@pchote

pchote Oct 14, 2017

Member

The easiest way would be to define a new explosion sequence with an Offset. The proper solution would be to adjust the infantry CenterPosition vertically, but that may open a can of worms and other regressions.

Member

pchote commented Oct 14, 2017

The easiest way would be to define a new explosion sequence with an Offset. The proper solution would be to adjust the infantry CenterPosition vertically, but that may open a can of worms and other regressions.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Oct 14, 2017

Contributor

Needs a rebase, too.

Contributor

reaperrr commented Oct 14, 2017

Needs a rebase, too.

@abc013

This comment has been minimized.

Show comment
Hide comment
@abc013

abc013 Oct 28, 2017

Contributor

Thats strange, but changing the CenterPosition doesn't affect the explosion 🤔.
But I thought about adding an own Offset to Explodes, do you think this would be better?

Rebased, sorry for late response.

Contributor

abc013 commented Oct 28, 2017

Thats strange, but changing the CenterPosition doesn't affect the explosion 🤔.
But I thought about adding an own Offset to Explodes, do you think this would be better?

Rebased, sorry for late response.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Oct 28, 2017

Member

I think you can just crate a duplicate small_napalm with an offset. But an Explodes: offset may be useful for other cases/modders too tho.

Member

MustaphaTR commented Oct 28, 2017

I think you can just crate a duplicate small_napalm with an offset. But an Explodes: offset may be useful for other cases/modders too tho.

@abc013

This comment has been minimized.

Show comment
Hide comment
@abc013

abc013 Oct 29, 2017

Contributor

Updated, added a comment to the new defined explosion sequence to explain it's for E4.

Contributor

abc013 commented Oct 29, 2017

Updated, added a comment to the new defined explosion sequence to explain it's for E4.

@abc013

This comment has been minimized.

Show comment
Hide comment
@abc013

abc013 Oct 31, 2017

Contributor

Ok, should be fine now.

Contributor

abc013 commented Oct 31, 2017

Ok, should be fine now.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Nov 3, 2017

Contributor

Needs a rebase, and that merge commit looks fishy too.

Contributor

reaperrr commented Nov 3, 2017

Needs a rebase, and that merge commit looks fishy too.

@abc013

This comment has been minimized.

Show comment
Hide comment
@abc013

abc013 Nov 11, 2017

Contributor

@reaperrr should be ok now. Again, sorry for late response :/.

Contributor

abc013 commented Nov 11, 2017

@reaperrr should be ok now. Again, sorry for late response :/.

@ltem

ltem approved these changes Nov 11, 2017

@pchote

pchote approved these changes Nov 12, 2017

@pchote pchote merged commit 3092927 into OpenRA:bleed Nov 12, 2017

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@abc013 abc013 deleted the abc013:E4Explodes branch Nov 13, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment