Skip to content
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

Handle VBScript errors #141

Open
freezy opened this issue Nov 9, 2019 · 0 comments
Open

Handle VBScript errors #141

freezy opened this issue Nov 9, 2019 · 0 comments
Labels
scripting Related to the VBScript port

Comments

@freezy
Copy link
Member

freezy commented Nov 9, 2019

One of VBScript's most embarrassing parts is its error handling. Basically you can turn it on and off at any time, at runtime. Error handling by side-effect, yay.

In JavaScript, there is no such thing. You can define a global error handler, but that won't resume code execution. For VBScript's API, that's less of a problem because we can handle errors ourselves (i.e. set the Err object accordingly), but for language features this doesn't work.

The first time we encounter this problem is how core.vbs updates lamps:

On Error Resume Next
	For ii = 0 To UBound(ChgLamp)
		idx = ChgLamp(ii, 0)
		Lights(idx).State = ChgLamp(ii, 1)
	Next
On Error Goto 0

If the ROM returns a lamp index that doesn't exist on the playfield, our transpiled script currently crashes, because Lights[idx] returns undefined, and undefined.State throws an error.

In VBScript this works, because error handling muted by On Error Resume Next. If Lights(idx) doesn't exist, well, never mind and continue.

There are a few ways of handling that, none particularly elegant.

Wrap every statement into a function

Since whether throw or swallow errors is a runtime condition, we can't determine at compile time which statements to wrap into a function that doesn't throw in case errors are muted. That means wrapping every statement of the table script into a function.

Apart from the code becoming an unreadable mess, this is also pretty bad for performance. Take the following snippet:

Click to view
function wrap(fct) {
	try {
		fct();
	} catch {
		// no nothing
	}
}

const ni = 1e9;
const o = {};

const time1 = Date.now();
for (let i = 0; i < ni; i++) {
	o.test = i;
}
const dTime1 = Date.now() - time1;

const time2 = Date.now();
for (let i = 0; i < ni; i++) {
	wrap(() => o.test = i);
}
const dTime2 = Date.now() - time2;

console.log('before: %sms', dTime1);
console.log('after:  %sms', dTime2);
Results with one billion iterations:
before after
Node.js 12.8, Win64 551ms 3064ms
Chrome 78, Win64 545ms 2349ms
Chrome Canary 80 602ms 3621ms

So this adds factor three to six. Which is pretty bad. Maybe there are sufficiently little statements in an average table script to stays below budget, but I doubt that.

Wrap only language features

We could start by wrapping only member assignments, until the next road block. At some point we might need to be able to catch division by zero as well and end up like the first solutions, but it would be a more lightweight approach.

Concretely, Lights(idx).State = ChgLamp(ii, 1) would compile to __vbsHelper.set(Lights[idx], 'State', ChgLamp[ii][1]), and the set method would only fail if error handling is enabled.

Fix this specific case by returning fake-lights

Our table could simply return 200 lights like core.vbs loops over that do nothing, and this particular case would be solved. It comes with a (memory) overhead as well, and doesn't solve the actual problem.

Use a core.vbs that doesn't rely on On Error

We can also fix this at the source, at core.vbs. We've got our hands on it, since we bundle it. This would however mean that as core.vbs evolves in the future, we need to re-apply our patches (does happen a few times a year). It also doesn't solve the problem for in-table shipped scripts.

Conclusion

I would reevaluate this once we get core.vbs working. If there are no other cases, we might as well use the third approach. If there are more, but only for undefined properties, the second approach might be better. The first one, while the "most proper" one, should be avoided, given the performance hit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
scripting Related to the VBScript port
1 participant