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

Publish the official rot.js to npmjs.org #53

Merged
merged 2 commits into from
Feb 26, 2015
Merged

Conversation

statico
Copy link
Contributor

@statico statico commented Feb 26, 2015

(CC @blinkdog)

NPM is the package manager for Node.js. It would be great to publish this awesome package to npmjs.org and make its functionality server-side:

$ node -e 'ROT = require("rot.js"); map = new ROT.Map.Rogue(); map.create(); console.log(map.map)' 
[ [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 ],
  [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 ],
  [ 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1 ],
  [ 1, 1, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 1, 1 ],
  [ 1, 1, 0, 0, 0, 0, 0, 1, 1, 1, 1, 0, 0, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, 1, 1 ],
...

@blinkdog appears to have already made some progress here with the rot-js package. They've done some great work on making a console display. However, that fork is a bit out of date.

I think that there should be one NPM package (say, https://www.npmjs.com/package/rot-js) which points to the primary repo -- this one -- which is kept up to date on NPM. This PR does the minimal work of making that happen.

More information on publishing to NPM: https://docs.npmjs.com/getting-started/publishing-npm-packages

@ondras
Copy link
Owner

ondras commented Feb 26, 2015

Makes sense. I will merge this, but:

  1. the window check must be added to pre-concatenated raf.js as well,

  2. I strongly dislike node's ability to directly assign to module.exports (this has the potential to screw certain things), so I will replace your simple assignment module.exports = ROT with a for-loop.

ondras added a commit that referenced this pull request Feb 26, 2015
Publish the official rot.js to npmjs.org
@ondras ondras merged commit 6bae439 into ondras:master Feb 26, 2015
@ondras
Copy link
Owner

ondras commented Feb 26, 2015

Ah, forget 1), I mis-read the diff.

@ondras
Copy link
Owner

ondras commented Feb 26, 2015

Adjusted the Makefile (https://github.com/ondras/rot.js/blob/master/Makefile#L53) as planned. Done, thanks for your work!

@statico
Copy link
Contributor Author

statico commented Feb 26, 2015

Thanks! It'd be great to get @blinkdog's signoff for publishing as the rotjs repo if they agree.

@blinkdog
Copy link
Contributor

Just two days ago I was thinking "In preparation for 7DRL 2015, maybe I should pull the latest rot.js, port my Node support, and offer a pull request to @ondras". Then, this morning, I see your e-mails about Issue #53 ... nice timing!

I'm agreeable to using rot-js as the repository. let me know which npmjs user names to npm owner add and I'll get them added as soon as I get a chance. If you want to use rotjs instead of rot-js, just go ahead and publish; I'll remove the rot-js package from circulation, to avoid confusion with the official release.

I'm excited that Node.js is becoming an officially supported platform. Back when I published the package, rot.js seemed browser-focused, and I didn't want to bother @ondras with my stuff ("Hey, add my Node-specific code to your browser-focused library!"). But, I noticed a short while ago that a bower package was created, and that's what got me thinking about offering a Node pull request.

As for my code; the Node shim and xterm display support, would you like me to port them to the latest rot.js? If you think they'd be useful, I'm happy to do it. If not, I'll leave my old branch alone.

@ondras
Copy link
Owner

ondras commented Feb 27, 2015

Hmm, this is complex stuff.

I am certain that some parts of rot.js are platform-independent (RNG, FOV, ...), but ROT.Display is one of the most used. I am, however, not sure what features does your shim cover.

We should keep in mind that the ROT.Display API is designed for a browser environment. Some of its features are completely illogical for a native TTY: font size, font family/style, dimensions, padding/spacing, hexes... you probably just want to use what the user's terminal emulator already has, right?

Also, graphical tiles were introduced recently. Bringing them to Node is not going to be easy.

Perhaps some folks experienced with ncurses, bearlib and other native TTY libs would have a reasonable opinion on whether it is worth to adjust ROT.Display for server-side usage (or use some server-specific simpler API instead).

As for the npm package name, I really have no opinion here. I have no interest in running server-side code, so the whole npm quest is just "for other devs out there". Having only one name would make things less confusing for a user, that is for sure.

@blinkdog
Copy link
Contributor

I am certain that some parts of rot.js are platform-independent
(RNG, FOV, ...), but ROT.Display is one of the most used. I am, however,
not sure what features does your shim cover.

The shim is "just enough" browser environment to make ROT.Display comfortable in Node.
Basically, I went through ROT.Display and created stubs for each field/method it accessed.

Some examples:

  • rot.js uses window, in the browser this is the global environment. Node doesn't have window, but the closest equivalent is global. So the shim creates window and makes it an alias to global. Now rot.js can use Node's global without any code changes.
  • rot.js uses document.createElement() to create a <canvas> element to use for the console display. Node doesn't have a document nor a DOM, so the shim creates document with a createElement method. Unlike the browser createElement, which can create any arbitrary DOM element, the shim createElement creates only objects to act as a <canvas> shim, because that's the only type that rot.js ever requests. ROT.Display can use this <canvas> shim object without any code changes.

We should keep in mind that the ROT.Display API is designed for a browser
environment. Some of its features are completely illogical for a native
TTY: font size, font family/style, dimensions, padding/spacing, hexes...
you probably just want to use what the user's terminal emulator already
has, right?

Perhaps some folks experienced with ncurses, bearlib and other native
TTY libs would have a reasonable opinion on whether it is worth to
adjust ROT.Display for server-side usage (or use some server-specific
simpler API instead).

Agreed. There is a lot of ROT.Display that doesn't translate well to a terminal.

That said, the API itself is simple and direct (it doesn't get much easier than draw or drawText). And internally, ROT.Display has some nice universal features as well, like the "dirty" tile tracking and automatic rendering loop.

A goal of mine was to keep the drawing code portable. That is, if I develop a roguelike in Node, I wanted porting to the browser to be a change from layout: "term" to layout: "rect" and done. Or, "The same rot.js you know and love (and it works in Node, too!)".

There are probably some ways to more aggressively refactor ROT.Display. But I decided not to do that, because I thought:

  • It would make it harder for me to merge your future changes to rot.js
  • You would be less likely to incorporate my Node support into the master branch someday
  • It would be rude to publish a rot-js npm package with an incompatible API

And, even if the Node shim and xterm display support are there, the author targeting Node is not required to use them. If one likes the simple ROT.Display API, it can be used without modification (in so far as a terminal can support the features). If not, one can always use their favorite curses library for Node, like blessed, for example.

Also, graphical tiles were introduced recently. Bringing them to Node
is not going to be easy.

Agreed. Doing this will probably require Node bindings for SDL or alike. Especially if it is to achieve any kind of cross-platform support. There is still some research I need to do this in area, as I haven't had a need for graphical tiles in any of my personal projects yet.

My suggestion for integrating these features (if you want them) is to have a separate build target for Node. The standard rot.js (for the browser) doesn't need the shim or xterm support, so no way they should go into the main code. The Node build target can include them and get published to the npm repository.

Do you have an account on npmjs.com @ondras ?

@ondras
Copy link
Owner

ondras commented Feb 27, 2015

Do you have an account on npmjs.com @ondras ?

No, I do not. I can create one, though. No problem.

A goal of mine was to keep the drawing code portable. That is, if I develop a roguelike in Node, I wanted porting to the browser to be a change from layout: "term" to layout: "rect" and done. Or, "The same rot.js you know and love (and it works in Node, too!)".

Ah, a dedicated layout module is a nice approach (I was initially thinking about your shim fixing the existing "rect" and "hex" modules) that keeps most of the code untouched. I like it.

I find the "separate build target for Node.js" approach most straightforward. I assume this implies you maintaining the shim + build repo, pulling current rot.js source in the (build) process? I am able to guarantee you some amount of backwards compatibility, so there shall be no issues when building against a more recent rot.js version.

@blinkdog
Copy link
Contributor

No, I do not. I can create one, though. No problem.

Great! Just let me know your username, and I'll add you as an owner on the rot-js package.

I find the "separate build target for Node.js" approach most straightforward. I assume this implies
you maintaining the shim + build repo, pulling current rot.js source in the (build) process? I am
able to guarantee you some amount of backwards compatibility, so there shall be no issues
when building against a more recent rot.js version.

I was thinking to add some target to the Makefile, like rot.js.node. This would create the Node version of rot.js in a way that follows most Node idioms (code in lib directory, index file pointing to code for require, etc.) Then publishing would be just a simple npm publish (well, as long as you're an owner on npmjs).

I'll pull the latest rot.js, make a branch, and offer you a pull request. 😃

@ondras
Copy link
Owner

ondras commented Feb 28, 2015

Cool, thanks.

My npmjs.com username is "ondras". Let me know when your adjustments are ready.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants