Page MenuHomeWildfire Games

Unify BuildDirEntList with FindJSONFiles to kill globalscripts/ JSON hacks
ClosedPublic

Authored by elexis on Dec 4 2017, 1:18 AM.

Details

Summary

The GUI, simulation, AI and random map generation run in separate threads with different custom functions exposed to JS.
This meant that we couldn't write one piece of code that works for all contexts.
rP20576 unified ReadJSONFile, this patch unified the listing of files and thus allows removing of these hacks noone could look at lightheartedly.

Test Plan

ComponentManager.cpp registers simulation/ functions. Notice that it doesn't fall into pieces when running the game.
Optionally notice that you get an error message when trying to list files from gui/ when calling it from the simulation.

Event Timeline

elexis created this revision.Dec 4 2017, 1:18 AM
Vulcan added a subscriber: Vulcan.Dec 4 2017, 1:27 AM

Build failure - The Moirai have given mortals hearts that can endure.

Vulcan added a comment.Dec 4 2017, 1:28 AM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
elexis added inline comments.Dec 4 2017, 6:53 PM
binaries/data/mods/public/globalscripts/Resources.js
3

This GUI Interface copying can be nuked too by just including JSInterface_VFS.cpp from the CCmpAIManager.cpp. In some later diff.

wraitii added a subscriber: wraitii.Dec 4 2017, 6:53 PM

Shouldn't the name Engine.BuildDirEntList be changed though?

elexis added a comment.Dec 4 2017, 6:55 PM

Shouldn't the name Engine.BuildDirEntList be changed though?

It's still the same function and can list names of files other than JSON too

elexis added inline comments.Dec 4 2017, 6:56 PM
binaries/data/mods/public/simulation/components/DataTemplateManager.js
59

See P97 where this is going

Maybe it's just me but "builddirentlist" has no meaning whatsoever. I'd prefer "ListDirectoryFiles" or something.

elexis added a comment.Dec 4 2017, 7:00 PM

Maybe it's just me but "builddirentlist" has no meaning whatsoever. I'd prefer "ListDirectoryFiles" or something.

Fine, since the size of the patch only increases by few lines

elexis updated this revision to Diff 4539.Dec 4 2017, 7:03 PM

Rename the JS BuildDirEntList to ListDirectoryFiles as proposed by wraitii

elexis added inline comments.Dec 4 2017, 8:19 PM
binaries/data/mods/public/gui/common/functions_utility.js
38

This is the same function used in the DataTemplateManager (see reasons why unifying is good). It should be in globalscripts/vfs.js P97. Same for getCheatsData() in messages.js.

Vulcan added a comment.Dec 4 2017, 8:22 PM

Successful build - Chance fights ever on the side of the prudent.

Updating workspaces...
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...
Vulcan added a comment.Dec 4 2017, 8:24 PM
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/globalscripts/Templates.js
|  91| »   »   ····················||·(c[0]·!=·"!"·&&·classes.indexOf(c)·!=·-1)))
|    | [NORMAL] JSHintBear:
|    | Misleading line break before '||'; readers may interpret this as an expression boundary.
s0600204 accepted this revision.Dec 4 2017, 11:32 PM

Looks good, with a little bit of tidying work to do which can be done before commit.

binaries/data/mods/public/gui/common/functions_utility.js
38

(Out-of-scope observation: getCheatsData() could use Engine.ListDirectoryFiles directly - no point stripping path and extension if they're just going to be added back.)

source/ps/scripting/JSInterface_VFS.cpp
87

Don't forget to update this function's block comment as appropriate. (And there's a copy in the .h file to deal with as well.)

source/simulation2/system/ComponentManager.cpp
1195

Don't forget to remove appropriate definitions in ComponentManager.h if they're no longer being used. (lines 118-22, 336-9)

This revision is now accepted and ready to land.Dec 4 2017, 11:32 PM

thanks for the review!

binaries/data/mods/public/gui/common/functions_utility.js
38

There is a lot more where this came from.

source/simulation2/system/ComponentManager.cpp
1195

FAK

This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Dec 5 2017, 12:47 AM