Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#3507 closed defect (fixed)

[PATCH] some technologies and auras don't affect all damage types

Reported by: Palaxin Owned by: scythetwirler
Priority: Should Have Milestone: Alpha 19
Component: UI & Simulation Keywords: patch
Cc: Patch:

Description (last modified by Palaxin)

For some unit types with mixed damage (e.g. spear infantry or slingers) only one damage type is affected by the technology "Will to fight" and by the Leonidas hero aura.

I added the missing damage types in the patch. Note that there may be more files to be corrected.

Attachments (3)

patch_attack_v1.7z (1.4 KB ) - added by Palaxin 8 years ago.
patch for "Will to fight" technology and Leonides template
attack_v1.patch (2.5 KB ) - added by Palaxin 8 years ago.
attack_v2.patch (2.1 KB ) - added by Palaxin 8 years ago.
I have simplified it a bit. It didn't work when I used "Male" or "Male Unit" for "affects". Is there a possiblity to do that? However, this would not cause the intended changes. For example we have a slave template, and slaves should definitely not get bonus attack through this technology though they are males. I added "Heros", so basically all human fighting units should profit from this tech now.

Download all attachments as: .zip

Change History (17)

by Palaxin, 8 years ago

Attachment: patch_attack_v1.7z added

patch for "Will to fight" technology and Leonides template

comment:1 by Palaxin, 8 years ago

Description: modified (diff)

comment:2 by bb, 8 years ago

Palaxin you should upload a patch/diff with your changes. Use "svn diff" in command line or use TortoiseSVN when working on Windows. See: http://trac.wildfiregames.com/wiki/SubmittingPatches

Last edited 8 years ago by bb (previous) (diff)

comment:3 by Palaxin, 8 years ago

Sorry and thank you for the link. I do not have experience with such tools and did only edit the templates in my A18 installation folder so far, but I saw in GitHub that the problem still exists. Nevertheless I will try to use SVN and get the .diff files.

comment:4 by bb, 8 years ago

You already found the build instructions for the svn? http://trac.wildfiregames.com/wiki/BuildInstructions Please make your patches from the newest svn version, thats easier for developers.

comment:5 by elexis, 8 years ago

Also notice you don't need to compile on windows, as the repository comes with a precompiled .exe file (autobuild, which is updated more or less often enough).

by Palaxin, 8 years ago

Attachment: attack_v1.patch added

comment:6 by Palaxin, 8 years ago

This patch is made with tortoiseSVN from the newest SVN. I will try to compile myself, otherwise I use the autobild (I use Windows 10). Thank you for your help.

Last edited 8 years ago by Palaxin (previous) (diff)

comment:7 by Palaxin, 8 years ago

If you ask me, the Leonidas aura should give defensive and not offensive stats. The mission at Thermopylae was to resist against the Persian invaders as long as possible and not to slaughter them as there was no chance to defeat them anyway. So I would add 2-3 armor instead of +20% attack. But I'm not in the position to make such changes so I hope this patch makes you happy.

comment:8 by elexis, 8 years ago

I wonder if it can't be simplified, so that we would only have to change one line? Can't we just make all "Male" units to modify all "Attack" values or something like that?

comment:9 by Palaxin, 8 years ago

There are several other technologies which always list the three damage types. So this would be very convenient, but I'm afraid someone else would have to implement this (perhaps in attack.js?)

comment:10 by elexis, 8 years ago

Keywords: patch review added
Milestone: BacklogAlpha 20

by Palaxin, 8 years ago

Attachment: attack_v2.patch added

I have simplified it a bit. It didn't work when I used "Male" or "Male Unit" for "affects". Is there a possiblity to do that? However, this would not cause the intended changes. For example we have a slave template, and slaves should definitely not get bonus attack through this technology though they are males. I added "Heros", so basically all human fighting units should profit from this tech now.

comment:11 by Niek, 8 years ago

the 'affects' refers to unit classes. AFAIK Male isn't one of them.

comment:12 by scythetwirler, 8 years ago

Owner: set to scythetwirler
Resolution: fixed
Status: newclosed

In 17275:

Fixes #3507. Patch by Palaxin.

comment:13 by scythetwirler, 8 years ago

Keywords: review removed
Milestone: Alpha 20Alpha 19

Thanks for the patch. :) Not a big problem, but in the future, try to generate your patches from the root of the repository (that has the folders binaries, build, libraries, source etc.) for convenience.

comment:14 by Palaxin, 8 years ago

Ok I will do so :)

Note: See TracTickets for help on using tickets.