vpdb / vpx-js

:video_game: Visual Pinball in the Browser
GNU General Public License v2.0
51 stars 11 forks source link

VBS function calls and invalid scope #144

Closed neophob closed 4 years ago

neophob commented 4 years ago

vpm-controller.ts (class) exposes this function:

    public Run() {
//whatever
    }

when this class is called, this is assigned to the function instead to the class. Bcktrace of this call:

    at Function.Run (http://localhost:8080/b5e7b3b55421.worker.js:9222:55)
    at VBSHelper.getOrCall (http://localhost:8080/b5e7b3b55421.worker.js:12642:33)
    at TableApi.eval (tablescript.js:17:25)
    at TableApi.emit (http://localhost:8080/b5e7b3b55421.worker.js:101716:5)

    getOrCall(obj, ...params) {
        if (typeof obj === 'function') {
            return obj.bind(obj)(...params); <<
        }
        for (const param of params) {
            obj = obj[param];
        }
        return obj;
    }

this is definitive correct for setter and getter calls, not sure if thats the case for all class methods.

Proposal to solve this:

  1. fix how we call the functions in the VBS Helper
  2. use pure functions in the vpm-controller.ts (no class) - however not sure what other functions VBS Helper is calling
freezy commented 4 years ago

Hmm, does removing .bind() in the helper keep the scope to this?

neophob commented 4 years ago

nop, just tried, played around with this but did not have a good solution either atm.

a VERY ugly fix would be:

 let self: VpmController;

...
    constructor(player: Player) {
...
        self = this;
        }

    public Run() {
        logger().debug('RUN', self.gameName, new Error('x').stack);
        }
neophob commented 4 years ago

After reading https://www.freecodecamp.org/news/this-is-why-we-need-to-bind-event-handlers-in-class-components-in-react-f7ea1a6f93eb/ I propose this solution:

for each function which is NOT a get/set function, use this in the constructor:

    constructor(player: Player) {
                 ...
        this.Run = this.Run.bind(this);
    }

any objections?

freezy commented 4 years ago

Can be closed now, right?