Add enter-cloak & exit-cloak effect for Cloak #19881

dnqbob · 2022-01-14T06:29:09Z

close #19880

OpenRA.Mods.Common/Traits/Cloak.cs

abcdefg30 · 2022-01-14T08:44:43Z

OpenRA.Mods.Common/Traits/Cloak.cs

@@ -167,16 +181,34 @@ void ITick.Tick(Actor self)
 					cloakedToken = self.GrantCondition(Info.CloakedCondition);

 				// Sounds shouldn't play if the actor starts cloaked
-				if (!(firstTick && Info.InitialDelay == 0) && !otherCloaks.Any(a => a.Cloaked))
+				if (!(firstTick && Info.InitialDelay == 0))


Suggested change

if (!(firstTick && Info.InitialDelay == 0))

if (!firstTick || Info.InitialDelay > 0)

OpenRA.Mods.Common/Traits/Cloak.cs

abcdefg30 · 2022-01-14T08:54:54Z

OpenRA.Mods.Common/Traits/Cloak.cs

@@ -63,6 +64,24 @@ public class CloakInfo : PausableConditionalTraitInfo
 		[Desc("The condition to grant to self while cloaked.")]
 		public readonly string CloakedCondition = null;

+		[Desc("Which image to use for effect when enter/exit cloak.")]


Suggested change

[Desc("Which image to use for effect when enter/exit cloak.")]

[Desc("Which image to use for the effect played when cloaking or uncloaking.")]

abcdefg30 · 2022-01-14T08:55:46Z

OpenRA.Mods.Common/Traits/Cloak.cs

+		[Desc("Which image to use for effect when enter/exit cloak.")]
+		public readonly string EffectImage = null;
+
+		[Desc("Which sequence to use for effect when enter cloak.")]


Suggested change

[Desc("Which sequence to use for effect when enter cloak.")]

[Desc("Which effect sequence to play when cloaking.")]

abcdefg30 · 2022-01-14T08:56:04Z

OpenRA.Mods.Common/Traits/Cloak.cs

+
+		[Desc("Which sequence to use for effect when enter cloak.")]
+		[SequenceReference(nameof(EffectImage), allowNullImage: true)]
+		public readonly string EnterEffectSequence = null;


Suggested change

public readonly string EnterEffectSequence = null;

public readonly string CloakEffectSequence = null;

abcdefg30 · 2022-01-14T08:56:27Z

OpenRA.Mods.Common/Traits/Cloak.cs

+		[SequenceReference(nameof(EffectImage), allowNullImage: true)]
+		public readonly string EnterEffectSequence = null;
+
+		[Desc("Which sequence to use for effect when exit cloak..")]


Suggested change

[Desc("Which sequence to use for effect when exit cloak..")]

[Desc("Which effect sequence to play when uncloaking.")]

abcdefg30 · 2022-01-14T09:00:35Z

OpenRA.Mods.Common/Traits/Cloak.cs

+
+		[Desc("Which sequence to use for effect when exit cloak..")]
+		[SequenceReference(nameof(EffectImage), allowNullImage: true)]
+		public readonly string ExitEffectSequence = null;


Suggested change

public readonly string ExitEffectSequence = null;

public readonly string UncloakEffectSequence = null;

abcdefg30 · 2022-01-14T09:17:12Z

OpenRA.Mods.Common/Traits/Cloak.cs

+		public readonly string EffectPalette = "effect";
+		public readonly bool IsEffectPlayerPalette = false;
+
+		[Desc("Offset for effect when enter/exit cloak.")]


Suggested change

[Desc("Offset for effect when enter/exit cloak.")]

[Desc("Offset for the effect played when cloaking or uncloaking.")]

abcdefg30 · 2022-01-14T09:21:10Z

OpenRA.Mods.Common/Traits/Cloak.cs

+
+		[PaletteReference(nameof(IsEffectPlayerPalette))]
+		public readonly string EffectPalette = "effect";
+		public readonly bool IsEffectPlayerPalette = false;


