dwmkerr / crosswords-js

Tiny, lightweight crossword control for the web.
https://dwmkerr.github.io/crosswords-js/
MIT License
71 stars 27 forks source link

expose cleanCell and resetCell helpers #40

Closed mmkal closed 8 months ago

mmkal commented 9 months ago

also allow them to be reset to a particular letter

@dwmkerr I've applied the equivalent of this as a local patch to allow collaborative playing - basically it lets me programmatically update a cell's content

Default behaviour is unchanged, so this is non-breaking.

mmkal commented 9 months ago

Hi Misha/@mmkal, there is an bug in line 267: cleanCell -> content.

You're absolutely right, corrected.

Stylistically, I don't think overriding cleanCell and resetCell is a good solution as your change doesn't align with the semantic intent of the function name, which makes the code harder to grok.

That's fair. Maybe a better implementation would involve these existing two functions calling another with a name more aligned with its job. Having said that, I do want to reset the cell's cheat-shame and wrongness-marker state - so resetCell felt about right. I don't use cleanCell for now.

Also, you mentioned previously that your downstream application needs to set the cell content remotely. Without seeing your code, I don't see how you're passing the cell id over the wire , as cell is a local object reference?

Yes, resetCell is called locally. I have a supabase pubsub system, and messages are broadcast containing cell x,y coordinates. The local code receives the message, gets the local cell reference and calls resetCell.

I have another PR about to land with a new method exposed: CrosswordController.setGridCell(cellElementId, character), where cellElementId is the string id value for the cell ($row,$column).

Yes it sounds like that would work!

I've optimised the CellMap in the PR by using cellElement ids as hash values. The cellElement.id is the id value of the associated modelCell - which now has a toString() method which returns ($row,$column) .

Does that align with what you want to achieve?

Sounds good to me. One note - I noticed that on squares.io each cell element has a data-xy="2,3" html attribute. In my project I extended the CrosswordjsController to add that attribute and make E2E testing much easier. Maybe crosswords-js should follow that convention? i.e. x=2, y=3 would have id 2,3 rather than (2, 3). Either way it would solve my problem. However I don't think there's any harm in exposing at least resetCell as a low-level helper for advanced users to try at their own risk.

pvspain commented 9 months ago

Maybe I'm just an old dog - yes, I am an old dog :) - but using cartesian (x,y) coordinates for a grid hurts my brain!

Two real-world grid examples are maps (latitude, longitude) and matrices (row, column) (mebbe not so real world!) where the vertical dimension precedes the horizontal dimension.

Ultimately, we should use what feels natural for our end-users, whom are puzzle-setters in this case. Maybe I'm drawing a long bow, but I think they are more likely to come from a linguistic or humanities background than a mathematical or comp sci background.

That's my $0.02! I'm not rusted on to row-column.

@dwmkerr set it up originally using cartesian coordinates - what's your vote here Dave?

pvspain commented 9 months ago

I'm just having a look at squares.io. They reference supported formats (.puz, .jpz, .ipuz, .rgz). I'll see how cell coords are referenced in those formats. If they're all cartesian then we have a winner!

pvspain commented 9 months ago

I've had a chat with one of the puzzle setters for the SMH here in Australia. He recommends investigating IPUZ. It's an open JSON format with specs for a bunch of puzzle types - crossword, sudoku, acrostic... PUZ is popular but its proprietary and not officially documented, so has possible legal implications. I've documented PUZ and i'll look into IPUZ today.

mmkal commented 9 months ago

I've been using https://www.npmjs.com/package/@confuzzle/puz-crossword to parse .puz files and it works pretty well. I'm perfectly happy using that for now but could be worth looking at that for implementation hints.

pvspain commented 9 months ago

I've been using https://www.npmjs.com/package/@confuzzle/puz-crossword to parse .puz files and it works pretty well. I'm perfectly happy using that for now but could be worth looking at that for implementation hints.

Interesting. Neither PUZ nor IPUZ look anything like our format! In IPUZ, the puzzles are laid out cell-by-cell in row-major order starting in the top left hand corner. It is human-readable, but verbose and not text-editor friendly. I guess most people use software to compile puzzles. My contact says he uses Crossword Compiler to generate the puzzles. It looks like a good bit of (proprietary) software.

According to the guys that reverse engineered PUZ, it uses a heuristic to determine the grid layout! I'm surprised that would even be deterministic. The joy of brute-force!

pvspain commented 9 months ago

@confuzzle/puz-crossword has a low dependency count, which is nice! There's a python library on PyPi for ipuz

mmkal commented 9 months ago

Here's one more that I'm told one of the people setting crosswords for my app has been trying: https://github.com/viresh-ratnakar/exet - that also has a companion solver UI in JavaScript, https://github.com/viresh-ratnakar/exolve, which seems similar in its goal to this repo. I haven't looked into either of those yet but just in case you hadn't come across them.

pvspain commented 9 months ago

Thanks Misha. I had a quick look at those repos. There are a lot of crossword apps out there!

pvspain commented 9 months ago

The formats I've investigated, including your puzzle setter above, have all encoded grids. Very different to the approach in this repo. It reminds me of the two streams in computer graphics - raster graphics (laid out pixel-by pixel) and vector graphics (annotated vectors). This repo is in the vector graphics camp!

pvspain commented 9 months ago

Hi Misha @mmkal!

I've reverted the cell id to be '(x,y)', i.e. cartesian coordinates, in PR42.

pvspain commented 8 months ago

Hi @mmkal, Is there anything outstanding here - have you tried controller.setCell() from PR#42? Can we close off this PR?

mmkal commented 8 months ago

I'll try it out and open a new one if necessary. Thanks!