Page MenuHomeWildfire Games

Add missing auras
ClosedPublic

Authored by fatherbushido on Jan 4 2017, 4:15 PM.

Details

Summary

Posted also in the forum. I can't use arcanist with it due to the renamed file.

Test Plan

agreed by scyte.
Posted here for inline comment.

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

fatherbushido updated this revision to Diff 128.Jan 4 2017, 4:15 PM
fatherbushido retitled this revision from to Add missing auras.
fatherbushido updated this object.
fatherbushido edited the test plan for this revision. (Show Details)
fatherbushido added a reviewer: elexis.
fatherbushido set the repository for this revision to rP 0 A.D. Public Repository.
Vulcan added a subscriber: Vulcan.Jan 4 2017, 5:57 PM

Build is green

Updating workspaces.
Build (release)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
../../../source/lib/tex/tex_png.cpp: In member function ‘virtual Status TexCodecPng::encode(Tex*, DynArray*) const’:
../../../source/lib/tex/tex_png.cpp:309:9: warning: variable ‘ret’ might be clobbered by ‘longjmp’ or ‘vfork’ [-Wclobbered]
  Status ret = ERR::FAIL;
         ^
Build (debug)...
../../../source/gui/CChart.cpp:38:41: warning: unused parameter ‘Message’ [-Wunused-parameter]
 void CChart::HandleMessage(SGUIMessage& Message)
                                         ^
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/89/ for more details.

elexis edited edge metadata.Jan 4 2017, 7:43 PM

Mentioned forum thread: https://wildfiregames.com/forum/index.php?/topic/21371-adding-auras-to-easter-egg-hero-regicide-mode/
Wouldn't mind seeing more unit variety and thinking about making those bugs features, but that's not the scope of this diff.
Ack on the balancing and approach.

binaries/data/mods/public/simulation/data/auras/iber_hero_viriato.json
7 ↗(On Diff #128)

Resilience doesn't fit to a faster training time in my opinion. Under that name I would suggest that units endure longer (i.e. die less quickly, i.e more HP or armor).
Perhaps something about mobilization would work.

10 ↗(On Diff #128)

Newline at end of file

binaries/data/mods/public/simulation/data/auras/mace_hero_craterus.json
6 ↗(On Diff #128)

Wasn't there a recent commit about adding all attack classes to make it more consitent?

binaries/data/mods/public/simulation/data/auras/mace_hero_pyrrhus.json
12 ↗(On Diff #128)

Since this game is also supposed to be about history education, we should not just write the pure balancing effects but also explain the idea and reasoning behind them:

From wiki/Pyrrhic_victory:

A Pyrrhic victory is a victory that inflicts such a devastating toll on the victor that it is tantamount to defeat. Someone who wins a Pyrrhic victory has been victorious in some way. However, the heavy toll negates any sense of achievement or profit.

binaries/data/mods/public/simulation/data/auras/ptol_hero_cleopatra_2.json
10 ↗(On Diff #128)

Perhaps quickly check at transifex whether there are requests for capitalization changes.

Some brief historical otherwise immersive text wouldn't hurt in case you have some ref.

binaries/data/mods/public/simulation/templates/units/mace_hero_craterus.xml
6 ↗(On Diff #128)

This guy doesn't have a history tag, wouldn't hurt to add one, if we have the resources for that. Perhaps we should ask on the cited forum thread quickly since some of these auras comes from wowgetoffyourcellphone's mod.

From Craterus

Craterus or Krateros (Greek: Κρατερός; c. 370 BC – 321 BC) was a Macedonian general under Alexander the Great and one of the Diadochi.
He was the son of a Macedonian nobleman named Alexander from Orestis and brother of admiral Amphoterus. Craterus commanded the phalanx and all infantry on the left wing in Battle of Issus (333 BC). In Hyrcania he was sent on a mission against the Tapurians, his first independent command with the Macedonian army. At the Battle of the Hydaspes River (326 BC, near modern Jhelum) he commanded the rearguard, which stayed on the western bank; his men crossed the river only during the final stages of the battle.

fatherbushido added inline comments.Jan 4 2017, 8:13 PM
binaries/data/mods/public/simulation/data/auras/iber_hero_viriato.json
7 ↗(On Diff #128)

Dunno wich one fits better to the history tag of that hero (I will not copy it here).
I don't know how you understand Resilience, but here it sounds okin his psychological meaning (here as the ability to recover from huge loss and to recruit new army).

binaries/data/mods/public/simulation/data/auras/mace_hero_craterus.json
6 ↗(On Diff #128)

Yes, I will add them if ever...

binaries/data/mods/public/simulation/data/auras/mace_hero_pyrrhus.json
12 ↗(On Diff #128)

It doesn't fit in auraDescription.

binaries/data/mods/public/simulation/templates/units/mace_hero_craterus.xml
6 ↗(On Diff #128)

Let's adress that for another ticket / patch.
There are also stuff plan to display those history tooltip somewhere iirc.
(Else sure I agree for the history goal).

elexis accepted this revision.Jan 4 2017, 9:02 PM
elexis edited edge metadata.

Well, then sorry for wasting your time mostly ;) Didn't test, but I assume you know what you're doing.

binaries/data/mods/public/simulation/data/auras/iber_hero_viriato.json
7 ↗(On Diff #128)

Not convinced, if units die like and reproduce like flies, they can't be considered resilient. But the name can be used, I don't mind.

binaries/data/mods/public/simulation/data/auras/mace_hero_craterus.json
6 ↗(On Diff #128)
binaries/data/mods/public/simulation/data/auras/mace_hero_pyrrhus.json
12 ↗(On Diff #128)

Hm, you are right that this field should remain as brief as possible, at least all other auraDescription entries besides the following avoid storytelling:

teambonuses/athen_player_teambonus.json: "auraDescription": "Shortly after the great naval victories at Salamis and Mycale, the Greek city-states instituted the so-called Delian League in 478 BC, whose purpose was to push the Persians out of the Aegean region. The allied states contributed ships and money, while the Athenians offered their entire navy."

Perhaps "Achieve victory with devastating tolls. All soldiers...." would still be short, but we can leave it out too to to focus on the gameplay effects.

This revision is now accepted and ready to land.Jan 4 2017, 9:02 PM
fatherbushido updated this revision to Diff 167.Jan 8 2017, 4:43 PM
fatherbushido edited edge metadata.

Fix some stuff with previous remarks. For the moment don't add Iberian and Scipio auras.

fatherbushido updated this revision to Diff 169.EditedJan 8 2017, 4:56 PM

Address some remarks and fix some stuff. I will wait a bit before adding that(those) Iberians(s) auras and the Roman Scipio's one (or placeholder) .

This revision was automatically updated to reflect the committed changes.
Vulcan added a comment.Jan 8 2017, 5:27 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/120/ for more details.

Vulcan added a comment.Jan 8 2017, 6:56 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running debug tests...
Running cxxtest tests (302 tests)..............................................................................................................................................................................................................................................................................................................OK!

http://jw:8080/job/phabricator/121/ for more details.

Vulcan added a comment.Jan 8 2017, 7:05 PM

Build has FAILED

Link to build: http://jw:8080/job/phabricator/122/
See console output for more information: http://jw:8080/job/phabricator/122/console

elexis changed the visibility from "All Users" to "Public (No Login Required)".Jun 26 2017, 2:25 PM