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 devastator self destruct upon deployment #13884

Merged
merged 2 commits into from Oct 6, 2017

Conversation

Projects
None yet
7 participants
@Mailaender
Member

Mailaender commented Aug 21, 2017

This time I added the atomic reactor overload with just conditional traits.

Closes #4926.

@MustaphaTR

This comment has been minimized.

Show comment
Hide comment
@MustaphaTR

MustaphaTR Aug 21, 2017

Member

Not tested, but looking from the code, this wouldn't work as in original. In original;

a) Devastator that is being self destruct don't take any actual damage.

b) When it is killed by an external source rather than overcharge being completed, Devastator don't explode.

a can be solved by replacing negative SelfHeal with KillsSelf with a delay, not sure about b tho.

Member

MustaphaTR commented Aug 21, 2017

Not tested, but looking from the code, this wouldn't work as in original. In original;

a) Devastator that is being self destruct don't take any actual damage.

b) When it is killed by an external source rather than overcharge being completed, Devastator don't explode.

a can be solved by replacing negative SelfHeal with KillsSelf with a delay, not sure about b tho.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Aug 21, 2017

Member

This is kind of on purpose not faithful to the original, because the feature seemed useless if you need a charge-up sequence to complete in the heat of a battle. Tried it at first, but it didn't feel right from a technical point as well if you want re-usable conditional traits instead of hard-coded one purpose ones. See also #4926 (comment).

Member

Mailaender commented Aug 21, 2017

This is kind of on purpose not faithful to the original, because the feature seemed useless if you need a charge-up sequence to complete in the heat of a battle. Tried it at first, but it didn't feel right from a technical point as well if you want re-usable conditional traits instead of hard-coded one purpose ones. See also #4926 (comment).

@ltem

This comment has been minimized.

Show comment
Hide comment
@ltem

ltem Aug 21, 2017

Contributor

Some animations seem to be a bit off.

-The explosion animation (Link) starts in the middle of an explosion, which seems to be the wrong one.
-A strange overlay (This here Link) (seems to be a projectile (rocket)) is hovering permanently over devastator.

However, finally my awful screencap and gif-conversion skills pay off and I can show the explosion animation.

optimized_slow
Contributor

ltem commented Aug 21, 2017

Some animations seem to be a bit off.

-The explosion animation (Link) starts in the middle of an explosion, which seems to be the wrong one.
-A strange overlay (This here Link) (seems to be a projectile (rocket)) is hovering permanently over devastator.

However, finally my awful screencap and gif-conversion skills pay off and I can show the explosion animation.

optimized_slow
@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Aug 21, 2017

Member

Is the blue circle hovering over the mouse from the recording software or actually from the original game?

Member

Mailaender commented Aug 21, 2017

Is the blue circle hovering over the mouse from the recording software or actually from the original game?

@ltem

This comment has been minimized.

Show comment
Hide comment
@ltem

ltem Aug 21, 2017

Contributor

No, this is a Virtualbox Linux-Host Windows-Guest issue. I didn't get rid of it and it only appears for Dune 2000.

The charging animation could be starting at 3839 and is ~14 frames long.
The explosion animation seems to start at 3947 and is ~17 frames long

Contributor

ltem commented Aug 21, 2017

No, this is a Virtualbox Linux-Host Windows-Guest issue. I didn't get rid of it and it only appears for Dune 2000.

The charging animation could be starting at 3839 and is ~14 frames long.
The explosion animation seems to start at 3947 and is ~17 frames long

@GraionDilach

This comment has been minimized.

Show comment
Hide comment
@GraionDilach

GraionDilach Aug 21, 2017

Contributor

SelfHealing/KillsSelf having a damagetype with that damagetype triggering the new explosion would resolve @MustaphaTR's point B.

Contributor

GraionDilach commented Aug 21, 2017

SelfHealing/KillsSelf having a damagetype with that damagetype triggering the new explosion would resolve @MustaphaTR's point B.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Aug 21, 2017

Member

Updated with proper sequences (thanks!) and fixed the issue where the overlay was always displayed.

Member

Mailaender commented Aug 21, 2017

Updated with proper sequences (thanks!) and fixed the issue where the overlay was always displayed.

@ltem

This comment has been minimized.

Show comment
Hide comment
@ltem

ltem Sep 2, 2017

Contributor

The "overload" animation isn't complete but I don't know which part is missing and counting/comparing frames didn't work out.

Original OpenRA (without explosion)
devastatorboom2 devastatorboom_openra2

I 'm not 100% sure but it could be a combination of 3839 (~14 frames) and 4152 (~12 frames) and they run at the same time, overlaying each other.

Any opinion if my oberservation is right and if yes how this could be implemented?

Second, which behavior do we want to implement? I lean towards the original (deploy activates countdown to explosion) because the new one seems for me a bit too strong.

Contributor

ltem commented Sep 2, 2017

The "overload" animation isn't complete but I don't know which part is missing and counting/comparing frames didn't work out.

