Skip to content

implements flashing on healing units#21269

Merged
PunkPun merged 1 commit into
OpenRA:bleedfrom
CDVoidwalker:flash-effect
Dec 31, 2023
Merged

implements flashing on healing units#21269
PunkPun merged 1 commit into
OpenRA:bleedfrom
CDVoidwalker:flash-effect

Conversation

@CDVoidwalker
Copy link
Copy Markdown
Contributor

Implements warhead that applies flashing effect to units in given area or target depending on properties. My only concern is that I was not sure which approach for area is correct and consistent with the project. It could very well be 2 different warheads with a lot of overlap (FlashesTargetsInRadius / FlashesTarget ) or this odd approach with radius that may be confusing for users.

Fixes #21068

Comment thread mods/ts/weapons/healweapons.yaml Outdated
Warhead@1Dam: TargetDamage
ValidTargets: Repair
Warhead@2Eff: FlashTargetsInRadius
ValidTargets: Vehicle
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These don't seem right. Should they have matching ValidTargets?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Not really, unless I reorder the warheads. Repair is condition based target for units with less than full hp. If first warhead heals to full then the second will not be valid

ValidTargets: Heal
Warhead@2Eff: FlashTargetsInRadius
ValidTargets: Infantry
ValidRelationships: Ally
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

shouldn't the heal warhead then only heal allies as well?

Copy link
Copy Markdown
Contributor Author

@CDVoidwalker CDVoidwalker Dec 31, 2023

Choose a reason for hiding this comment

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

armaments have TargetRelationships: Ally so it's fine

edit: added validRelationships for consistency with RA


namespace OpenRA.Mods.Common.Warheads
{
[Desc("Used to trigger a FlashPostProcessEffect trait on the world actor.")]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
[Desc("Used to trigger a FlashPostProcessEffect trait on the world actor.")]
[Desc("Trigger a flash effect on the targeted actor, or actors within a circle.")]

Was this a placeholder until you were clear on if the area effect should be split? If so, it can't hurt to have a comment as a reminder.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah I forgot about that part, thanks

Copy link
Copy Markdown
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

LGTM

@PunkPun PunkPun merged commit da638a4 into OpenRA:bleed Dec 31, 2023
@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Dec 31, 2023

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.

Units should blink when being healed by a medic

3 participants