Page MenuHomeWildfire Games

Transform "Upgrade.js" into an "Abilities.js" that is much more generic.
AbandonedPublic

Authored by wraitii on Mar 31 2017, 12:20 PM.

Details

Reviewers
None
Group Reviewers
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Restricted Owners Package(Owns No Changed Paths)
Summary

This patch changes the Upgrade.js component into an "Abilities.js" component, that can do a lot more. In fact, perhaps a little too much, but I don't really see how this could be updated to support an arbitrary number of different and custom abilities in another, saner way.

To show the flexibility of this new component, I have removed Upgrade entirely, and I have also removed the Gate panel in the Gui, replaced by abilities (no difference from the user POV).
In principle, this can be used to create mostly any stateless ability (key being stateless), though this can have a temporary state while active (e.g. for a timer). Exemples would include self-destruct, damage around yourself, all the current cheats, transforming entities into other entities, activating spells, locking/unlocking gates, choosing seeds for farms, mounting/dismounting units and really whatever.

Current features include:
-a charging time (taken from upgrade.js)
-a cool down time
-incompatible abilities (in two shapes)
-supports arbitrary code and is extensible (might be dangerous, need 2nd opinion)
-GUI support

Ultimately, it should be easy-ish to extend this to allow God Powers like in AoM, since an ability can remain "ready" eternally once charged, and we could add GUI elements to click an ability, then click an entity and trigger it there or whatever really.
It should also be reasonably easy to extend this to have a limited number of uses per ability, and to have more advanced conditions to trigger abilities.

I intend to use this to implement D266, by having technologies create a temporary entity with the required ability.

Combined with D274 and D270, this could be used to create temporary auras, temporary technologies, temporary modifiers, and basically pretty much anything.

Could probably include the Packing code in there too, though that might be a little annoying. Technically could include ProductionQueue I guess, but that doesn't seem worth it :P .

Known issues: had to take out the tooltip for entity limit on upgrade, since it's not convenient to get it from the GUI and I believe the tooltip helpers are GUI-only, need to change that.

Test Plan

Test, review.

I have implemented the following changes, some of which should not be committed:
-Gates locking and unlocking is an Ability.
-Upgrading to a gate is an ability.
-The Gaul CC has a "inflict damage to self" ability that damages it, with a cool down, and an "upgrade to briton CC" ability.

Diff Detail

Repository
rP 0 A.D. Public Repository
Branch
/ps/trunk
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 971
Build 1537: Vulcan BuildJenkins
Build 1536: arc lint + arc unit

Event Timeline

wraitii created this revision.Mar 31 2017, 12:20 PM
Owners added a subscriber: Restricted Owners Package.Mar 31 2017, 12:20 PM
wraitii updated this revision to Diff 1032.Mar 31 2017, 12:28 PM

Accidentally left my changes from D274 in there too.

Vulcan added a subscriber: Vulcan.Mar 31 2017, 1:07 PM

Build has FAILED

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).ERROR: JavaScript error: simulation/helpers/ValueModification.js line 6
ReferenceError: IID_ModifiersManager is not defined
  ApplyValueModificationsToEntity@simulation/helpers/ValueModification.js:6:6
  Attack.prototype.GetRange@simulation/components/Attack.js:397:8
  Attack.prototype.GetFullAttackRange@simulation/components/Attack.js:287:15
  @simulation/components/tests/test_Attack.js:104:26
  attackComponentTest@simulation/components/tests/test_Attack.js:95:2
  @simulation/components/tests/test_Attack.js:99:1

In TestComponentScripts::test_scripts:
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_Attack.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
ERROR: JavaScript error: simulation/helpers/ValueModification.js line 6
ReferenceError: IID_ModifiersManager is not defined
  ApplyValueModificationsToEntity@simulation/helpers/ValueModification.js:6:6
  @simulation/components/tests/test_Auras.js:96:19
  testAuras@simulation/components/tests/test_Auras.js:92:2
  @simulation/components/tests/test_Auras.js:95:1
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_Auras.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
ERROR: JavaScript error: simulation/helpers/ValueModification.js line 6
ReferenceError: IID_ModifiersManager is not defined
  ApplyValueModificationsToEntity@simulation/helpers/ValueModification.js:6:6
  Capturable.prototype.GetRegenRate@simulation/components/Capturable.js:168:14
  testCapturable@simulation/components/tests/test_Capturable.js:85:19
  @simulation/components/tests/test_Capturable.js:91:1
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_Capturable.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
ERROR: JavaScript error: simulation/helpers/ValueModification.js line 6
ReferenceError: IID_ModifiersManager is not defined
  ApplyValueModificationsToEntity@simulation/helpers/ValueModification.js:6:6
  Attack.prototype.PerformAttack@simulation/components/Attack.js:461:12
  @simulation/components/tests/test_Damage.js:122:1
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_Damage.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
ERROR: CVFSFile: file simulation/components/interfaces/Upgrade.js couldn't be opened (vfs_load: -110101)
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:50: Error: Assertion failed: componentManager->LoadScript(VfsPath(L"simulation/components") / pathname)
ERROR: JavaScript error: simulation/components/GuiInterface.js line 319
ReferenceError: IID_Abilities is not defined
  GuiInterface.prototype.GetEntityState@simulation/components/GuiInterface.js:319:6
  @simulation/components/tests/test_GuiInterface.js:566:25
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_GuiInterface.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
ERROR: JavaScript error: simulation/helpers/ValueModification.js line 6
ReferenceError: IID_ModifiersManager is not defined
  ApplyValueModificationsToEntity@simulation/helpers/ValueModification.js:6:6
  Pack.prototype.GetPackTime@simulation/components/Pack.js:101:9
  Pack.prototype.GetProgress@simulation/components/Pack.js:111:28
  @simulation/components/tests/test_Pack.js:71:18
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_Pack.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
ERROR: JavaScript error: simulation/helpers/ValueModification.js line 27
ReferenceError: IID_ModifiersManager is not defined
  ApplyValueModificationsToTemplate@simulation/helpers/ValueModification.js:27:6
  ApplyValueModificationsToPlayer@simulation/helpers/ValueModification.js:21:9
  Player.prototype.GetMaxPopulation@simulation/components/Player.js:158:20
  Player.prototype.GetPopulationLimit@simulation/components/Player.js:148:18
  @simulation/components/tests/test_Player.js:27:18
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_Player.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
ERROR: JavaScript error: simulation/helpers/ValueModification.js line 27
ReferenceError: IID_ModifiersManager is not defined
  ApplyValueModificationsToTemplate@simulation/helpers/ValueModification.js:27:6
  VisionSharing.prototype.AddSpy@simulation/components/VisionSharing.js:126:40
  @simulation/components/tests/test_VisionSharing.js:134:1
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_VisionSharing.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
...............................................................................................................................................................................................................................................................................................................
Failed 1 and Skipped 0 of 305 tests
Success rate: 99%

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