Suggested change

public readonly bool IsEffectPlayerPalette = false;

public readonly bool EffectPaletteIsPlayerPalette = false;

Actually this seems unused so please remove it.

third-party mod may need this on stealth effect

fix the spelling, not removed

OpenRA.Mods.Common/Traits/Cloak.cs

abcdefg30 · 2022-01-15T14:30:59Z

OpenRA.Mods.Common/Traits/Cloak.cs

+							palette += self.Owner.InternalName;
+
+						self.World.AddFrameEndTask(w => w.Add(new SpriteEffect(
+							pos + Info.EffectOffset, w, Info.EffectImage, Info.CloakEffectSequence, palette)));


Suggested change

pos + Info.EffectOffset, w, Info.EffectImage, Info.CloakEffectSequence, palette)));

pos + Info.EffectOffset, w, Info.EffectImage, Info.UncloakEffectSequence, palette)));

abcdefg30 · 2022-01-15T16:31:59Z

mods/ts/rules/nod-vehicles.yaml

 		PauseOnCondition: cloak-force-disabled || empdisable
+		CloakedCondition: cloak-self


Did you test what happens when you build a Stealth Tank within the radius of a Stealth Generator? I would expect it still plays the cloak sound with this setup. Thinking more about it we might need to restore the old checks, as I'm no longer sure this is doable using conditions.

Yes, it is still here. going restore the check but I will change it by getting the same cloak type on the unit.

ummm, I meet a issue when get the same cloak type. In fact it is cloak types, like targetables. could we make the "InitialDelay = 0" of TS stealth tank's Cloak to avoid this?

Still the example: submarine is under the effect of Stealth Generator.

Submerge is a kind of cloak, Stealth is a kind of cloak. In regular logic, being stealth and make the sound of steath field doesn't(or may not) prevent submarine make sound of go under the water.

The use case here is more complicated: only one of the two stealth generators can work. In orignal TS, if what I remeber is correct, the Stealth Tank doesn't recieve the buff from Stealth Generator: when it get heavily damaged, Stealth Generator cannot cloak it like other heavily damage vehicle. If we want to use the gameplay setting like orignal TS it is also simple to solve.

In conclusion, we have 3 ideas here:

make InitialDelay=0 for stealth tank cloak (I confirm it is working)

change back to TS orignal setting: Stealth Generator doesn't affect Stealth Tank

add Cloak something to identify the same cloak method, without using CloakTypes (or we use CloakTypes with a not so confusing rule), which may makes the code more confusing.

If you look at the yaml it makes perfect sense why they cloak twice, so I think it should also be solved via yaml. IMO in this case hardcoding the solution only limits modders

I can see 2 easy ways of fixing this

Stealth tank overwrites stealth generator stealth instead of being separate

Stealth generator stealth is disabled on stealth tank

And one feature creep way. Add a property on cloak called TriggerCloakOnConditions. That way you could disable external cloak on stealth tank, but add the external condition to stank cloak. The TriggerCloakOnConditions would ignore the internal cloak delay and cloak immediately when any of the set conditions become true cloakgenerator, crate-cloak. I'm not sure if we can set conditions this way though

What is the problem with using CloakTypes?

What is the problem with using CloakTypes?

CloakTypes cannot be used as a standard that tell OpenRA some Cloak traits belong to one kind of cloak, which have the same cloak&uncloak sound/visual effect and have some special rule when playing the sound/visual effect. For example, Stealth Tank cloak and Cloak Generator cloak.

if we want to solve this problem perfectly, we need another property in CloakInfo like CloakClass/CloakKind or something, restore the check but this check is only effective on the same CloakClass/CloakKind.

To make it more readable I want to rename CloakTypes into DetectableTypes, and make another CloakType to differ the same cloak.

To make it more readable I want to rename CloakTypes into DetectableTypes, and make another CloakType to differ the same cloak.

That sounds like a good idea. I suggest DetectionTypes (because they're used for detection, and the types themselves are not detectable).

And it is done

abcdefg30

Untested but looks good now in theory. Can you add an update rule for the CloakTypes -> DetectionTypes changes?

dnqbob · 2022-01-17T14:29:53Z

Untested but looks good now in theory. Can you add an update rule for the CloakTypes -> DetectionTypes changes?

I'd love to but I never did the same thing before so could you tell me how to use the rule system?

abcdefg30 · 2022-01-17T14:35:43Z

1fab49e is an example for how to do an update rule:
Create a new file called RenameCloakTypes.cs in the OpenRA.Mods.Common/UpdateRules/Rules/20201213 folder (20201213 because that was the first playtest after release-20200503). Then add an instance of that class to the // Bleed only changes here update path in UpdatePath.cs.
The content of the file can be similar to my example: Search for each Cloak trait, find the LastChildMatching CloakTypes and rename that node (there is a rename function). Then do the same for all DetectCloaked traits.

dnqbob · 2022-01-17T15:10:01Z

thanks, idk if now it is correct

dnqbob · 2022-01-18T02:14:44Z

Change the !(firstTick && Info.InitialDelay == 0) && (otherCloaks == null || !otherCloaks.Any(a => a.Cloaked)) into (otherCloaks == null || !otherCloaks.Any(a => a.Cloaked)) && !(firstTick && Info.InitialDelay == 0), in the hoping that more performance can be saved

Update: ummm acutally not

abcdefg30 · 2022-01-18T09:48:40Z

Update: ummm acutally not

Can you change it back? (Calling the Any before the rest will likely be less performant fwiw, although it won't matter much in practice.)

dnqbob · 2022-01-18T11:28:49Z

it is back now

abcdefg30

(You can remove the test cases from RA now; before we accidentally merge them. 😅)

dnqbob · 2022-01-20T21:29:57Z

(You can remove the test cases from RA now; before we accidentally merge them. 😅)

Which one, both?
I kinda want to keep them :(

Inq8 · 2022-01-22T08:49:05Z

Can this be expanded to grant a condition when cloaking and a condition when uncloaking?

pchote

One typo I spotted, not a proper review.

I second the request to remove the testcases, IMO the submarine splash in particular doesn't look good and may be confusing for players thinking that something is firing on it.

pchote · 2022-01-22T18:30:57Z

OpenRA.Mods.Common/Traits/Cloak.cs


 		[GrantedConditionReference]
 		[Desc("The condition to grant to self while cloaked.")]
 		public readonly string CloakedCondition = null;

+		[Desc("The type of cloak. Same type of cloaks won't tigger cloaking and uncloaking sound and effect.")]


How about the other usecase of stealth apc?

Fixed and dropped

reaperrr

Ok, though I'm not 100% sure about the RA STNK sparkle effect

reaperrr · 2022-01-30T11:15:18Z

Not merging yet since I don't know how others feel about the STNK sparkle effect.

dnqbob · 2022-02-03T05:08:22Z

Can this be expanded to grant a condition when cloaking and a condition when uncloaking?

Maybe not in this branch, but it is possible to make it grant a condition when cloaking and a condition when uncloaking

Smittytron · 2022-02-03T21:14:41Z

The animation doesn't follow a moving actor, is this intended?

abcdefg30 · 2022-02-03T22:34:29Z

How about the other usecase of stealth apc?

There hasn't been great support for this change on Discord, so I'd prefer if we can fix the animation not following actors and then remove the effect from the Phase Transport.

dnqbob · 2022-02-04T02:37:41Z

The animation doesn't follow a moving actor, is this intended?

Yes, considering usecase of submarine (appear and disappear with a water splash) and ninja (disappear with a smoke screen), the cloak effect of those should not follow the actor, but play at the effect triggered position instead.

However, now I make the user can set the follow or not.

There hasn't been great support for this change on Discord, so I'd prefer if we can fix the animation not following actors and then remove the effect from the Phase Transport.

Spark still not remove for you to test out and test the setting of follow/not follow function. Once it is done I will remove the testcase here.

abcdefg30

Tracking works. ✅

abcdefg30 · 2022-02-04T09:13:24Z

OpenRA.Mods.Common/Traits/Cloak.cs

+		public readonly WVec EffectOffset = WVec.Zero;
+
+		[Desc("Should the effect track the actor.")]
+		public readonly bool EffectTrackActor = true;


Suggested change

public readonly bool EffectTrackActor = true;

public readonly bool EffectTracksActor = true;

abcdefg30 · 2022-02-04T09:14:25Z

OpenRA.Mods.Common/Traits/Cloak.cs

+							palette += self.Owner.InternalName;
+
+						self.World.AddFrameEndTask(w => w.Add(new SpriteEffect(
+							posFunc: posfunc, facingFunc: () => WAngle.Zero, w, Info.EffectImage, Info.CloakEffectSequence, palette)));


Naming the parameters should not be necessary, or is it?

Suggested change

posFunc: posfunc, facingFunc: () => WAngle.Zero, w, Info.EffectImage, Info.CloakEffectSequence, palette)));

posfunc, () => WAngle.Zero, w, Info.EffectImage, Info.CloakEffectSequence, palette)));

Should be all fixed here and drop the test case

dnqbob · 2022-02-05T03:09:51Z

Still I hope there can be somewhere use the cloak effect, for maintaining unused code will be hard without an usecase

Mailaender · 2022-02-12T18:33:32Z

Changelog

abcdefg30 reviewed Jan 14, 2022

View reviewed changes

dnqbob force-pushed the cloak-effect branch from 68f8a5a to f1d42ad Compare January 15, 2022 13:09

abcdefg30 reviewed Jan 15, 2022

View reviewed changes

OpenRA.Mods.Common/Traits/Cloak.cs Show resolved Hide resolved

dnqbob force-pushed the cloak-effect branch 2 times, most recently from 8cb6815 to 1e37cc2 Compare January 15, 2022 14:28

abcdefg30 reviewed Jan 15, 2022

View reviewed changes

dnqbob force-pushed the cloak-effect branch from 1e37cc2 to 7d3c03c Compare January 15, 2022 14:33

dnqbob requested review from pchote and abcdefg30 January 15, 2022 15:10

abcdefg30 reviewed Jan 15, 2022

View reviewed changes

dnqbob force-pushed the cloak-effect branch from e9c5f47 to acf1206 Compare January 17, 2022 14:08

dnqbob requested a review from abcdefg30 January 17, 2022 14:14

dnqbob force-pushed the cloak-effect branch from acf1206 to 585a1e6 Compare January 17, 2022 14:18

abcdefg30 reviewed Jan 17, 2022

View reviewed changes

dnqbob requested a review from abcdefg30 January 17, 2022 15:10

dnqbob force-pushed the cloak-effect branch from b0fb2b8 to 70c1c20 Compare January 18, 2022 02:05

dnqbob force-pushed the cloak-effect branch from 70c1c20 to 850ab16 Compare January 18, 2022 09:51

abcdefg30 previously approved these changes Jan 20, 2022

View reviewed changes

abcdefg30 added the PR: Needs +2 label Jan 20, 2022

pchote reviewed Jan 22, 2022

View reviewed changes

dnqbob dismissed abcdefg30’s stale review via a0ad610 January 23, 2022 10:28

dnqbob force-pushed the cloak-effect branch from 850ab16 to a0ad610 Compare January 23, 2022 10:28

dnqbob force-pushed the cloak-effect branch from a0ad610 to e4dd92c Compare January 23, 2022 10:29

dnqbob requested review from pchote and abcdefg30 January 23, 2022 10:30

reaperrr previously approved these changes Jan 30, 2022

View reviewed changes

dnqbob dismissed reaperrr’s stale review via 2283e4d February 4, 2022 02:28

dnqbob force-pushed the cloak-effect branch from e4dd92c to 2283e4d Compare February 4, 2022 02:28

abcdefg30 previously approved these changes Feb 4, 2022

View reviewed changes

dnqbob and others added 2 commits February 4, 2022 17:33

Add enter-cloak & exit-cloak effect for Cloak

da9abdf

Update rule on renaming 'CloakTypes' to 'DetectionTypes'

1c30253

dnqbob dismissed abcdefg30’s stale review via 1c30253 February 4, 2022 09:42

dnqbob force-pushed the cloak-effect branch from 2283e4d to 1c30253 Compare February 4, 2022 09:42

dnqbob requested review from abcdefg30 and reaperrr February 4, 2022 10:14

abcdefg30 approved these changes Feb 4, 2022

View reviewed changes

Mailaender merged commit 6c33d47 into OpenRA:bleed Feb 12, 2022

dnqbob deleted the cloak-effect branch February 13, 2022 00:22

PunkPun mentioned this pull request Jul 7, 2022

Cloak effects lacks visual transition #12195

Open

3 tasks

	if (!(firstTick && Info.InitialDelay == 0))
	if (!firstTick \|\| Info.InitialDelay > 0)

	[Desc("Which image to use for effect when enter/exit cloak.")]
	[Desc("Which image to use for the effect played when cloaking or uncloaking.")]

	[Desc("Which sequence to use for effect when enter cloak.")]
	[Desc("Which effect sequence to play when cloaking.")]

	public readonly string EnterEffectSequence = null;
	public readonly string CloakEffectSequence = null;

	public readonly string ExitEffectSequence = null;
	public readonly string UncloakEffectSequence = null;

	[Desc("Offset for effect when enter/exit cloak.")]
	[Desc("Offset for the effect played when cloaking or uncloaking.")]

	public readonly bool IsEffectPlayerPalette = false;
	public readonly bool EffectPaletteIsPlayerPalette = false;

	pos + Info.EffectOffset, w, Info.EffectImage, Info.CloakEffectSequence, palette)));
	pos + Info.EffectOffset, w, Info.EffectImage, Info.UncloakEffectSequence, palette)));

		PauseOnCondition: cloak-force-disabled \|\| empdisable
		CloakedCondition: cloak-self

	public readonly bool EffectTrackActor = true;
	public readonly bool EffectTracksActor = true;

	posFunc: posfunc, facingFunc: () => WAngle.Zero, w, Info.EffectImage, Info.CloakEffectSequence, palette)));
	posfunc, () => WAngle.Zero, w, Info.EffectImage, Info.CloakEffectSequence, palette)));

Add enter-cloak & exit-cloak effect for Cloak #19881

Add enter-cloak & exit-cloak effect for Cloak #19881

Conversation

dnqbob commented Jan 14, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnqbob Jan 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

PunkPun Jan 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnqbob Jan 16, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abcdefg30 left a comment

Choose a reason for hiding this comment

dnqbob commented Jan 17, 2022

abcdefg30 commented Jan 17, 2022

dnqbob commented Jan 17, 2022

dnqbob commented Jan 18, 2022 • edited Loading

abcdefg30 commented Jan 18, 2022

dnqbob commented Jan 18, 2022

abcdefg30 left a comment

Choose a reason for hiding this comment

dnqbob commented Jan 20, 2022

Inq8 commented Jan 22, 2022

pchote left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reaperrr left a comment

Choose a reason for hiding this comment

reaperrr commented Jan 30, 2022

dnqbob commented Feb 3, 2022

Smittytron commented Feb 3, 2022

abcdefg30 commented Feb 3, 2022

dnqbob commented Feb 4, 2022 • edited Loading

abcdefg30 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dnqbob commented Feb 5, 2022

Mailaender commented Feb 12, 2022

dnqbob Jan 16, 2022 •

edited

Loading

PunkPun Jan 16, 2022 •

edited

Loading

dnqbob Jan 16, 2022 •

edited

Loading

dnqbob commented Jan 18, 2022 •

edited

Loading

dnqbob commented Feb 4, 2022 •

edited

Loading