Original OpenRA (without explosion)
devastatorboom2 devastatorboom_openra2

I 'm not 100% sure but it could be a combination of 3839 (~14 frames) and 4152 (~12 frames) and they run at the same time, overlaying each other.

Any opinion if my oberservation is right and if yes how this could be implemented?

Second, which behavior do we want to implement? I lean towards the original (deploy activates countdown to explosion) because the new one seems for me a bit too strong.

@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Sep 2, 2017

Member

deploy activates countdown to explosion

I went with the original mechanic due to popular demand now.

Member

Mailaender commented Sep 2, 2017

deploy activates countdown to explosion

I went with the original mechanic due to popular demand now.

@ltem

This comment has been minimized.

Show comment
Hide comment
@ltem

ltem Sep 8, 2017

Contributor

So I think I found the other sequence (see below) and played a bit witht the Tick-duration.
Additional I added a condition to make the tank immobile during the overload (it's like that in the original). If this suggestions are accepted as correct and the other requests fixed, I would approve this PR.

Original OpenRA 1 Overlay (old) OpenRA 2 Overlays (new)
devastatorboom2 devastatorboom_openra2 devastatorboom_openra4_async
diff --git a/mods/d2k/rules/vehicles.yaml b/mods/d2k/rules/vehicles.yaml
@@ -316,6 +316,7 @@ devastator:
                TurnSpeed: 3
                Speed: 31
                Crushes: crate, infantry, spicebloom, wall
+               RequiresCondition: !overload
        RevealsShroud:
                Range: 4c768
        Armament:
@@ -341,6 +342,9 @@ devastator:
        WithIdleOverlay@OVERLOAD
                Sequence: active
                RequiresCondition: overload
+       WithIdleOverlay@OVERLOAD_2
+               Sequence: active_2
+               RequiresCondition: overload
        KillsSelf@MELTDOWN:
                Delay: 240
                RequiresCondition: overload

diff --git a/mods/d2k/sequences/vehicles.yaml b/mods/d2k/sequences/vehicles.yaml
@@ -202,7 +202,12 @@ devastator:
        active: DATA.R8
                Start: 3839
                Length: 14
-               Tick: 120
+               Tick: 130
+               BlendMode: Additive
+       active_2: DATA.R8
+               Start: 4152
+               Length: 12
+               Tick: 110
                BlendMode: Additive
        icon: DATA.R8
                Start: 4289
Contributor

ltem commented Sep 8, 2017

So I think I found the other sequence (see below) and played a bit witht the Tick-duration.
Additional I added a condition to make the tank immobile during the overload (it's like that in the original). If this suggestions are accepted as correct and the other requests fixed, I would approve this PR.

Original OpenRA 1 Overlay (old) OpenRA 2 Overlays (new)
devastatorboom2 devastatorboom_openra2 devastatorboom_openra4_async
diff --git a/mods/d2k/rules/vehicles.yaml b/mods/d2k/rules/vehicles.yaml
@@ -316,6 +316,7 @@ devastator:
                TurnSpeed: 3
                Speed: 31
                Crushes: crate, infantry, spicebloom, wall
+               RequiresCondition: !overload
        RevealsShroud:
                Range: 4c768
        Armament:
@@ -341,6 +342,9 @@ devastator:
        WithIdleOverlay@OVERLOAD
                Sequence: active
                RequiresCondition: overload
+       WithIdleOverlay@OVERLOAD_2
+               Sequence: active_2
+               RequiresCondition: overload
        KillsSelf@MELTDOWN:
                Delay: 240
                RequiresCondition: overload

diff --git a/mods/d2k/sequences/vehicles.yaml b/mods/d2k/sequences/vehicles.yaml
@@ -202,7 +202,12 @@ devastator:
        active: DATA.R8
                Start: 3839
                Length: 14
-               Tick: 120
+               Tick: 130
+               BlendMode: Additive
+       active_2: DATA.R8
+               Start: 4152
+               Length: 12
+               Tick: 110
                BlendMode: Additive
        icon: DATA.R8
                Start: 4289
@Mailaender

This comment has been minimized.

Show comment
Hide comment
@Mailaender

Mailaender Sep 17, 2017

Member

Updated with both @ltem's patch and @pchote's suggestion.

Member

Mailaender commented Sep 17, 2017

Updated with both @ltem's patch and @pchote's suggestion.

@ltem

ltem approved these changes Sep 21, 2017 edited

One small nit, everything else is fine 👍

Show outdated Hide outdated mods/d2k/chrome/ingame-player.yaml

@reaperrr reaperrr merged commit d64a9e6 into OpenRA:bleed Oct 6, 2017

2 checks passed

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

@Mailaender Mailaender deleted the Mailaender:devastator-overload branch Oct 6, 2017

@penev92 penev92 referenced this pull request Oct 22, 2017

Open

Finalize the D2k mod #7751

22 of 42 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment