search

cocos2d / cocos2d-x

Cocos2d-x is a suite of open-source, cross-platform, game-development tools utilized by millions of developers across the globe. Its core has evolved to serve as the foundation for Cocos Creator 1.x & 2.x.
https://www.cocos.com/en/cocos2d-x
18.24k stars 7.06k forks source link

ScriptingCore::evalString not work #16541

Open flybird opened 8 years ago

flybird commented 8 years ago

Steps to Reproduce: I use latest version 3.13. I found I call ScriptingCore::evalString but It don't fire. the code always return false, the "script" is nullptr,

JSAutoCompartment ac(cx, global); JS::PersistentRootedScript script(cx); if (script == nullptr) { return false; }

I think it should be compare with point "&script"

if (&script == nullptr)

thuydx55 commented 8 years ago

I'm also getting this issue

DavidDeSimone commented 8 years ago

I was able to trivially reproduce this by running the line CCASSERT(ScriptingCore::getInstance()->evalString("(function() { cc.log ('World Hello!'); })();"), "Testing EvalString"); in AppDelegate.cpp after ScriptingCore initialization. The reasoning seems to be that we are initializing the JS::PersistentRootedScript without an initialization value, which will mean it's operator== against nullptr will return true. Like the reporter suggests we should use the overridden operator& of the script, comparing that against nullptr.

Also on a slightly related note, I am unsure on why we are using a JS::PersistentRootedScript here over a JS::RootedScript. The script in this use case has a definite lifetime, and is not cached or stored globally, so I see no point for using a PersistentRooted here.

I will submit a PR that fixes this issue. @pandamicro

PR is here https://github.com/cocos2d/cocos2d-x/pull/16597

pandamicro commented 8 years ago

Yes, you are right, this was due to code merging, I used to new a PersistentRootedScript, and delete it at the end, but totally it's useless, let's use JS::RootedScript.

Sorry for the trouble.