ondras / rot.js

ROguelike Toolkit in JavaScript. Cool dungeon-related stuff, interactive manual, documentation, tests!
https://ondras.github.io/rot.js/hp/
BSD 3-Clause "New" or "Revised" License
2.33k stars 254 forks source link

using node-canvas #168

Closed GitNitneroc closed 4 years ago

GitNitneroc commented 4 years ago

Hi, and thanks for reading

I know my use case is very specific, but I need server-side image generation. I thought it would be trivial to replace the use of html canvas by node-canvas (module name "canvas"), but my lack of knowledge in TypeScript and maybe Make left me unable to make any progress.

Having a node-module named "canvas" was probably conflicting with your display.canvas class, so I renamed that class to ROTCanvas and everything seemed fine in my IDE, with types properly supported for node-canvas.

However the make command always ends up with an error message telling me TypeScript could not find the "canvas" module.

I know it's not really a rot problem, but could you give me pointers for this issue ? I'll be glad to submit a PR if I make it work and if this interests anyone

ondras commented 4 years ago

Hi @GitNitneroc,

turns out that I am no great TS guru either. But I hope we will find something out :)

Having a node-module named "canvas" was probably conflicting with your display.canvas class, so I renamed that class to ROTCanvas and everything seemed fine in my IDE, with types properly supported for node-canvas.

This caught my attention, because I do not exactly see how a builtin (node_modules) module could conflict with my own module specifiers, which are always specified via relative paths (their names start with a dot). Would you be willing to publish your code prior to renaming Display.Canvas to ROTCanvas? I would like to try to solve the conflict without renaming stuff.

However the make command always ends up with an error message telling me TypeScript could not find the "canvas" module.

I hope we will find the answer here: https://www.typescriptlang.org/docs/handbook/module-resolution.html. In particular, the paths mapping might be the key. But again, I would love to see the code in order to experiment with it.

GitNitneroc commented 4 years ago

First, thank you very much for reading my message !

I hope we will find the answer here: https://www.typescriptlang.org/docs/handbook/module-resolution.html. In particular, the paths mapping might be the key.

I looked at this file too, but your tsconfig has a target of es2017 so the Node resolution path should be used and rot.js/node_modules/ should definitely be used.

I would love to see the code in order to experiment with it.

I will upload a complete project if you want but here's a small example : I just used npm install and npm install canvas then I modified the src/display/canvas.ts file as such :

import Backend from "./backend.js";
import { DisplayOptions } from "./types.js";
import * as node_canvas from "canvas";

export default abstract class Canvas extends Backend {
    _ctx: CanvasRenderingContext2D;

    constructor() {
        super();
        //this._ctx = document.createElement("canvas").getContext("2d") as CanvasRenderingContext2D;
        this._ctx = node_canvas.createCanvas(32,32).getContext("2d");
    }

(I only pasted here the first lines, the rest didn't change)

then, running make, seems to suggest there really is a conflict between display.canvas and the canvas module:

node_modules/.bin/tsc
src/display/canvas.ts:11:27 - error TS2339: Property 'createCanvas' does not exist on type 'typeof import("/home/me/myProject/rot.js/src/display/canvas")'.

So I ended up changing your Canvas in ROTCanvas and renamed canvas.ts as ROTCanvas.ts. That's when make realized it could not find the canvas module.

src/display/ROTcanvas.ts:3:30 - error TS2307: Cannot find module 'canvas'.

3 import * as node_canvas from "canvas";

the strangest thing for me is that my IDE has typings for the canvas module, so it must have a valid typescript definition file and be importable...

Thank you very much for your interest in my issue

ondras commented 4 years ago

then, running make, seems to suggest there really is a conflict between display.canvas and the canvas module:

This sounds exactly like the case for the paths mapping: apparently TS is unable to properly resolve the module specifier and we should be able to provide an explicit hint.

I will hopefully find some time to get to your issue tomorrow.

ondras commented 4 years ago

I looked at this file too, but your tsconfig has a target of es2017 so the Node resolution path should be used and rot.js/node_modules/ should definitely be used.

Apparently not -- running tsc --traceResolution reveals usage of the Classic resolution strategy.

So I added --moduleResolution Node and it worked! You can either pass this as a CLI option in the Makefile or add it to the list of compilerOptions in tsconfig.json.

GitNitneroc commented 4 years ago

Amazing, thank you very much Ondras ! Reading the documentation still makes me think Node moduleResolution should be used with es2017 as a target, but I guess my English is bad...

Now I'll try to see if the naming of display.canvas and the canvas module still conflict.

Would you like a PR of this node display if I manage to make it work correctly ? My use case is pretty specific I'm not sure anyone would use it.

ondras commented 4 years ago

Would you like a PR of this node display if I manage to make it work correctly ? My use case is pretty specific I'm not sure anyone would use it.

Probably not. You might be actually better off mimicking the document.createElement("canvas") call by monkey-patching it using a node-specific loader, something along the lines of

global.document = {
  createElement() { return require("canvas").createCanvas() }
}

...so that this hack remains specific to your scenario -- no adjustments in rot.js might be necessary.

GitNitneroc commented 4 years ago

Sorry for not replaying earlier, and thank you very much for your directions.

For reference, if somebody ever needs this too, I also had to change requestAnimationFrame for a node counterpart, so here are those hacks :

global.document= {createElement : createCanvas};
global.requestAnimationFrame = function(toExecute){
        setImmediate(toExecute);
    };

However, I'm puzzled by the next issue that popped up when trying to use display.draw(). TypeError: Image or Canvas expected Tile.draw() seems to dislike my tileset, which is definitely an Image. I tried using a Canvas instead, but I get the same error. Since tile._ctx.drawImage() is native code, I can't check it, but I just cannot understand what the problem could be with my TileSet. This drawImage function must be from node-canvas module, and it should get along well with node-canvas' Image object.

I just don't see how I could debug this... I'd be very grateful if you had any insight on this problem thanks

ondras commented 4 years ago

However, I'm puzzled by the next issue that popped up when trying to use display.draw(). TypeError: Image or Canvas expected

Again, can you please show the relevant code? In particular,

1) your createCanvas implementation,

2) how do you create your tileSet

So that I can try to reproduce and debug the issue.

GitNitneroc commented 4 years ago

Thank you very much for taking the time to answer, your support is amazing.

I was crafting a readable version of my code to show you and I just fixed the problem ! I don't understand how the little things I changed could solve the issue, so I guess I made something stupid a while back when I was messing with your code. Trying to reproduce in a fresh environement solved my problem...

So, please excuse me for the false issue, and thank you very much for your support and project !

ondras commented 4 years ago

:+1: