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

Multiply damage and HP by 100 for RA #14301

Merged
merged 5 commits into from
Dec 12, 2017
Merged

Conversation

Arular101
Copy link
Contributor

@Arular101 Arular101 commented Nov 6, 2017

I multiplied all damage and HP by 100. See this issue #11719. I calculated everything in a spreadsheet to find out all the rounding errors it had, and adjusted it so there are little to no differences. I also changed the repair and selfheal values. Infantry with veterancy are now actually dealing more damage.

Here is an example of an easy change:
In ballistics.yaml the cannon has a damage of 40. Armor types affect the damage by a certain percentage. In this case, 30% for none, 75% for wood and light, and 50% for concrete armor. Calculating this has all rounded values. (40 x 0.3 = 12 | 40 x 0.75 = 30 | 40 x 0.5 = 20)

Some cases do not have rounded values. Therefore, the values are changed to reach the original values. Example:

Ballistics.yaml: 25mm:
Damage 27
Vs none 30%, is 8.1, in-game value is 8
Vs wood 40%, is 10.8, in-game value is 10
Vs light 110%, is 29.7, in-game value is 29
Vs heavy 45%, is 12.15, in-game value is 12
Vs concrete 30%, is 8.1, in-game value is 8

To reach the in-game values, I made these changes:
New damage 25
Vs none 32%, is 8
Vs wood 40%, is 10
Vs light 116%, is 29
Vs heavy 48%, is 12
Vs concrete 32%, is 8

And, of course, multiplied by 100.

The same procedure is used for these weapons:
Ballistics.yaml: 2Inch
Explosions.yaml: OreExplosion
Missiles.yaml: AntiGroundMissile, HellfireAA, MammothTusk, Nike, Stinger, APTusk
Smallcaliber.yaml: ZSU-23, HeavyMG, LightMG, Vulcan, ChainGun, ChainGun.Yak, Pistol, M1Carbine
Other.yaml: Flamer

Some values could not be rounded easily. I tried to reach as close as possible. These are the numbers:
In other.yaml:
PortaTesla: damage vs wood (45 x 0.75 = 33.75, in-game value is 33)
New value is 45 x 0.73 = 32.85 (so it deals 0.15 less damage than it used to do)
Claw: damage vs none (33 x 0.9 = 29.7, in-game value is 29)
New value is 30 x 0.97 = 29.1 (so it deals 0.1 more damage than it used to do)

So far, only these two values are not the same. However, there are other cases that are not the same anymore. These cases involve fallout. But luckily, the differences are also less than 1. These are nuke explosions, barrel explosions, cruisers and other weapons with area of effect damage. I tested these out and didn’t notice any differences.

Barrels do a lot of damage to buildings now compared to the previous release without the Hitshape changes. I will file a bug report for this to possibly find out a proper way to fix it. Edit: fixed in #14314.

Vulcan weapon is an interesting case. I see there is a PR #14244 from Smitty that will fix the inconsistency. I kept the old values for now. Edit: updated with the Vulcan changes.

Because there are a lot of changes, this PR is hard to review. I suggest to just play the game with these changes. It is maybe the easiest way to find out something is wrong. That’s how I found out that I forgot the repair and selfheal changes, also husk damage. Maybe one good way to do so is in a playtest with a streamer testing these changes. I did however double-check everything. Everything should be the same as before.

This, by the way, breaks all the custom rules in missions and maps. If this PR is accepted, I will make a new PR to change the rules there too. Edit: this breaks only the external custom maps and missions. Edit: upgrade rules added.

Fun fact: The pistol, which the technicians use, now actually do damage against armored units and buildings. Although it’s still not a lot :P

@ghost
Copy link

ghost commented Nov 6, 2017

The FIX, SPEN, and SYRD repairs units and will need the HpPerStep multiplied as well.
https://github.com/OpenRA/OpenRA/wiki/Traits#repairsunits

@Arular101
Copy link
Contributor Author

Updated.

Thanks for noticing that and giving me the right directions. I completely forgot about that.

@pchote
Copy link
Member

pchote commented Nov 7, 2017

I'm not a big fan of doing this manually (automated conversions save us from breaking downstream custom maps), but as you explained above doing this well is a lot more complicated than just multiplying numbers by a constant factor.

So I give a tentative 👍 to the approach (but haven't looked at the changes themselves!), but would like to hear what others such as @reaperrr and @GraionDilach have to think before we merge this.

@pchote
Copy link
Member

pchote commented Nov 7, 2017

We may want to do this in a two stage approach: a first commit applies an automated conversion that works on everything, and then a second commit that overwrites the mod rules with your hand-tuned values.

@Arular101
Copy link
Contributor Author

That's a very good idea. I found an upgrade rule from @reaperrr here. I think this can be modified to only apply to the RA-mod.

@GraionDilach
Copy link
Contributor

I disagree with the weapon recalculations based on principle - it labels most of the lost fractions from the damage calculations as a feature to being carried over instead of being a bug needing to be fixed.

On the other hand, even this method is better than what there is atm, so carry on.

@Arular101
Copy link
Contributor Author

I fully understand your principle. The problem is that the balance of the game is based on these values. To not upset the balance, these changes were made. I think these errors will be prevented for new mods if they are based on the new values.

@ghost
Copy link

ghost commented Nov 7, 2017

Keep in mind there are several official missions that define HP & damage for various units & weapons. These will need adjustment as well.

@Arular101
Copy link
Contributor Author

Arular101 commented Nov 9, 2017

I'm currently investigating all campaign files, and will make in the next few days a PR fixing some errors. After that, I think the upgrade rule should catch everything. I'll, of course, check all the stats of the shipped missions and maps again.

While I was investigating the campaign files, I remembered that I forgot to mention (in the openings post) some values that I kept the same, but resulting in 0.5 more damage. These are:
Ballistics.yaml: 155mm (inherits Artillery) only vs heavy
Explosions.yaml: UnitExplodeShip, UnitExplodeSubmarine and ArtilleryExplode (all inherits Explosion) vs wood and vs heavy
Others.yaml: FireballLauncher (inherits FireWeapon) only vs heavy

These values couldn't be changed that easily, I left it the same. Resulting in 0.5 more damage vs the mentioned armor types. Because most of it is vs heavy armor, it is neglectable.

@Arular101
Copy link
Contributor Author

I made this PR #14326 to remove almost all the custom rules for weapons in the campaign missions. This will unify the campaign experience with the skirmish/multiplayer experience. The upgrade rule should catch the rest (these are in Soviet-03, 9000 HP for the helicopter and fence). Only thing left to do is change the shipped custom maps/missions.

@Arular101 Arular101 force-pushed the RA-damage-HP-x100 branch 5 times, most recently from 52c7f04 to c3b0615 Compare November 12, 2017 20:12
@Arular101
Copy link
Contributor Author

Updated this PR with the changes of PR #14244.

@Arular101
Copy link
Contributor Author

Arular101 commented Nov 19, 2017

I went ahead and updated already the campaign and almost all the custom missions and maps.

Only thing left to do is Monster Tank Madness and Fort Lonestar.

Everything is double checked. Only one weapon does 0.5 more damage. In the mission Intervention, the weapon Maverick has a value of 175 damage.
Vs none 30%, is 52.5
Vs wood ans light 90%, is 157,5

I didn't want to change it because the damage is already high, and 0.5 damage difference is less than 1% of the total damage. So, there is no noticeable difference.

I'll keep the changes for the campaign/missions/maps in a separate commit.

@Arular101 Arular101 force-pushed the RA-damage-HP-x100 branch 3 times, most recently from aa8d547 to f8d9914 Compare November 26, 2017 11:05
@Arular101
Copy link
Contributor Author

Monster Tank Madness and Fort Lonestar is now updated also.

The damage of the SuperTankPrimary will probably change in this PR #14317. So I only multiplied it with 100.

For Fort Lonestar there where some adjustments to match the current damage for the following weapons: 120mm, TankNapalm and FLAK-23.
One instance does 0.5 more damage than it used to do: MammothTusk vs none 312.5 instead of 312, resulting in less than 1% more damage vs none.

Everything in the Red Alert mod is now adjusted. This part is finished. I'll come to the IRC to see what needs to be done and how we can proceed.

@Arular101 Arular101 force-pushed the RA-damage-HP-x100 branch 2 times, most recently from 7474335 to 93ebee1 Compare November 26, 2017 15:35
@Arular101
Copy link
Contributor Author

Updated with the balance changes from #14471. The light tank (25mm) is buffed vs wood armor. It does now (27 * 0.5 = 13.5 the actual value in game is 13) 13 damage.

The base damage was adjusted to 25, because this is a very nice number to fine-tune which is also the closest value to the original. This means 25 * 0.52 = 13 damage. That's why it is 52% here.

@Arular101
Copy link
Contributor Author

Updated with the new SuperTankPrimary damage from #14317.

@pchote pchote added this to the Next release milestone Dec 10, 2017
@pchote
Copy link
Member

pchote commented Dec 10, 2017

Adding to the milestone because IMO we either do this change now, or we forget about it forever.

reaperrr
reaperrr previously approved these changes Dec 10, 2017
var mod = modData.Manifest.Id;
if (mod == "ra")
{
if (node.Key == "Damage" && parent.Value.Value == "SpreadDamage")
Copy link
Member

Choose a reason for hiding this comment

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

Don't we also need to account for Damage, HealthPercentage and TargetDamage?

Copy link
Contributor

Choose a reason for hiding this comment

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

Damage is (currently) an abstract class so it can be ignored, but you're right about the other two.

Copy link
Contributor

Choose a reason for hiding this comment

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

I guess HPDW shouldn't be considered in this regard.

Copy link
Member

Choose a reason for hiding this comment

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

Ah right, I forgot that it uses the "Damage" variable for the percentage and doesn't define it's own field. Looks like we only need to care about TargetDamage then.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm, I couldn't find an example for the TargetDamage and there is no public readonly in that file. Could you help me, because I don't know which name I should use for the node.Key in the upgrade rules.

Copy link
Contributor

@reaperrr reaperrr Dec 12, 2017

Choose a reason for hiding this comment

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

It uses the same public readonly Damage that is located on the abstract (i.e. not usable on its own) DamageWarhead that all other *DamageWarheads inherit.

Just change the 'if' to

if (node.Key == "Damage" && (parent.Value.Value == "SpreadDamage" || parent.Value.Value == "TargetDamage"))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha, thanks!

@abcdefg30
Copy link
Member

Looks good to me otherwise. 👍

@Smittytron
Copy link
Member

I didn't catch anything out of the ordinary. This is one of those things where we'll need to (hopefully) catch anything doing too much or too little damage in the playtest.

@reaperrr
Copy link
Contributor

Needs a rebase, too.

@Arular101
Copy link
Contributor Author

Don't worry, this covers everything the RA-mod is shipped with. Everything is carefully changed, and it is checked multiple times. If someone has a question or doubts something, then I would like to be the first person to hear it before people jump to conclusions. So we can solve the problem. So please ping me if something is wrong. ^^ I'll be around.

@Arular101
Copy link
Contributor Author

Updated and rebased.

@reaperrr reaperrr merged commit 917d6b7 into OpenRA:bleed Dec 12, 2017
@reaperrr
Copy link
Contributor

changelog

@Arular101 Arular101 deleted the RA-damage-HP-x100 branch January 2, 2018 09:30
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.

None yet

6 participants