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

Added Tiberian Sun burning trees #18730

Merged
merged 1 commit into from Mar 12, 2022
Merged

Conversation

Mailaender
Copy link
Member

Closes #18657.

@pchote
Copy link
Member

pchote commented Oct 17, 2020

#18657 is specifically asking for the random spread probability (with documentation on how it worked!), and when discussing the feature in Discord @henskelion also specifically talked about the fire animation growing/dying off on the same tree instead of being picked randomly.

Can we justify not implementing this correctly in our TS?

@Mailaender
Copy link
Member Author

Updated to differentiate big and large fires. https://github.com/LipkeGu/TSIniFiles/blob/master/rules.ini#L354 mentions it in the comments, but the logic is hidden.

@pchote
Copy link
Member

pchote commented Oct 18, 2020

The logic isn't hidden, you just have to read the FIRE1, FIRE2, FIRE3 definitions in art.ini.

@Mailaender
Copy link
Member Author

Do you think the random element I omitted here is essential?

@pchote
Copy link
Member

pchote commented Oct 18, 2020

We are trying to recreate TS, which means that the default standard should be to match its behaviour. We know how the original behaviour here worked, so I think that the onus here is to justify why we should deliberately choose to ignore that and implement something different - which will likely require more work overall to polish and (maybe?) balance than using the original parameters.

@Mailaender
Copy link
Member Author

Indeed, it was a bit lazy to just shovel the @OpenHV setup, so I wrote a new trait that takes the original random elements from https://www.modenc.renegadeprojects.com/TreeFlammability into account. In practice the result is not so different.

@ABrandau
Copy link
Contributor

A couple of suggestions:

  • Damage can be applied with a changes health trait, i dont know if its necessary for the trait,
  • The trait could enable a condition to the actor to allow more customization into the unit.
  • A delay tag could be added into trait to customize how long it takes for the entire forest to be on fire?

On the TS behaviour.

Trees are not set on fire when they take damage, they seem to be set on fire only when attacked with a fire weapon, in fact, bullets do not harm trees.

Damage doesnt set trees of fire either, only fire damage (perhaps ion storms too if we get to that stage?)

treesfire

trees

@Mailaender
Copy link
Member Author

Rewrote this to be more generic, use only the condition system instead of dealing damage and added a delay interval.

Copy link
Member

@phrohdoh phrohdoh left a comment

Choose a reason for hiding this comment

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

Inline documentation suggestions.

OpenRA.Mods.Cnc/Traits/Spreads.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Cnc/Traits/Spreads.cs Outdated Show resolved Hide resolved
@Mailaender
Copy link
Member Author

Updated.

Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

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

How much influence do we have on the graphics? The tree in the middle is burning but that is not really visible at all.
grafik

OpenRA.Mods.Cnc/Traits/Spreads.cs Outdated Show resolved Hide resolved
@Mailaender
Copy link
Member Author

I tweaked Z coordinate and offset a bit.

@Mailaender
Copy link
Member Author

Tried the other blend modes. Nothing quite matches the effects from Tiberian Sun yet.

Copy link
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

Nothing quite matches the effects from Tiberian Sun yet.

This was fixed by #19122, which has caused a rebase conflict.

Interactable:
HiddenUnderShroud:
FrozenUnderFog:
Copy link
Member

Choose a reason for hiding this comment

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

This is important to the feature to work, but noting that this could potentially bring a significant performance regression (but hopefully now much less than when we changed FrozenUnderFogHiddenUnderShroud in the first place).

@Mailaender
Copy link
Member Author

Rebased.

Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

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

There are 8 warheads with sparky=yes in https://github.com/LipkeGu/TSIniFiles/blob/4cc5adac88b012e27a6a82ba7c93d182451d9d64/rules.ini , so I assume this is still missing from a few weapons (like the grenade).

Lgtm otherwise. ✅

OpenRA.Mods.Common/Traits/Conditions/SpreadsCondition.cs Outdated Show resolved Hide resolved
OpenRA.Mods.Common/Traits/Conditions/SpreadsCondition.cs Outdated Show resolved Hide resolved
@Mailaender
Copy link
Member Author

Added it to Orca, Disc Throwers and Cyborg Commando according to RULES.INI.

Copy link
Member

@abcdefg30 abcdefg30 left a comment

Choose a reason for hiding this comment

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

The SonicZap should also be able to affect trees.

mods/ts/weapons/missiles.yaml Outdated Show resolved Hide resolved
mods/ts/weapons/ballisticweapons.yaml Outdated Show resolved Hide resolved
@Mailaender
Copy link
Member Author

Updated.

abcdefg30
abcdefg30 previously approved these changes Feb 17, 2022
@Mailaender
Copy link
Member Author

Updated.

@abcdefg30 abcdefg30 merged commit 00356b8 into OpenRA:bleed Mar 12, 2022
@abcdefg30
Copy link
Member

Changelog

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.

Add Tiberian Sun forest fire/tree burning logic
6 participants