-
-
Notifications
You must be signed in to change notification settings - Fork 256
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
Display/Backend refactor #217
Conversation
This changes some settings in the tsconfig.json file to make this repo work with the new TypeScript project reference mechanism. It also adds some partial type declarations in the Term display backend so that including projects don't have to pull in the @types/node package. Two notable changes to the tsconfig.json file: - exactOptionalPropertyTypes is now set to true, so optional properties will no longer automatically have undefined added to their types. - sourceMap is now set to true, so the rot.js build process will create source maps; this allows including projects to view the .ts file rather than the .js output when following links or in devtools.
This removes the direct reference from Display to Backend, along with (theoretically) allowing library consumers to define their own Display backend. This also changes the DisplayData type to be an object interface rather than a tuple definition, as those are friendlier both to TypeScript and to the JS engine.
With this change, each display backend (layout) can specify its own Options interface as well as its own option defaults. The Display itself now only stores the literal options passed to the constructor or setOptions, and the Display.getOptions method takes an optional parameter for whether to return the effective options (the default) or only the caller-specified ones. The Term backend uses the ability to specify defaults to make the Display default to the width and height of the stdout terminal.
Following up the previous commit, this allows each display backend to specify a different type for the three components of display data: characters, fg style, and bg style. This means that a Display created for a tile backend (which allows arrays for all three components) will accept either an array or a bare string as arguments to the draw() method, while a Display created for a term backend (which has no character overlay support) only allows bare strings in the signature.
Now that each backend can specify its own types for allowable characters, the tile and tile-gl layouts can use number or symbol keys for each tile, rather than requiring that each "character" be a string.
This is a customization from Deiphage; it allows you to pass a number rather than a string for the fg-style on a Tile-backed display, which allows for per-tile opacity customization without the overhead of tileColorize.
Yeah, this is quite a lot to chew on. Especially since my ts-fu is not really up-to-date; I have no idea what Also, some CI checks are failing - probably because your IDEs TS implementation is a lot newer than the one used in the check/build process? I understand that most of the work is done purely to satisfy TS with the Display multi-morphic nature. I agree with the concept, but I also note that this makes the codebase a lot more complex to navigate and/or comprehend. A wild idea: is it really useful/necessary to try providing a unified ROT.Display API for all compatible backends? We are somewhat caught in the OOP inheritance trap, trying to shape all backend implementations into a common abstract interface. This introduces quite a lot of complex TS annotations and gives just one bonus: the ability to easily switch different backends/layouts. But: is someone actually doing that? Is there a point in having a This is something I am thinking about quite a lot recently, as I am planning an independent project with a DOM-based TTY-like renderer. And the differences between text, individual characters, tiles, gl, hexes etc. are simply too large for a unified interface. |
Hmm, those are good points - and for sure, being able to switch layouts on the fly is probably not a particularly useful bit of functionality. I'd been making an effort to keep the JS API unchanged, but since you're fine with larger breaking changes here, I'll do another pass on this with an eye towards simplifying and streamlining the codebase 😄 (It's also interesting that you mention a DOM-based renderer - I'd been thinking while I was working on this refactor that it wouldn't be too hard to make a DOM-based Display backend!) |
Hmm, one thing I'm thinking is, I think I'm going to make a distinction between backdrop and foreground items, on account of the fact that in roguelikes, it's extremely common to use a static or mostly-static background with just a few actors moving around. Being able to describe that a character has moved from one coordinate to another is a more intuitive than repainting the old position with a floor glyph and repainting the new one with the character glyph. (It also allows for, say, movement effects, especially with a DOM-based renderer: all it takes is a CSS Given that, and just thinking aloud, the display subsystem can be broken up into a few different components:
Any thoughts? This architecture should work well for all the things I've done or plan to do with Deiphage, but you've got a lot more experience with the roguelike genre than I do - what use cases have I not accounted for here? |
Yeah. DOM-based rendering is something I actually experimented with way baaack in my first JS roguelike (https://ondras.github.io/js-like/), but it turned out to be bad for performance (many DOM elements back in ~2008 or so). But the situation is different today; browsers are more performant, we have the Web Animations API and I am particularly interested in beautiful vector fonts with large glyphs. Will let you know once I find some time to actually implement something. |
This is not actually a pull request; I don't plan to take this out of draft state, as this isn't a particularly feature-rich change. This is, however, the end result of the first major bit of work I have planned, so I wanted to give you a chance to look over it and have opinions. This non-PR encompasses two changes:
Display genericization
The TL;DR of this is that the
Display
class is now generic; it will present different capabilities depending on the options object (and most specifically itslayout
property) used to construct it. The type signature isDisplay<TLayout, TChars, TFGColor, TBGColor>
, and all of these are set from theoptions
parameter. For example, creating a hex-layout Display withwill result in a Display object whose
draw()
method is typed as:since the Hex backend can support multiple characters (the
string[]
TChars type argument), but only a single foreground and background color (thestring
type arguments). On the other hand, the TileGL backend supports multiple values for all three parameters, the construct signature and resulting draw() method are:Note that, when explicitly typing variables and properties, only the first type parameter, TLayout, needs to be declared, and the other three will take their default values for the given backend:
Tile backend improvements
The above is most of the code change involved here, and all of that was in service of being able to pull some small tweaks to the
Tile
backend from the Deiphage codebase into rot.js proper. The major one is that you can now pass a number (or array of numbers) to thefg
parameter of Display.draw(), and they will be used as the opacity with which to draw the tile or tiles. So, the expanded signatures for the tile layout are:The other change is that the
Tile
andTileGL
backends will now accept tile mappings withsymbol
ornumber
keys, as well as those withstring
keys, or any combination or restriction thereof. For example, you can pass an array of [x, y] tile coordinates as thetileMap
parameter, in which case it acts as a number-keyed map:This is a fairly hefty code change, so you may want to go commit-by-commit on this one if and when you look through it; I've done my best to keep the history at least fairly coherent. And, as always, you're under no obligation, but if you do look it over, I'd certainly appreciate any feedback or thoughts!