Build has FAILED

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (305 tests).ERROR: CVFSFile: file simulation/components/interfaces/Upgrade.js couldn't be opened (vfs_load: -110101)

In TestComponentScripts::test_scripts:
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:50: Error: Assertion failed: componentManager->LoadScript(VfsPath(L"simulation/components") / pathname)
ERROR: JavaScript error: simulation/components/GuiInterface.js line 319
ReferenceError: IID_Abilities is not defined
  GuiInterface.prototype.GetEntityState@simulation/components/GuiInterface.js:319:6
  @simulation/components/tests/test_GuiInterface.js:566:25
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Test failed: L"Running script simulation/components/tests/test_GuiInterface.js"
/mnt/data/jenkins/workspace/phabricator/source/simulation2/components/tests/test_scripts.h:44: Error: Assertion failed: scriptInterface.LoadScript(pathname, content)
...............................................................................................................................................................................................................................................................................................................
Failed 1 and Skipped 0 of 305 tests
Success rate: 99%

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

wraitii updated this revision to Diff 1035.Mar 31 2017, 3:31 PM

Ought to fix tests (not sure, I'm rebasing on another branch locally).

Build is green

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

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

wraitii updated this revision to Diff 1259.Apr 15 2017, 11:17 AM

Rebase, fix bugs, add a few things.

Build is green

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

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

Stan awarded a token.Oct 29 2017, 10:37 AM
Stan rescinded a token.
Stan awarded a token.

This is one I'd like some kind of comment on. Do people like the idea? Hate it?

asterix added a subscriber: asterix.May 5 2018, 4:15 PM
In D281#60602, @wraitii wrote:

This is one I'd like some kind of comment on. Do people like the idea? Hate it?

I like the idea and I also discussed it with Neph from Hyrule Conquest and he will also like it since it increases the possibilities some kind of effects as AOM does and other things.

Stan added a subscriber: Stan.Jun 15 2019, 11:57 AM
Stan added inline comments.
binaries/data/mods/public/gui/session/selection_panels_helpers.js
1–3

Why that change ?

binaries/data/mods/public/simulation/components/Abilities.js
90

Is it worth to serialize/deserialize all that ?

163

JSDOC

164

Maybe just add :) we are in the ability class after all, same for parameters.

179

Same here.

187

JSDOC

253

just return object.keys ?

264

JSDOC in the header @todo

312
324

Maybe it could have been done gradually above instead of reiterating through the whole thing again.

331

Capital

336

jsdoc

420

Capital.

432

Capital.

448

That sounds fishy, why one and two ?

binaries/data/mods/public/simulation/components/GuiInterface.js
319–321

let

binaries/data/mods/public/simulation/data/abilities/DealDamage.json
4

Newline :)

binaries/data/mods/public/simulation/data/abilities/EntityUpgrade.json
4

Newline :)

binaries/data/mods/public/simulation/data/abilities/Gates.json
4

Newline :)

binaries/data/mods/public/simulation/data/abilities/GlobalUpgrade.json
4 ↗(On Diff #1259)

Newline :)

binaries/data/mods/public/simulation/helpers/Abilities.js
126

I guess all this could be unified somehow with two more params to call functions like

if (!this.effects.has(effect) || !this.effects.get(effect).[functionName1])
    return [];

return this.effects.get(effect).[functionName2](entity, templateData);
binaries/data/mods/public/simulation/helpers/Commands.js
668

could maybe add the ability name in the message as well :)

binaries/data/mods/public/simulation/templates/other/palisades_rocks_fort.xml
3

Maybe it could disable only the Upgrade part ?

wraitii abandoned this revision.Jun 15 2019, 12:23 PM

Erf, to be honest... I meant to abandon this rev - thought I had, in fact.

My current thinking is that we should move in helpers whatever needs to be, then just add as many components as we want Abilities, even if basically 2 or 3 entities have these components. Loading JS at runtime doesn't seem much better than just adding new components.

There's one thing that could change this, and that would be that adding N new component types might slow down other code too much... That should be checked, perhaps.