Page MenuHomeWildfire Games

Complete support of {civ} replacement in Builder list (as in ProductionQueue)
ClosedPublic

Authored by fatherbushido on Jul 4 2017, 5:31 PM.

Details

Summary

It would allow to have a builder list in generic templates and don't have errors when the replacement fails (ie the template doesn't exist for the said civ) as it's done for ProductionQueue list.
That was discussed here: https://wildfiregames.com/forum/index.php?/topic/22453-extending-civ-function-in/#comment-332475

Test Plan
  • Checks it works as expected (the modified function is tested in the updated test)
  • I didn't check if the AI is ok with that (mimo? sandarac?)
  • Check the struct tree

Event Timeline

fatherbushido created this revision.Jul 4 2017, 5:31 PM
Vulcan added a subscriber: Vulcan.Jul 4 2017, 6:41 PM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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

Vulcan added a comment.Jul 4 2017, 6:42 PM
Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/285/ for more details.

s0600204 requested changes to this revision.Jul 4 2017, 7:19 PM
s0600204 added inline comments.
binaries/data/mods/public/gui/structree/structree.js
179 ↗(On Diff #2806)
  1. Indent this line.
  2. You could negate the condition, and use a continue:
if (!g_ParsedData.structures[structCode])
    continue;

and thus not have to indent the following code block.

263 ↗(On Diff #2806)

Indent the line.

binaries/data/mods/public/simulation/components/Builder.js
46

let

This revision now requires changes to proceed.Jul 4 2017, 7:19 PM
Sandarac edited edge metadata.Jul 5 2017, 6:21 AM

I don't believe the AI will need adaptation to these changes. The AI never accesses the Builder component (through AIProxy or otherwise); it instead uses its own
buildableEntities() function in entity.js (which is basically the equivalent of cmpBuilder.GetEntitiesList()). And in any case, this change to GetEntitiesList() just adds a template existence check, and the AI already does checks for this in the appropriate places.

fatherbushido edited edge metadata.
fatherbushido edited the test plan for this revision. (Show Details)

Adress s0600204 remarks.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/330/ for more details.

Remove structure tree change as civ-repalcement is handled in D295

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/348/ for more details.

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

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

Executing section Default...
Executing section Source...
Executing section JS...
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jw:8080/job/phabricator_lint/349/ for more details.

@wowgetoffyourcellphone: could you try that version against the current svn and your mod?

s0600204 accepted this revision.Aug 7 2017, 4:39 PM

Tested locally by removing structures/ptol_lighthouse from all ptol units, and adding structures/{civ}_lighthouse to the parent template_unit_infantry template.

Patch works, with the ptol infantry units still having the lighthouse; the infantry of non-ptol sides not having it; and no errors thrown.


(For the purposes of the record, Delenda Est is not currently compatible with current svn state at all. (https://wildfiregames.com/forum/index.php?/topic/22707-delenda_est-error/) He is partway through prepping an A22 release.)

This revision is now accepted and ready to land.Aug 7 2017, 4:39 PM
This revision was automatically updated to reflect the committed changes.

Thanks for the review