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

Damage calculations dialog should show "×2 (charge)" for the defender too #4408

Open
YellowKingValley opened this issue Sep 30, 2019 · 18 comments

Comments

@YellowKingValley
Copy link

commented Sep 30, 2019

The Rise of Wesnoth. Horseman has -30% impact resistance instead of +30%. Occurs in other campaigns as well.

The Battle for Wesnoth version 1.14.9 (9eba78f-Modified)
Running on Microsoft Windows 10 (10.0.18362)

@soliton-

This comment has been minimized.

Copy link
Member

commented Sep 30, 2019

Can you show what modifications you have applied to 1.14.9?

Does this happen without any addons as well?

@YellowKingValley

This comment has been minimized.

Copy link
Author

commented Oct 1, 2019

I just installed the game through Steam recently. Happened even before I installed any addons. I am not sure where I can find the modifications I have applied. Hope these info helps. Should I uninstall all addons and try again?

The Battle for Wesnoth version 1.14.9 (9eba78f-Modified)
Running on Microsoft Windows 10 (10.0.18362)

Game paths

Data dir: D:\Program Files (x86)\Steam\steamapps\common\wesnoth
User config dir: C:\Users\USER\Documents\My Games\Wesnoth1.14
User data dir: C:\Users\USER\Documents\My Games\Wesnoth1.14
Saves dir: C:\Users\USER\Documents\My Games\Wesnoth1.14\saves
Add-ons dir: C:\Users\USER\Documents\My Games\Wesnoth1.14\data\add-ons
Cache dir: C:\Users\USER\Documents\My Games\Wesnoth1.14\cache

Libraries

Boost: 1.66
OpenSSL/libcrypto: 1.1.0f (runtime 1.1.0f)
Cairo: 1.10.2 (runtime 1.10.2)
Pango: 1.30.1 (runtime 1.30.1)
SDL: 2.0.8 (runtime 2.0.8)
SDL_image: 2.0.3 (runtime 2.0.3)
SDL_mixer: 2.0.2 (runtime 2.0.2)
SDL_ttf: 2.0.14 (runtime 2.0.14)

Features

JPG screenshots: yes
Lua console completion: yes
Legacy bidirectional rendering: no
Win32 notifications back end: yes

@YellowKingValley

This comment has been minimized.

Copy link
Author

commented Oct 1, 2019

Capture

@Wedge009

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

I did a quick check of an Iron Mauler (impact) against Horseman and the damage is reduced from 25 to 18, which is 30% impact resistance after rounding.

@soliton-

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

Ah, a vulnerability of 0.7 means it's actually a 30% resistance.

@Wedge009

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

That Damage Calculations image above shows the defender attacking with 30% reduction.

@YellowKingValley

This comment has been minimized.

Copy link
Author

commented Oct 1, 2019

6 x 0.7 x 1.25 = 5.25

6 / 0.7 x 1.25 = 10.71 ~ 11

@Wedge009

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

Hmm, so the Total Damage figure appears incorrect.

Edit: No, in this case the Horseman is the attacker. Attacks with Charge do double damage to both the defender and the attacker. The calculations are correct.

Edit: 6 x 0.7 x 1.25 x 2 = 10.5 ~= 11

@YellowKingValley

This comment has been minimized.

Copy link
Author

commented Oct 1, 2019

Ugh my bad. False alarm then. Sorry and thanks.

@Wedge009

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

No worries. I was thinking maybe the Charge modifier should be on the Defender side as well, to make this clear.

@jostephd

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

@YellowKingValley, in the future, please share not only your conclusions but also the observations you based those conclusions on.

I will add that there is a bug in that dialog: "×2 (charge)" should be shown in the defender side of the dialog as well. However, that bug is already fixed in master:

2019-10-01-122002_1920x1080_scrot

And for reference, that screenshot is Horseman v. strong Thug at night.

@Wedge009

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

Didn't know it was added in master. Good that it's already done. (:

@jostephd

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

It should be done in 1.14 too, though.

@jostephd jostephd reopened this Oct 1, 2019
@jostephd jostephd added Stable 1.14 and removed Invalid labels Oct 1, 2019
@jostephd jostephd changed the title Cavalry impact resistance became susceptibility Damage calculations dialog should show "×2 (charge)" for the defender too Oct 1, 2019
@jostephd jostephd added UI and removed Units labels Oct 1, 2019
@jostephd

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

I think it might have been fixed here, as part of a larger patch:

ebb980a#diff-95ed73f0bfc3aa6b8db7c0e357cca851

This change is similar:

47dd4eb#diff-95ed73f0bfc3aa6b8db7c0e357cca851

@Wedge009

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

I was looking for that... how to only pull out the Charge addition?

Oh, it seems to be a change in the weapon filtering. Not such an easy fix for me...

@jostephd

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

I think what's needed here is basically to port the changes to attack_predictions::set_data, and then to port those "add a parameter" changes until it compiles. It might be easier to do it the other way, to backport the entire patch except for the changelog and unit::ability_affects_weapon changes.

@Wedge009

This comment has been minimized.

Copy link
Member

commented Oct 1, 2019

Maybe @newfrenchy83 and/or @CelticMinstrel can comment on what they feel is the best approach for this one.

@CelticMinstrel

This comment has been minimized.

Copy link
Member

commented Oct 2, 2019

Not 100% sure, but my initial suspicion would be that the key lies in the addition of opp_ctx. I'm not sure whether the added parameters to damage_from and under_leadership are required; they might be?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.