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

Implement GrantPrerequisiteChargeDrainPower logic #16347

Merged
merged 5 commits into from Nov 17, 2019

Conversation

@pchote
Copy link
Member

pchote commented Mar 23, 2019

This PR generalizes the the support power logic to give implementations more flexibility in overriding their charging and targeting behaviour, and adds two new traits:

  • GrantPrerequisiteChargeDrainPower: Grants a prerequisite when activated, and then counts backwards at a rate controlled by DischargeModifier until the timer reaches 0. The player can deactivate the power before it runs out by selecting it again, and the timer will recharge from the intermediate point. If the power was disabled manually (i.e. did not completely run out) then the player can reactivate it at any time.

  • DrainPrerequisitePowerOnDamage: If enabled, any damage recieved by this actor will be converted to a time using DamageMultiplier and DamageDivisor which will be subtracted from the GrantPrerequisiteChargeDrainPower with matching OrderName.

These traits implement the special case behaviour for the TS Firestorm Defense support power described by UseChargeDrain, ChargeToDrainRatio, FirestormWall, and DamageToFirestormDamageCoefficient.

This is the first of three PRs that will resolve #10789. The second PR will overhaul projectile blocking and effects before a third PR implements the Fire Storm Generator / Firestorm Wall / Firestorm Defense properly in TS.

The testcase demonstrates its use on the RA Iron Curtain.

@pchote pchote force-pushed the pchote:discharge-supportpower branch from 9c7b028 to 8c100b9 Mar 24, 2019
@ghost

This comment has been minimized.

Copy link

ghost commented Mar 24, 2019

Looks really interesting! Found a crash when you try to demolish something while the power is active:

