-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Build system overhaul #366
Conversation
This change is being made because the primary offering and use case of OverPy is the VS Code Extension. Having the primary functionality be at root level makes it easier to configure build tools.
small, self-explanatory, well-documented commits? what's that?
Because esbuild is traversing the graph of imports to determine which files to include, the ordering of imports within this one file is very important. I wish it wasn't so brittle, but it does make sense here.
Although this does provide less security than before, it will still terminate a memory bomb application eventually. Profiling debug tools have also been added to the code.
interpreter.setProperty( | ||
console, | ||
"log", | ||
interpreter.createNativeFunction(consoleLogWrapper), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does that not lead back to the security issue (since you can now do shit like console.log.prototype I guess) ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, from my understanding JS Interpreter does not allow access to the prototype of a function passed in in this manner. This is the recommended method for passing in a function, so unless that breaks the sandbox, this should not be an issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From the JS-Interpreter documentation:
Additionally, API calls may be added to the interpreter's global object during creation. Here is the addition of alert() and a url variable:
var myCode = 'alert(url);';
var initFunc = function(interpreter, globalObject) {
interpreter.setProperty(globalObject, 'url', String(location));
var wrapper = function alert(text) {
return window.alert(text);
};
interpreter.setProperty(globalObject, 'alert',
interpreter.createNativeFunction(wrapper));
};
var myInterpreter = new Interpreter(myCode, initFunc);
Alternatively, API calls may be added to an object:
var myCode = 'robot.forwards(robot.fast);';
var initFunc = function(interpreter, globalObject) {
// Create 'robot' global object.
var robot = interpreter.nativeToPseudo({});
interpreter.setProperty(globalObject, 'robot', robot);
// Define 'robot.fast' property.
interpreter.setProperty(robot, 'fast', 99);
// Define 'robot.forwards' function.
var wrapper = function forwards(speed) {
return realRobot.forwards(speed);
};
interpreter.setProperty(robot, 'forwards',
interpreter.createNativeFunction(wrapper));
};
var myInterpreter = new Interpreter(myCode, initFunc);
This code essentially acts like the last example, but substituting console
for robot
and the log
function for forwards
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, since it's a wrapper you can't do JS shenanigans. Pretty neat
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ain't reading all that, but it does compile Ana PB successfully 🎉
The only thing I noticed is that OverPy doesn't include comments from Values in @Condition
s anymore:
(In
https://github.com/JinkoMinko/ana-paintball/blob/827626c696392a4b139cf62984a9fb60daf55561/medals.opy#L136)
but it does include them for Values in the actions block, like in here: https://github.com/JinkoMinko/ana-paintball/blob/dfa292183722614709f363a767505f13b705c032/nano.opy#L8
Resolved in a91e4e5 |
This necessitated adding back the ability to build a standalone version of OverPy as well. Unsure what it would be used for, but here it is.
This PR does a lot of things at once:
TODO:
Introduce unit tests (Jest?)(no longer planned)Introduce behavior testing (documented at https://code.visualstudio.com/api/working-with-extensions/testing-extension)(no longer planned)