This is taken from D1844 with an extra update of the JS int limits, and some whitespace. It mainly tests features that are useful for modding.
Details
Check that tests pass.
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
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/1301/display/redirect
binaries/data/mods/_test.sim/simulation/components/test-hotload1.js | ||
---|---|---|
23 ↗ | (On Diff #7877) | Missing spaces. |
binaries/data/mods/_test.sim/simulation/components/test-modding2.js | ||
2 ↗ | (On Diff #7877) | Missing spaces between operators. |
source/scriptinterface/tests/test_ScriptInterface.h | ||
267 ↗ | (On Diff #7877) | Anyway to not have the hardcoded js ? |
No, this should be in before, so that SM45 is shown to not break those (it doesn't).
binaries/data/mods/_test.sim/simulation/components/test-hotload1.js | ||
---|---|---|
23 ↗ | (On Diff #7877) | Sure I'll change that. |
source/scriptinterface/tests/test_ScriptInterface.h | ||
267 ↗ | (On Diff #7877) | What do you mean by hardcoded? This is not supposed to be moddable or anything, this is just the engine testing that it can run some specific JS code and yield the expected results from it. |
source/scriptinterface/tests/test_ScriptInterface.h | ||
---|---|---|
267 ↗ | (On Diff #7877) | Well as elexis said here https://wildfiregames.com/forum/index.php?/topic/25393-moving-components-schemas-to-xml-files/, having two languages in the same file is an anti pattern, and since you seemed to be loading scripts in Component manager I was wondering why you didn't do it as well. man.LoadScript(L"simulation/components/test-modding1.js") In binaries/data/mods we have, _test.dae, _test.sim etc we could have _test.scriptinterface, and there you could just tweak the js code without having to recompile. Just an idea. |
source/scriptinterface/tests/test_ScriptInterface.h | ||
---|---|---|
267 ↗ | (On Diff #7877) | No, this is a test file for the script interface, which parses and executes JS code in the C++ engine, so having the two languages in the file is perfectly natural. In the component manager, I'm testing the ability to load scripts and overload them when modding/hotloading. If I moved this code to a JS file, I would be testing the loading from the file and the snippet I actually want to test, which would make the test non-unitary (i.e. if it fails you don't know what failed). We could have a _test.scriptinterface mod indeed, but we would use it for different things like tests for globalscripts hotloading when we have that, testing the fix for #5376 when we have it, etc. |
source/scriptinterface/tests/test_ScriptInterface.h | ||
---|---|---|
267 ↗ | (On Diff #7877) | Sure was just as suggestion :) |
source/scriptinterface/tests/test_ScriptInterface.h | ||
---|---|---|
267 ↗ | (On Diff #7877) | Yeah no problem, I'm just explaining it in detail so that you learn new things ? |
source/scriptinterface/tests/test_ScriptInterface.h | ||
---|---|---|
267 ↗ | (On Diff #7877) | Thanks ! Also it will be useful if someone ask me the question in the future :) |
source/scriptinterface/tests/test_ScriptConversions.h | ||
---|---|---|
133 ↗ | (On Diff #7877) | If this is valid for SM38, why is this changing? |
source/scriptinterface/tests/test_ScriptConversions.h | ||
---|---|---|
133 ↗ | (On Diff #7877) | This should have been changed in a previous SM upgrade, yes. I don't know which one bumped jsval limits to the current values, but the new cassert will allow us to not miss future changes. Note that this is valid because the old value is now completely irrelevant and is a i32 value like any other. |
source/scriptinterface/tests/test_ScriptConversions.h | ||
---|---|---|
133 ↗ | (On Diff #7877) | Ah, right. Any reason yo not use JSVALA_INT_MAX directly here? |
source/scriptinterface/tests/test_ScriptConversions.h | ||
---|---|---|
133 ↗ | (On Diff #7877) | Ah, it could be used for the first argument yes indeed, and would remove the need for the cassert and all those comments. I basically kept things as they were, and it's quite readable this way, but doing what you suggest is probably better. |
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/3/display/redirect
Build failure - The Moirai have given mortals hearts that can endure.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/4/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/7/display/redirect
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... source/scriptinterface/tests/test_ScriptConversions.h | 31| class·TestScriptConversions·:·public·CxxTest::TestSuite | | [MAJOR] CPPCheckBear (syntaxError): | | Code 'classTestScriptConversions:' is invalid C code. Use --std or --language to configure the language. source/scriptinterface/tests/test_ScriptInterface.h | 26| class·TestScriptInterface·:·public·CxxTest::TestSuite | | [MAJOR] CPPCheckBear (syntaxError): | | Code 'classTestScriptInterface:' is invalid C code. Use --std or --language to configure the language. source/simulation2/tests/test_ComponentManager.h | 41| class·TestComponentManager·:·public·CxxTest::TestSuite | | [MAJOR] CPPCheckBear (syntaxError): | | Code 'classTestComponentManager:' is invalid C code. Use --std or --language to configure the language. Executing section JS... Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/135/display/redirect