lance-gg / lance

Multiplayer game server based on Node.JS
https://lance-gg.github.io/
Apache License 2.0
1.59k stars 168 forks source link

The ClientEngine.start() promise is not resolved #124

Open T-vK opened 5 years ago

T-vK commented 5 years ago

Can we please change the code of ClientEngine.start so that it resolves once it's ready?

I was extremely confused that this just wouldn't work:

export default class MyGameClientEngine extends ClientEngine {
    // ...
    async start() {
        await super.start(); // Execution stops right here
        this.gameEngine.once('renderer.ready', () => {
            // ...
            document.querySelector('#joinGame').addEventListener('click', (clickEvent) => {
                console.log("CLICK #joinGame");
                if (Utils.isTouchDevice()){
                    this.renderer.enableFullScreen();
                }
                clickEvent.currentTarget.disabled = true;
                this.socket.emit('requestRestart');
            });
            // ...
        });
        // ...
    }
    // ...
}

Or this:

const clientEngine = new MyGameClientEngine(gameEngine, options);
clientEngine.start().then(()=>{
    // This doesn't get executed because `ClientEngine.start` doesn't resolve
    clientEngine.renderer.setupBackground();
    clientEngine.renderer.setRendererSize();
}).catch(console.error); 

It seems like a very bad idea to not resolve the promise immediately. (Not to mention that rejecting with undefined makes debugging a nightmare.)

I'd be willing to make a PR, but I wanted to discuss this first.

namel commented 5 years ago

that would be an excellent PR.

xaroth8088 commented 5 years ago

For anyone looking for a workaround, you can hack around it by doing the following:

    if (this.clientEngineRef.resolveGame) {
        this.gameEngineRef.initialized = true;
    }
    getPlayerGameOverResult() {
        return this.initialized;
    }

This works because ClientEngine stores the resolve function from ClientEngine::start()'s promise in a class variable that only ever gets called during ClientEngine::step(), and even then only if GameEngine::getPlayerGameOverResult() returns a truthy value.

Hacky? Yes. But, it does cause ClientEngine::start()'s behavior to match the documentation, so... 🤷‍♀

this comment was edited to make the workaround more reliable