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

Functionally accumulating values #98

Open ianbollinger opened 8 years ago

ianbollinger commented 8 years ago

One problem I've been having in writing PureScript bindings for rot.js is getting data out of the functions that rely on callbacks. Functions like FOV.compute require the user to pass in a callback that is obligated to accumulate mutable state. While this is possible in PureScript, it's inelegant and difficult to compose.

This pull request allows the user to instead accumulate a value in a functional manner. Functions like FOV.compute now take an additional argument, initialValue that is passed through to the callback as the last argument, which then returns the accumulator which is again passed to the callback, and so on. Finally, FOV.compute returns the accumulator when finished. Because these have been tacked on to the parameters list, the change should be backwards compatible. (undefined will be accumulated.)

Optimally, you'd be able to do this for the map and lighting interfaces as well. Unfortunately, they already return a value. The only value they return, however, is this. I'd personally change them to return an accumulator as well, but this might break existing code.

Anyway, I understand this is a weird request and will understand if you don't like it.

PS. Sorry about deleting the trailing whitespace. I keep forgetting to turn of that "feature" of Atom.

ondras commented 8 years ago

Hi Ian,

(removing the trailing whitespace is very OK -- I like that Atom feature myself and let the editor eradicate it freely)

this is a tricky one indeed. I understand your motivation, but I see your proposed changes a little bit too invasive. In particular, they modify the relevant functions without actually improving them; they add complexity (one more argument, caring about returned value) but this complexity is not related to the underlying computation, but rather to the general code/value flow.

I still would like to satisfy your needs. What about some wrapper that takes care about the state management? This is a long blank shot, but I am thinking something similar to

function wrap(callbackBasedGenerator, thisp) {
  let results = [];
  let cb = function() { results.push(arguments); }
  return function() {
    callbackBasedGenerator.call(thisp, cb);
    return results;
  }
}

let pureGenerator = wrap(fov.compute, fov);
let collectedValues = pureGenerator(x, y, R);

I have exactly zero experience with PureScript, so I have no idea how feasible/possible would this approach be. Please let me know what you think...

ianbollinger commented 8 years ago

Well, the problem with that approach (and what I'm currently using) is that it isn't data type agnostic. However, having to accumulate all the arguments in an array and then fold over that array isn't the end of the world. :)