OpenRA engine version {DEV_VERSION}
Red Alert mod version {DEV_VERSION}
on map 6bbf6ff670d9f19ccbb380e475df6c5129477ddb (Pie of Animosity by Janitor).
Date: 2019-03-24 19:57:10Z
Operating System: Linux (Unix 4.15.0.46)
Runtime Version: Mono 5.18.1.0 (tarball Fri Mar 15 20:41:32 UTC 2019) CLR 4.0.30319.42000
Exception of type `System.NullReferenceException`: Object reference not set to an instance of an object
  at OpenRA.Mods.Cnc.Traits.DrainPrerequisitePowerOnDamage.OpenRA.Mods.Common.Traits.IDamageModifier.GetDamageModifier (OpenRA.Actor self, OpenRA.Traits.Damage damage) [0x0000d] in <fdcc358eb2bd4b99bf0601f870d6f4a3>:0 
  at OpenRA.Mods.Common.Traits.Demolishable+<OpenRA_Traits_ITick_Tick>c__AnonStorey0.<>m__0 (OpenRA.Mods.Common.Traits.IDamageModifier t) [0x00000] in <9a40f0e9252a4872a80d696c3cefc9f5>:0 
  at System.Linq.Enumerable+SelectEnumerableIterator`2[TSource,TResult].MoveNext () [0x00048] in <e32d3889c5e24db788c761b532f9d8a8>:0 
  at OpenRA.Mods.Common.Util.ApplyPercentageModifiers (System.Int32 number, System.Collections.Generic.IEnumerable`1[T] percentages) [0x00035] in <9a40f0e9252a4872a80d696c3cefc9f5>:0 
  at OpenRA.Mods.Common.Traits.Demolishable.OpenRA.Traits.ITick.Tick (OpenRA.Actor self) [0x00089] in <9a40f0e9252a4872a80d696c3cefc9f5>:0 
  at OpenRA.World.<Tick>m__4 (OpenRA.TraitPair`1[T] x) [0x00000] in <f53580216b274bd8afd1c4c9ded93f38>:0 
  at (wrapper delegate-invoke) System.Action`1[OpenRA.TraitPair`1[OpenRA.Traits.ITick]].invoke_void_T(OpenRA.TraitPair`1<OpenRA.Traits.ITick>)
  at OpenRA.WorldUtils.DoTimed[T] (System.Collections.Generic.IEnumerable`1[T] e, System.Action`1[T] a, System.String text) [0x00022] in <f53580216b274bd8afd1c4c9ded93f38>:0 
  at OpenRA.World.Tick () [0x00084] in <f53580216b274bd8afd1c4c9ded93f38>:0 
  at OpenRA.Game.InnerLogicTick (OpenRA.Network.OrderManager orderManager) [0x00237] in <f53580216b274bd8afd1c4c9ded93f38>:0 
  at OpenRA.Game.LogicTick () [0x00050] in <f53580216b274bd8afd1c4c9ded93f38>:0 
  at OpenRA.Game.Loop () [0x000d6] in <f53580216b274bd8afd1c4c9ded93f38>:0 
  at OpenRA.Game.Run () [0x00042] in <f53580216b274bd8afd1c4c9ded93f38>:0 
  at OpenRA.Game.InitializeAndRun (System.String[] args) [0x00011] in <f53580216b274bd8afd1c4c9ded93f38>:0 
  at OpenRA.Program.Main (System.String[] args) [0x0004f] in <f53580216b274bd8afd1c4c9ded93f38>:0 
@pchote pchote force-pushed the pchote:discharge-supportpower branch from 8c100b9 to 6eaabf5 Mar 29, 2019
@pchote pchote force-pushed the pchote:discharge-supportpower branch from 6eaabf5 to 291541d Mar 29, 2019
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Mar 29, 2019

Rebased and fixed. It turns out that this wasn't the only trait that suffered from that crash, so I added a new commit to fix all of them.

@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Apr 21, 2019

Ping @OpenRA/engine-hackers. This is blocking the rest of the firestorm defense logic.

mods/ra/rules/defaults.yaml Outdated Show resolved Hide resolved
@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Apr 22, 2019

Could also use a rebase (while there are no conflicts, we had dependency changes and the dll.config fix in the meantime).

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Apr 22, 2019

Looks good to me once the year numbers are fixed and the other comment is adressed.

Copy link

ghost left a comment

Can not find any issues, 👍 when @reaperrr is happy.

@pchote pchote added this to the Future milestone May 23, 2019
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented May 23, 2019

With our current lack of review bandwidth this is too low priority to consider just now. I'll revisit this once things have settled down.

@pchote pchote closed this May 23, 2019
@pchote pchote reopened this Aug 26, 2019
@pchote pchote modified the milestones: Future, Next+1 Aug 26, 2019
@pchote pchote force-pushed the pchote:discharge-supportpower branch from 291541d to 78cbfa9 Aug 26, 2019
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Aug 26, 2019

Fixed and rebased.

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Aug 26, 2019

OpenRA.Utility(1,1): Error: ra|rules/structures.yaml:437 refers to a trait field `Condition` that does not exist on `GrantPrerequisiteChargeDrainPower`.
OpenRA.Utility(1,1): Error: ra|rules/structures.yaml:438 refers to a trait field `OnFireSound` that does not exist on `GrantPrerequisiteChargeDrainPower`.
@pchote pchote force-pushed the pchote:discharge-supportpower branch from 78cbfa9 to 2882ce8 Aug 27, 2019
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Aug 27, 2019

Fixed.

Copy link
Contributor

teinarss left a comment

Otherwise i did look good.

{
if (!IsTraitDisabled && damage != null)
{
var damageSubTicks = (int)(damage.Value * 100L * Info.DamageMultiplier / Info.DamageDivisor);

This comment has been minimized.

Copy link
@teinarss

teinarss Aug 28, 2019

Contributor

Guard for overflow?

This comment has been minimized.

Copy link
@pchote

pchote Nov 17, 2019

Author Member

The overflow is already guarded by casting to a long with the 100L.

notifiedCharging = false;

// Fully depleting the charge disables the power until it is again fully charged
if (!Active || remainingSubTicks >= TotalTicks * 100)

This comment has been minimized.

Copy link
@teinarss

teinarss Aug 28, 2019

Contributor

TotalTicks * 100
Not sure if its worth to extract this to a property and use it here and the two places below.

This comment has been minimized.

Copy link
@pchote

pchote Nov 17, 2019

Author Member

IMO not worth it.

Key = key;
TotalTicks = info.ChargeInterval;
remainingSubTicks = info.StartFullyCharged ? 0 : TotalTicks * 100;

This comment has been minimized.

Copy link
@teinarss

teinarss Aug 28, 2019

Contributor

TotalTicks * 100
Possible here too

@reaperrr

This comment has been minimized.

Copy link
Contributor

reaperrr commented Nov 17, 2019

Still looks good to me.
Low priority or not, after @teinarss' comments are adressed, testcase is removed and a rebase, LGTM.

@pchote pchote force-pushed the pchote:discharge-supportpower branch from 2882ce8 to 9409d47 Nov 17, 2019
@pchote

This comment has been minimized.

Copy link
Member Author

pchote commented Nov 17, 2019

Fixed/commented, testcase removed, rebased.

@reaperrr reaperrr merged commit f39b688 into OpenRA:bleed Nov 17, 2019
2 checks passed
2 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@pchote pchote mentioned this pull request Nov 17, 2019
2 of 5 tasks complete
@pchote pchote deleted the pchote:discharge-supportpower branch Jan 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants
You can’t perform that action at this time.