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

Replace bogus ROF values in missions #14317

Merged
merged 1 commit into from Dec 7, 2017

Conversation

Projects
None yet
5 participants
@abcdefg30
Member

abcdefg30 commented Nov 8, 2017

Fixes #14313.

The reason for the change in MTM from 70 to 1 is that the intro sequence was balanced around the fact that ROF: 70 was bogus and thus falling back to the default value of 1. (This was then "disturbed" by #13543 which let the SuperTankPrimary inherit ^Cannon with another ReloadDelay value, hence I need to restate the default value again.)

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Nov 9, 2017

Contributor

the intro sequence was balanced around the fact that ROF: 70 was bogus

In my opinion this is what should really be fixed, although it might be better to defer the proper fix to Next+1.

Contributor

reaperrr commented Nov 9, 2017

the intro sequence was balanced around the fact that ROF: 70 was bogus

In my opinion this is what should really be fixed, although it might be better to defer the proper fix to Next+1.

@reaperrr reaperrr added this to the Next release milestone Nov 9, 2017

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Nov 17, 2017

Contributor

Since we're now leaning towards a playtest instead of a bugfix release, I'd prefer we give the super tanks a sane ReloadDelay and rebalance damage instead.

Contributor

reaperrr commented Nov 17, 2017

Since we're now leaning towards a playtest instead of a bugfix release, I'd prefer we give the super tanks a sane ReloadDelay and rebalance damage instead.

@Arular101

This comment has been minimized.

Show comment
Hide comment
@Arular101

Arular101 Nov 19, 2017

Contributor

Please, would you mind to also remove some redundant overrides?

In Survival 02:
Report: chute1.aud

In Monster Tank Madness:
TurretGun: Projectile: Blockable: false This was probably used to shoot over sandbags.

Edit: see comment below.

For SuperTankPrimary:
Range: 4c768
InvalidTargets: Air, Infantry *
Projectile: Bullet
Speed: 682
Image: 120MM
Spread: 128
InvalidTargets: Air, Infantry *
DamageTypes: Prone50Percent, TriggerProne, ExplosionDeath
Warhead@2Smu: LeaveSmudge
SmudgeType: Crater
Warhead@3Eff: CreateEffect
Explosions: small_explosion
Warhead@4EffWater: CreateEffect
Explosions: small_splash

These are all inherited form Cannon.

I only noticed one problem. The InvalidTargets: Air, Infantry is strange. It was removed in this PR #12348, but it is back again here and in ballistics.yaml line 71 and line 76. Should I open an issue ticket and or PR?

Contributor

Arular101 commented Nov 19, 2017

Please, would you mind to also remove some redundant overrides?

In Survival 02:
Report: chute1.aud

In Monster Tank Madness:
TurretGun: Projectile: Blockable: false This was probably used to shoot over sandbags.

Edit: see comment below.

For SuperTankPrimary:
Range: 4c768
InvalidTargets: Air, Infantry *
Projectile: Bullet
Speed: 682
Image: 120MM
Spread: 128
InvalidTargets: Air, Infantry *
DamageTypes: Prone50Percent, TriggerProne, ExplosionDeath
Warhead@2Smu: LeaveSmudge
SmudgeType: Crater
Warhead@3Eff: CreateEffect
Explosions: small_explosion
Warhead@4EffWater: CreateEffect
Explosions: small_splash

These are all inherited form Cannon.

I only noticed one problem. The InvalidTargets: Air, Infantry is strange. It was removed in this PR #12348, but it is back again here and in ballistics.yaml line 71 and line 76. Should I open an issue ticket and or PR?

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Nov 19, 2017

Contributor

The InvalidTargets: Air, Infantry is strange. It was removed in this PR #12348, but it is back again here and in ballistics.yaml line 71 and line 76. Should I open an issue ticket and or PR?

ballistics.yaml looks correct to me. #12348 only made the warhead valid against infantry so they'll recieve splash damage when hit by accident.

I agree that the weapon should only override what's necessary though, maybe make it inherit 120mm instead of ^Cannon to minimize the overrides.

Contributor

reaperrr commented Nov 19, 2017

The InvalidTargets: Air, Infantry is strange. It was removed in this PR #12348, but it is back again here and in ballistics.yaml line 71 and line 76. Should I open an issue ticket and or PR?

ballistics.yaml looks correct to me. #12348 only made the warhead valid against infantry so they'll recieve splash damage when hit by accident.

I agree that the weapon should only override what's necessary though, maybe make it inherit 120mm instead of ^Cannon to minimize the overrides.

@Arular101

This comment has been minimized.

Show comment
Hide comment
@Arular101

Arular101 Nov 19, 2017

Contributor

Ah, thanks for the clarification. In that case, the SuperTankPrimary could be replaced with this (also in the rules.yaml):
120mm:
ReloadDelay: 6
Report: turret1.aud

A ReloadDelay less than 6 will result in that only one barrel of the "Mammoth tank" is used. 6 is the lowest value where it uses two barrels.
I also think that the intention of the tank is to have a high as possible rate of fire. Because the tanks needs to destroy the enemy base as fast as possible. It has only 3 minutes before it self-destructs. With a slower rate of fire it wouldn't reach the second enemy base. An other possible change is to increase the damage to compensate the slower rate of fire.

Edit: intro sequence is fine with ReloadDelay: 6.

Contributor

Arular101 commented Nov 19, 2017

Ah, thanks for the clarification. In that case, the SuperTankPrimary could be replaced with this (also in the rules.yaml):
120mm:
ReloadDelay: 6
Report: turret1.aud

A ReloadDelay less than 6 will result in that only one barrel of the "Mammoth tank" is used. 6 is the lowest value where it uses two barrels.
I also think that the intention of the tank is to have a high as possible rate of fire. Because the tanks needs to destroy the enemy base as fast as possible. It has only 3 minutes before it self-destructs. With a slower rate of fire it wouldn't reach the second enemy base. An other possible change is to increase the damage to compensate the slower rate of fire.

Edit: intro sequence is fine with ReloadDelay: 6.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Dec 2, 2017

Contributor

@Arular101 could you test it with

SuperTankPrimary:
	Inherits: 120mm
	ReloadDelay: 70
	Report: turret1.aud
	Warhead@1Dam: SpreadDamage
		Damage: 500

and report back?

Contributor

reaperrr commented Dec 2, 2017

@Arular101 could you test it with

SuperTankPrimary:
	Inherits: 120mm
	ReloadDelay: 70
	Report: turret1.aud
	Warhead@1Dam: SpreadDamage
		Damage: 500

and report back?

@Arular101

This comment has been minimized.

Show comment
Hide comment
@Arular101

Arular101 Dec 2, 2017

Contributor

@reaperrr I tested the mission and these values look good. I tested it in three variations:
First one is with the Super tanks at the village in the south. They destroyed the southern base, and had just enough time to reach the northern base to destroy some structures.
Second time is with the Super tanks near the hospital in the middle of the map. They also destroyed the southern base and destroyed some structures in the northern base.
Third time is with the Super tanks near the players base. The Super tanks went first to the northern base, and had just enough time to reach the southern base to destroy some structures.

Contributor

Arular101 commented Dec 2, 2017

@reaperrr I tested the mission and these values look good. I tested it in three variations:
First one is with the Super tanks at the village in the south. They destroyed the southern base, and had just enough time to reach the northern base to destroy some structures.
Second time is with the Super tanks near the hospital in the middle of the map. They also destroyed the southern base and destroyed some structures in the northern base.
Third time is with the Super tanks near the players base. The Super tanks went first to the northern base, and had just enough time to reach the southern base to destroy some structures.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Dec 5, 2017

Member

Updated.

Member

abcdefg30 commented Dec 5, 2017

Updated.

@reaperrr

This comment has been minimized.

Show comment
Hide comment
@reaperrr

reaperrr Dec 5, 2017

Contributor

Those minor nits aren't blockers, but ideally they should also be adressed before this is merged.

Contributor

reaperrr commented Dec 5, 2017

Those minor nits aren't blockers, but ideally they should also be adressed before this is merged.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Dec 6, 2017

Member

Updated.

Member

abcdefg30 commented Dec 6, 2017

Updated.

@penev92

penev92 approved these changes Dec 7, 2017

I count 2 approving votes, so merging (I trust that @Smittytron is perfectly capable of reviewing this).

@penev92 penev92 merged commit 6dd42f7 into OpenRA:bleed Dec 7, 2017

2 checks passed

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

This comment has been minimized.

Show comment
Hide comment
@penev92
Member

penev92 commented Dec 7, 2017

@abcdefg30 abcdefg30 deleted the abcdefg30:rof branch Dec 7, 2017

@Arular101

This comment has been minimized.

Show comment
Hide comment
@Arular101

Arular101 Dec 9, 2017

Contributor

Ahh, the redundant overrides from my first post weren't removed.

Contributor

Arular101 commented Dec 9, 2017

Ahh, the redundant overrides from my first post weren't removed.

@abcdefg30

This comment has been minimized.

Show comment
Hide comment
@abcdefg30

abcdefg30 Dec 9, 2017

Member

Ops, sorry. Seems like I missed that during all this back and forth.

Member

abcdefg30 commented Dec 9, 2017

Ops, sorry. Seems like I missed that during all this back and forth.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment