Polishing our Word Puzzle Game

Last post, we created a word matching game and it was functional. But not really pretty. Today we're going to change that. Hopefully by the end the game will not only be visually attractive, but its codebase will be aesthetically more attractive as well.

Code Organization ↩

So, while our goal is going to change the visual elements in front of the user to be better, before we do that, I think we need to address something that's been bothering me since about halfway through the previous post.

Everything is in one big ol' file.

Now, this isn't inherantly a bad thing. 800 lines of code or so really isn't that bad to fit into your noggin. But it was getting tedius, even with vim motions and search-jumping, to move around the codebase and tweak things while we were working last time. So I want to address this. I enjoy using Sublime Text's "find anything" type features to quickly fuzzy find to a class in a file, to use :linenumber to jump to a line, that sort of thing. And it's a lot easier to do that if we break things up a bit.

Now, the logical and nominative thing to do here would be to say of course, let's use the node package manager! or pnpm, or webpack, or grunt, or whatever new flavor of bundler exists. The mere fact that I'm pointing this out should probably tell you that we're about to do something a bit different.

For whatever reason, I found myself compelled to read the documentation on modules the other night. All the way through, and it made me think to myself a bit. Why should I taint my personal computer with npm, nvm, and all of its friends if I could just write modularized code without having to do any of that? The fact that we're doing this project in Javascript does not mean I need to actually hide my dislike of the language. It gets the job done, but I will forever resent it because no matter what one seems to do, you can't not use Javascript if you want to do something on a website.

My gripes aside, this is also a good learning opportunity I think. A moment for us to move beyond programming by coincidence when it comes to our package managers and instead, learn to handle the damn thing ourselves and maybe learn why these various tools that normally handle it for us do the things the way they do. In fact, I think reading the part of the docs on the importmap type actually helps quite a bit.

Import maps allow developers to instead specify almost any text they want in the module specifier when importing a module; the map provides a corresponding value that will replace the text when the module 555555555 URL is resolved.

With that in mind, you can now understand how a package json file probably works with npm and friends. Think, normally you write something like 

const foo = require('packageyouinstalled');
or you write something like 
const { specificfoo } = require('packageyouinstalled');
and probably take for granted that you ran npm install packageyouinstalled, it did a bunch of stuff and now you can require it from wherever in your code and it just works ™. So, let's see if we can make something "just work" and move something to a module.

Taking a look at our code, anything I've defined as a class stands out as something that I'd like to potentially move to its own file. Like I said, I like being able to jump around the code easily and for me that means that following a Java-like convention of keeping 1 class to a file, or a Java-lite approach where there's maybe a few related classes in one file being exported strike me as a good idea. So, let's choose a class. Say, the Sound class from before, and move it to its own file: 

// in sounds.js
export class Sound {
    constructor(path) {
        this.audio = new Audio(path); 
        this.canPlay = false;
        const ref = this;
        this.audio.addEventListener("canplaythrough", (event) => {
            ref.canPlay = true;
        });
    }

    play() {
        if (this.canPlay) {
            this.audio.play();
        }
    }
}

In my main index file, there's two changes we need to do besides removing the now-duplicate class. First, in order to be able to write import { Sound } from 'sound'; at the top of our file. We need to define where one gets that module from: 

<script type="importmap">
    {
        "imports": {
            "sound": "./sounds.js"
        }
    }
</script>

Second, you can't type in import foo from 'bar' if your script tag isn't a module. And, currenty our script tag on the page looks like: 

<script type="text/Javascript">

Thankfully, as far as our top level file that's running the game goes, it doesn't really seem to matter if we change the type attribute to be module instead. Everything will keep on working as expected as noted by the documentation except for one pretty important thing.

Last but not least, let's make this clear — module features are imported into the scope of a single script — they aren't available in the global scope. Therefore, you will only be able to access imported features in the script they are imported into, and you won't be able to access them from the JavaScript console, for example. You'll still get syntax errors shown in the DevTools, but you'll not be able to use some of the debugging techniques you might have expected to use.

A wonderful thing for security. A terrible thing for us doing trial and error in the console during our debugging and fiddling when we were constructing various methods for the Trie last time. I'm pretty sure we can use the debugger to jump in somewhere and do that sort of thing still, but what a bother. Anyway, we get increased code clarity for this loss so I'm hoping it will be worth it. 1

To that effect. I'm going to do two moves right off the bat with the sound. The Sound class moving to its own file as we've already described, and the creation of each instance of that class to that file too. Then, we can export those instances of the classes out to the rest of the program to be used. 

const sound_activate = new Sound('/games/words/SCI-FI_UI_SFX_PACK/Tone2/Fifth/Tone2B_FifthOctaveDown.wav');
const sound_release_wrong = new Sound('/games/words/SCI-FI_UI_SFX_PACK/Tone2/Minor/Tone2B_MinorTriadUp.wav');
const sound_release_right = new Sound('/games/words/SCI-FI_UI_SFX_PACK/Tone1/Major/Tone1B_MajorTriadUp.wav');
const sound_victory = new Sound('/games/words/Collectables Sound Effects Pack/Collectables Sound Effects Pack/Samples/Sound_1/Runs/High/Major Scale H.wav');

export const SOUNDS = {
    activate: sound_activate,
    release_right: sound_release_right,
    release_wrong: sound_release_wrong,
    victory: sound_victory,
};

And update the calller sites in our main script to use these. 

import { SOUNDS } from 'sound';
document.addEventListener(SOUND_EVENTS.ACTIVATE, () => {
    SOUNDS.activate.play();
});

document.addEventListener(SOUND_EVENTS.NO_MATCH, () => {
    SOUNDS.release_wrong.play();
});

document.addEventListener(SOUND_EVENTS.YES_MATCH, () => {
    SOUNDS.release_right.play();
});

document.addEventListener(SCENE_EVENTS.ALLMATCHED, () => {
    SOUNDS.victory.play();
});

Note that I didn't move the event names out. I thought about doing this, but since these are just names of the events, I feel like putting them into multiple disaparate files is a bad idea. If we name an event with the same word or phrase, but don't notice it, that could probably lead to some weird and hard to debug behavior. The events and orchestrating all the points that the program is going to braid our pieces together feel top level to me, so locality of behavior suggests we keep it where it is for now.

Moving right along though, a decently logical separation would be to move the UI elements to their own file. Those classes are taking up quite a bit of space. But they're also leveraging some global variables we've declared. So this might take a bit more work, but let's start small. 

class VictoryScreen {
    draw(ctx) {
        ctx.save();
        ctx.font = '48px Impact';
        ctx.fillText('You win', canvas.width / 2, canvas.height / 2);
        ctx.restore();
    }
}

Moving this class means we need to look decide what to do about the canvas global. Because if we don't, then we'll see something like this:

I think that it would make sense to follow the instructions and notes mentions in the Aggregate modules section. Since then we can write out the import like 

import { VictoryScreen } from 'ui';

From the main file, but place the files into locations like ul/VictoryScreen.js and the like. The only extra bit of boilerplate to do this is that we'll need to define the aggregate file ui.js and include and export for the class from its inner path: 

export { VictoryScreen } from './ui/VictoryScreen.js';

And of course, we need to update the importmap as well: 

{
    "imports": {
        "sound": "./sounds.js",
        "ui": "./ui.js"
    }
}

But now I can load the VictoryScreen class with an import and move it to its own file. But we still need to fix the canvas problem. The canvas isn't a global variable since the type of our script is now a type=module. The simplest thing to do would be to take in the canvas either during the draw method or the constructor. I'd prefer to keep the draw method free of extra, class-specific, arguments. So constructor it is. 

export class VictoryScreen {
    constructor(canvas) {
        this.canvas = canvas;
    }
    draw(ctx) {
        ctx.save();
        ctx.font = '48px Impact';
        ctx.fillText('You win', this.canvas.width / 2, this.canvas.height / 2);
        ctx.restore();
    }
}

And in the index file, we just need to update its usage to pass in the canvas. 

import { VictoryScreen } from 'ui';
let firstTime = true;
document.addEventListener(SCENE_EVENTS.ALLMATCHED, () => {
    if (firstTime) {
        document.dispatchEvent(new Event(SCENE_EVENTS.NEWGAME));
        firstTime = false;
    } else {
        const victory = new VictoryScreen(canvas);
        entities.length = 0;
        entities.push(victory);
        setTimeout(() => {
            document.dispatchEvent(new Event(SCENE_EVENTS.ROUNDOVER));
        }, 1000);
    }
});

That's one down. Now we repeat the process for the other UI elements. So, taking stock of our classes, the next logical one to migrate would be the LetterButton class. Why? Because every other class we've got is using it. The StartSelect uses it, the LetterSelect uses it and the WordRow class users it. It's our basic building block for showing a letter. So, let's move it along and see what breaks.

Copying over the entire class into ui/LetterButton.js is a simple step. Then we can add in the export keyword to get 

export class LetterButton

Followed up by the update to the ui.js aggregate file to re-export the file itself and then I'm able to do this in our main index file: 

import {LetterButton as Foo} from 'ui';
new Foo(1,2,'h');

I haven't removed the LetterButton from the index file yet, in case you're wondering why I'm doing as Foo in the code above. I want my code to execute without error, at least, up until the attempt to instantiate an instance of the class: 

LetterButton.js:5  Uncaught ReferenceError: letter_box_width is not defined
    at new LetterButton (LetterButton.js:5:24)
    at index.html:191:9

Lovely. This was within expections but now we have to actually figure out what to do about these global constants we created in the last post. At the moment, I'm thinking that it makes some degree of sense to simply inline the values, but in an overrideable way. So, rather than having this.radius = 58 / 2 I'll remove radius entirely as a field and replace it with 

radius() {
    return (50 + 8) / 2
}

And then the calling sites just have to be updated to use () instead of just a value. Is this technically more inefficient than it was before? Yes. Yes it is. But polishing work means a bit of animation and interactivity, and being able to make the letter get bigger when you hover over it sounds a whole lot like it'd be useful. We'd have to refactor to use a method to compute the radius to do that anyway, so I don't think we're losing much here. Also we're using javascript. If I was worried about performance I'd be using something that could compile to WASM. Anyway, stuff like the within method inside of the LetterButton can now be updated: 

within(mx, my) {
    if (this.x - this.radius() <= mx && mx <= this.x + this.radius()) {
        if (this.y - this.radius() <= my && my <= this.y + this.radius()) {
            return true;
        }
    }
    return false;
}

And we can move onto the next error we get when trying to create a button: 

LetterButton.js:6  Uncaught ReferenceError: BUTTON_STATE is not defined
    at new LetterButton (LetterButton.js:6:22)
    at index.html:191:9

Unlike the constant. This one isn't something I want to inline. Or, so I thought. But thinking on it, and examining our code. The usage is within just this class. And since I didn't make any other kind of button besides this one, the state of said button's state machine rightfully belongs alongside it. So, we can handily move it in and we don't even need to bother exporting it since no one else but the LetterButton class needs it. 

// no need to even export it. Just dump it in the file with LetterButton
const BUTTON_STATE = {
    IDLE: 0,
    HOVER: 1,
    ACTIVATED: 2
};

Javascript being dynamic means that I no longer see an error in the terminal. Strictly because the constructor is now able to run. But that's not the whole picture is it, so if we change our StartSelect code to create Foo instances instead... 

LetterButton.js:37  Uncaught ReferenceError: CLICK_STATE is not defined
    at LetterButton.update (LetterButton.js:37:34)
    at StartSelect.update (index.html:408:37)
    at index.html:117:23
    at Array.forEach (<anonymous>)
    at update_entities (index.html:115:22)
    at game_loop (index.html:128:13)

It'll break the moment we mouse over a button since that seems to trigger the animation frame itself in the browser. Cool, well, BUTTON_STATE was only used locally, are we lucky enough that we only used the CLICK_STATE constants in one place? 

addEventListener("mousedown", (e) => {
    cursor.click = CLICK_STATE.DOWN;
});
addEventListener("mouseup", (e) => {
    cursor.click = CLICK_STATE.UP;
})

No.

Nope, the click state constants are used not only inside of the LetterButton to check out if we're clicking, but they're used by the top level file to set our cursor object's state. So, let's instead consider where we're using this state: 

update(mx, my, click) {
    if (this.state === BUTTON_STATE.IDLE) {
        if (this.within(mx, my)) {
            this.state = BUTTON_STATE.HOVER;
        }
    } else if (this.state === BUTTON_STATE.HOVER) {
        if (!this.within(mx, my)) {
            this.state = BUTTON_STATE.IDLE;
        } else if (click === CLICK_STATE.DOWN) {        <---- HERE ----
            this.notify(EVENT_NAME.ACTIVATED, this);
            document.dispatchEvent(new Event(SOUND_EVENTS.ACTIVATE));
            this.state = BUTTON_STATE.ACTIVATED;
        }
    } else if (this.state === BUTTON_STATE.ACTIVATED) {
        if (click == CLICK_STATE.UP) {                  <---- HERE ----
            this.state = BUTTON_STATE.IDLE;
            this.notify(EVENT_NAME.RELEASED, this);
        }
    }
}

So, we just want to know if the mouse's click state is up or down. Rather than passing in a singular value for the click and checking a flag like we're doing now... let's instead do this: 

// Still in index.html:
const CLICK_STATE = {
    UP: 0,
    DOWN: 1
};
const cursor = {
    x: 0,
    y: 0,
    click: CLICK_STATE.UP,
    isUp: () => {                                      <--- this is new from here down
        return cursor.click === CLICK_STATE.UP;
    },
    isDown: () => {
        return cursor.click === CLICK_STATE.DOWN;
    }
};
...
function update_entities() {
    entities.forEach((e) => {
        if (e.update) {
            e.update(cursor.x, cursor.y, cursor.click, cursor);
        }
    })
}

Are you scratching your head? Why the heck did he add cursor as a 4th parameter? If that's your thought, then consider the fact that we have a bunch of other code that is still expecting those first three values that we haven't touched yet. I'm a fan of keeping things working, even at a little bit of temporary grossness as the expense. Once we've moved and tweaked all the existing update functions to take in a cursor object we can ditch the first three parameters easily enough.

Anyway, the code we care about and are changing right now is easily updated now: 

update(mx, my, click, cursor) {
    if (this.state === BUTTON_STATE.IDLE) {
        if (this.within(cursor.x, cursor.y)) {
            this.state = BUTTON_STATE.HOVER;
        }
    } else if (this.state === BUTTON_STATE.HOVER) {
        if (!this.within(cursor.x, cursor.y)) {
            this.state = BUTTON_STATE.IDLE;
        } else if (cursor.isDown()) {
            this.notify(EVENT_NAME.ACTIVATED, this);
            document.dispatchEvent(new Event(SOUND_EVENTS.ACTIVATE));
            this.state = BUTTON_STATE.ACTIVATED;
        }
    } else if (this.state === BUTTON_STATE.ACTIVATED) {
        if (cursor.isUp()) {
            this.state = BUTTON_STATE.IDLE;
            this.notify(EVENT_NAME.RELEASED, this);
        }
    }
}

Or well. Mostly updated. 

LetterButton.js:38  Uncaught ReferenceError: EVENT_NAME is not defined
    at LetterButton.update (LetterButton.js:38:29)
    at StartSelect.update (index.html:414:37)
    at index.html:123:23
    at Array.forEach (<anonymous>)
    at update_entities (index.html:121:22)
    at game_loop (index.html:134:13)

We've still got at least two globals still. The EVENT_NAME and the SOUND_EVENTS constants. Harder now than the clicks, these globals are also shared amidst the other classes, but there's not really an object to be attached to. They're also not really encapsulated in the same way, as these are used between the different ui classes to trigger behavior on the subscribers.

So. Let's move these events to their own file and import then from there. A shared concern? A shared place to import from. Simple solution: 

// events.js
export const SCENE_EVENTS = {
    WORDSLOADED: 'wordsloaded',
    NEWGAME: 'newgame',
    ROUNDOVER: 'roundover',
    ALLMATCHED: 'allmatched',
};

export const SOUND_EVENTS = {
    ACTIVATE: 'sfx_activated',
    NO_MATCH: 'sfx_no_match',
    YES_MATCH:'sfx_yes_match'
};

export const EVENT_NAME = {
    ACTIVATED: 'activated',
    RELEASED: 'released',
    SELECTED: 'selected',
    MATCHED: 'matched',
};

And a quick update to our import map and replacing the consts with the import itself: 

<script type="importmap">
    {
        "imports": {
            "sound": "./sounds.js",
            "ui": "./ui.js",
            "event": "./events.js"
        }
    }
</script>
<script type="module">
    import { SCENE_EVENTS, SOUND_EVENTS, EVENT_NAME } from 'event';

And of course, we add import { SOUND_EVENTS, EVENT_NAME } from "event"; into the LetterButton class at the top, and then we've got everything in scope again and the game works! Our update method signature is a bit bloated with that cursor object. But we can fix it as we move over the classes. Speaking of. Our next class to move? The StartSelect class!

Since the StartSelect class only consumes the LetterButton and doesn't use WordRow, I figure that it's worth it to move this one first since it should be easier. For starters we can just create a file in the UI folder and shift it over there plus the obligatory import: 

// this is ui/StartSelect.js
import { LetterButton } from 'ui';

export class StartSelect {
    constructor(x, y, letters) {
        ...

Updating the ui.js file to re-export the StartSelect button lets me import it over in our index file like we did with the LetterButton class. So, Foo once again: 

import {StartSelect as Foo } from 'ui';
new Foo(1,2, []);

And surprisingly, no errors in the terminal. So no constants or glovals to worry about in the constructor. But, if I swap where I use the class definition from the current index file to our newly modularized one? 

StartSelect.js:15  Uncaught ReferenceError: letter_box_width is not defined
    at new StartSelect (StartSelect.js:15:28)
    at setup_start (index.html:582:38)
    at HTMLDocument.<anonymous> (index.html:614:13)
    at index.html:610:26

There it is. surprisingly no one, I had a constant. Unlike before, this one isn't actually used during the drawing method though. It's just a one time constant that I use for spacing things out. I think it makes sense to just make it a local variable then: 

constructor(x, y, letters) {
    this.letters = [];
    this.buttons = [];

    this.x = x;
    this.y = y;
    this.letters = letters;
    this.buttons = [];
    for (let i = 0; i < letters.length; i++) {
        const letter = letters[i];
        const letter_box_width = 50;                  <--------- This is new.
        const lx = x + letter_box_width * i * 1.5;
        const ly = y + letter_box_width * 2;
        const letterButton = new LetterButton(lx, ly, letter);
        letterButton.subscribe(this);
        this.buttons.push(letterButton);
    }
    this.active_buttons = [];
    this.check_words = false;
    this.subscribers = [];
}

The start button letters now appear, and the error shifts over into the draw call that happens once per frame: 

StartSelect.js:42  Uncaught ReferenceError: cursor is not defined
    at StartSelect.draw (StartSelect.js:42:20)
    at index.html:598:39
    at Array.forEach (<anonymous>)
    at draw_entities (index.html:598:22)
    at game_loop (index.html:120:13)
    at HTMLDocument.<anonymous> (index.html:615:13)
    at index.html:610:26

This sounds like it likely relates to our mouse interactions. 

draw(ctx) {
    ...
    ctx.beginPath();
    for (let i = 0; i < this.active_buttons.length; i++) {
        const btn = this.active_buttons[i];
        if (i === 0) {
            ctx.moveTo(btn.x, btn.y);
        }
        ctx.lineTo(btn.x, btn.y);
    }
    ctx.lineTo(cursor.x, cursor.y);
    ctx.stroke();
    ...
}

Ah. Right. When you're dragging around and we're drawing the line between the letters that you've already highlighted, we draw the last segment to wherever your cursor is. We used global state for that since it was simple and easy, but now we've got separation, so we should do what we should have done before. Let's update track the cursor's coordinates via the update method rather than directly in the draw.

By the way, just like with the radius method in the previous class, this also lends itself to a way for us to do some refactoring and polish later. If you drag the mouse around, the line follows you wherever you go. That's probably fine on the start menu where there are no limits or boundaries. But in the other class, LetterSelect, there's a circular boundary around the butters, and it probably wouldn't be a bad idea to add boundaries and not draw the line outside of that maybe.

Anyway! Fixing this is straightforward! In the update method let's assign the cursor to some local state. I'm going to be lazy and just do a simple assignment, which technically could bite us if we ever try to modify that data. But, the StartSelect should always be a readonly participater in this relationship and I feel like doing Object.assign every frame for something we can avoid simply with some programmer discipline is fine. 

update(mx, my, click, cursor) {
    this.cursor = cursor;
    ...
}
                    
draw(ctx)  {
   ...
    ctx.lineTo(cursor.x, cursor.y);
   ...
}

In the constructor we can set the cursor object to {x: 0, y: 0} just in case and then we're good and ready to move on to our— Ah... Our next error 

StartSelect.js:49  Uncaught ReferenceError: width is not defined
    at StartSelect.draw (StartSelect.js:49:49)
    at index.html:598:39
    at Array.forEach (<anonymous>)
    at draw_entities (index.html:598:22)
    at game_loop (index.html:120:13)
    at HTMLDocument.<anonymous> (index.html:615:13)
    at index.html:610:26

Ah, the canvas width again, we saw this with the VictoryScreen before and solved it by passing in a reference to the canvas object in the constructor. We can do the same here and it should be a pretty minimal change of things. Once we do that update, then I'm greeted to the screen working again! And when I press the mouse button down to start a drag

As always. Our friendly white screen tells us there's a problem, and the console tells us what it is: 

StartSelect.js:58  Uncaught ReferenceError: EVENT_NAME is not defined
    at StartSelect.on_button (StartSelect.js:58:22)
    at LetterButton.notify (LetterButton.js:53:33)
    at LetterButton.update (LetterButton.js:39:22)
    at StartSelect.update (StartSelect.js:71:29)
    at index.html:108:23
    at Array.forEach (<anonymous>)
    at update_entities (index.html:106:22)
    at game_loop (index.html:119:13)

Hey we've seen this one before! This one is an easy fix since we already moved the events to their own file and we can import them quite easily. At the top of our ui/StartSelect.js file we add 

import { EVENT_NAME } from "event";

And everything works again! Hooray! We've only got two more classes in our mechanical process to go here. LetterSelect and WordRow. Considering that to create the StartSelect class we copied the former, it should be as simple as applying the same transformations to that file, right? Doing the export import dance and then instantiating our new Foo(1,2,[]) class like we did before nets us no errors, and then swapping it in to replace the version using the globals gets us 

LetterSelect.js:15  Uncaught ReferenceError: letter_box_width is not defined
    at new LetterSelect (LetterSelect.js:15:63)
    at setup_entities (index.html:482:38)
    at HTMLDocument.<anonymous> (index.html:559:13)
    at HTMLDocument.<anonymous> (index.html:546:26)
    at GameState.on_match (index.html:205:30)
    at WordRow.on_select (index.html:146:49)
    at StartSelect.update (StartSelect.js:77:37)
    at index.html:108:23
    at Array.forEach (<anonymous>)
    at update_entities (index.html:106:22)

Ah well that's familiar! Wait, but this time we've got a bit more to consider. The user of these globals are in the constructor and the draw method. The use of the value in the constructor is the same as it was in the other class, just a local thing we can set in there. The use in the draw method though? 

ctx.arc(this.x, this.y, letter_box_width * 3, 0, 2 * Math.PI);

It's sort of the same. But also, different. It's being used to compute the radius of the background for the letter select input. So, rather than have it hardcoded like this with the constant, I'm going to treat it like the draw method in the VictoryScreen class and move it to a helper. 

ctx.arc(this.x, this.y, this.backgroundRadius(), 0, 2 * Math.PI);

And just like that, the game is working again.

So one last class move to go! The WordRow class can be moved on over to its own file, we add an export keyword, do the import in the ui file and then trying to create it in the main index file to check its constructor like usual results in: 

WordRow.js:8  Uncaught ReferenceError: ROW_STATE is not defined
    at new WordRow (WordRow.js:8:22)
    at index.html:126:9

Aha, yet another set of state constants! We can check to see if it's used in the rest of the file, and... since it's not. We can just add it to the top of the new file and the error disappears. 

const ROW_STATE = {
    IDLE: 0,
    MATCHED: 1
};

export class WordRow {
    ...

So now we move on from the Foo usage and replace the actual class, as I'm sure that draw or update will probably be mad at us in some shape or form. A quick /%d%dd in vim motions later and removing the as Foo from my import and 

WordRow.js:41  Uncaught ReferenceError: letter_background_color is not defined
    at WordRow.draw (WordRow.js:41:25)
    at index.html:463:39
    at Array.forEach (<anonymous>)
    at draw_entities (index.html:463:22)
    at game_loop (index.html:120:13)
    at HTMLDocument.<anonymous> (index.html:480:13)
    at index.html:475:26

At this point these are getting decently routine, so inspecting where this constant is leads us to the draw method, and looking at its usage it's all pretty local. Besides the letter_background_color value, there's also our usual friend of letter_box_width, the related letter_box_height, and a new one of letter_color. All of these feel like they can become local to the class and I won't really mind updating them individually. 

const letter_background_color =  'rgb(200 200 200 / 50%)';
const letter_color = 'black';
const letter_box_height = 50;
const letter_box_width = 50;

Refreshing shows me I missed a spot with a padding which is just as easy to localized, and then the page loads without a broken white screen again. At least, until I actually trigger an event by dragging and clicking on the buttons. Which attempts to send out a notification to the listening word row and 

WordRow.js:21  Uncaught ReferenceError: EVENT_NAME is not defined
    at WordRow.on_select (WordRow.js:21:22)
    at StartSelect.update (StartSelect.js:77:37)
    at index.html:108:23
    at Array.forEach (<anonymous>)
    at update_entities (index.html:106:22)
    at game_loop (index.html:119:13)

Pretty sure we've seen that before and can fix it with import { EVENT_NAME } from "event"; and that'll resolve that. Dragging the mouse across the S T A R T buttons now triggers an update to the word row and then the game kicks up. We're back in business! With this shifting of the UI components around, we can clean out any constants related to them in the main index file and then shift our attention to the meatier classes, like the TrieNode implementation.

The hard part of course is where the trie should live. It's not a UI thing after all. So I guess maybe we just plop it into its own file and call it a day? The only other remaining class is the GameState and I think we need to assess that one with various other helpers to figure out where or if it should be moved around. So, I guess we can just put it in its own file. 

<script type="importmap">
    {
        "imports": {
            "sound": "./sounds.js",
            "ui": "./ui.js",
            "event": "./events.js",
            "trie": "./trie.js"
        }
    }
</script>

And moving the class into trie/js almost works. I get the error that tells me we were using a helper method: 

trie.js:51  Uncaught ReferenceError: coinFlip is not defined
    at TrieNode.randomWord (trie.js:51:13)
    at TrieNode.randomWord (trie.js:56:34)
    at TrieNode.randomWord (trie.js:56:34)
    at TrieNode.randomWord (trie.js:56:34)
    at TrieNode.randomWord (trie.js:56:34)
    at TrieNode.randomWord (trie.js:56:34)
    at TrieNode.randomWord (trie.js:56:34)
    at setup_entities (index.html:164:41)
    at HTMLDocument.<anonymous> (index.html:250:13)
    at HTMLDocument.<anonymous> (index.html:237:26)

That's not too hard to fix though. The coinFlip function is only used by this class after all, so just like the constants that were local to their ui classes, we can just move that function into the same file and keep it private. Technically, the coinFlip function is a sort of generate helper, so it could live in a place for those. But we haven't made a home for that yet, and I'm not sure if there are other functions that would end up living there or not yet, so for now let's go ahead and keep the locality of behavior high. 

function coinFlip() {
    return Math.random() < 0.5;
}

export class TrieNode {
    ...

Since everything was so self contained, that was the only issue it seemed. Not a bad thing, but I was sort of expecting a little but more tribulation to give me some time to think and mull over what to do with GameState. So I laid in bed for a while with my eyes closed. Thinking about the phrase "yak shaving" and wondering if anyone out there in the greater world is goig to appreciate my thought process as I work through refactoring all this code we wrote last time or not. Is this useful? Does this amuse people or help anyone think about things in a different way once they're done with this blog post? Why do so many breakcore song compilation have titles like "music to disassociate to" or other sorts of things. How many random questions can I write down before I finally get around to moving this last piece of code and quit procrastinating on the polishing that I don't really want to put the effort into yet?

Quite a few it seems. But after resting my eyes for a couple hours and beating a challenge quest in FGO I was back to typing this up. You see, the GameState classes biggest bother is its name. It's subscribed to the WordRow instances, and is used to fire off a couple events for sound and screen transition once you've found all the words. It is game state, but it's not really all of it. It feels more like a sort of WordRowListener to me. Or a MatchChecker, or maybe RoundState. It's used in both the setup_start and setup_entities functions to facilitate knowing that all the rows are completed and we should do something.

Basically, I was having so much trouble thinking about it because I think I named this thing wrong last time.

So I renamed GameState to MatchState since that felt more appropriate given its role in both the transition on-match from the start to game screen, and its handling of game screen to victory screen. Then I went to bed because it was 1:30am. In the morning after I finished my workout around 8, I was looking at the code again after my morning workout and asked myself a new question. "Does this need to be a class?" 

class MatchState {
    constructor(words) {
        this.words_to_find = words;
        this.found_words = new Set();
    }

    is_game_done() {
        return this.words_to_find.length === this.found_words.size;
    }

    on_match(name, word) {
        if (name !== EVENT_NAME.MATCHED) {
            return;
        }

        if (this.words_to_find.includes(word)) {
            this.found_words.add(word);
            document.dispatchEvent(new Event(SOUND_EVENTS.YES_MATCH));
        }

        if (this.is_game_done()) {
            document.dispatchEvent(new Event(SCENE_EVENTS.ALLMATCHED));
        }
    }
}

I mean, tracking the potential words and which words they've found so far is a related thing. And I need to have something be subscribed to the WordRow to listen for when it's matched. So, as its written now, yes, I need to have a listener set up in this way. But, if I were to change things around, maybe use the document event system rather than me custom on_match calls and friends?

Nah that's a bad idea. We'd have to track more stuff if we did that I think. Specifically, when we did something like: 

document.addEventListener(WORDMATCHEVENTNAME, matchState.on_match)

We'll need to remember, when we're starting up a new round to remove the event listener before we discard the match state. 

removeEventListener(WORDMATCHEVENTNAME, matchState.on_match)

Which is also something I don't think I can do with an arrow function unless I were to explicitly assign it to a variable which sort of defeats the nice way that reads. So. After this long back and forth with myself, I think I've basically come to the point of deciding that I'm going to leave the MatchState class sitting in the index file with the rest of the code that's orchestrating the main game loop and setup. No need to move it to a different place. 2

That said. There is one piece of code I do want to still move. And that's 

function letters_used(words) {
    const allLetters = Object.keys(words.join("").split("").reduce((accum, l) => {
        accum[l] = l;
        return accum;
    }, {}));
    const leftovers = words.map((word) => {
        let leftover = word;
        allLetters.forEach((l) => { 
            leftover = leftover.replace(l, '');
        });
        return leftover;
    }).filter(Boolean);
    if (leftovers.length) {
        allLetters.push(...letters_used(leftovers));
    }
    
    return allLetters;
}

This, unlike functions like game_loop or update_entities, is just a helper method. It exists solely for us to grab the unique set of letters that allow one to create the given words. It's not about orchestrating the events, or the primary game loop. It's only called by the two setup_* methods. This doesn't feel like something that should be in our main file that's handling the game. So where does it belong?

Well. My instinct, which I will follow, is to place this code alongside the Trie. Why?3 Because, conceptually, the letters of the words are just the letters from the Trie. So they're close to each other in my mind, and so when I think about "how do I get the letters" I'm also typically tacking on "how do I get the letters from the words I got from the Trie?" and so in 2 months when I'm looking at this code with older eyes, I'll be more likely to look there and find it. 

import { letters_used, TrieNode } from 'trie';

Feels right to me.

You know what doesn't feel right to me still though? That stupid annoying little one time use variable of firstTime that's used in the event handler for going from the start menu to the game screen. 

let firstTime = true;
document.addEventListener(SCENE_EVENTS.ALLMATCHED, () => {
    if (firstTime) {
        document.dispatchEvent(new Event(SCENE_EVENTS.NEWGAME));
        firstTime = false;
    } else {
        const victory = new VictoryScreen(canvas);
        entities.length = 0;
        entities.push(victory);
        setTimeout(() => {
            document.dispatchEvent(new Event(SCENE_EVENTS.ROUNDOVER));
        }, 1000);
    }
});

This bothers me. Not because the code offends proponents of "clean code" that follows unix philosophies of doing only one thing at a time. But because it forces us to have this little toggle variable. That doesn't even make sense if we were to give the option to the user to go back to the start screen. What if we add in a score system or an options menu? Does it make sense for the variable to be called firstTime then?

No. No it doesn't. So let's obliterate it. 

function firstTime() {
    document.dispatchEvent(new Event(SCENE_EVENTS.NEWGAME));
    document.removeEventListener(SCENE_EVENTS.ALLMATCHED, firstTime);
}
document.addEventListener(SCENE_EVENTS.ALLMATCHED, firstTime);
            
function otherTimes() {
    const victory = new VictoryScreen(canvas);
    entities.length = 0;
    entities.push(victory);
    setTimeout(() => {
        document.dispatchEvent(new Event(SCENE_EVENTS.ROUNDOVER));
    }, 1000);
}

All I've done is move the branches of the if into separate functions, then attach and remove each from the document's event handler list in place of a global switch. Remember how I said I'd like to be able to read this code in a month though? Let's give them more meaningful names so it's easier to do that: 

function startMenuNewGameHandler() {
    document.dispatchEvent(new Event(SCENE_EVENTS.NEWGAME));
    document.removeEventListener(SCENE_EVENTS.ALLMATCHED,  startMenuNewGameHandler);
}
document.addEventListener(SCENE_EVENTS.ALLMATCHED, startMenuNewGameHandler);
            
function victoryOnAllMatchHandler() {
    const victory = new VictoryScreen(canvas);
    entities.length = 0;
    entities.push(victory);
    setTimeout(() => {
        document.dispatchEvent(new Event(SCENE_EVENTS.ROUNDOVER));
    }, 1000);
}

Now. Just, one more teensy thing before we move onto the next step of our polishing progress. Look at this code: 

const potentialWords = new TrieNode();
fetch("words.txt")
    .then((response) => response.text())
    .then((w) => { 
        window.words = w.split("\n");
        window.words.forEach((word) => {
            if (word.length >= 3) {
                potentialWords.insert(word)
            }
        })
        document.dispatchEvent(new Event(SCENE_EVENTS.WORDSLOADED))
});

It's loading the words file and letting us know when it's got our dictionary setup for the game. All well and good, yeah? Well. Take a gander at this section of the imports page. I got kind of excited when I first read through this: we can load CSS files with an import? It made me think maybe we could also replace the above code with an import statement and not have to do all this fetching and whatnot.

Sadly, lists off only json and css as options. And there's a note on the page

Note: The actual type attribute value does not correspond directly to the MIME type. It's separately specified by the HTML specification.

And that specification page doesn't really note anything else. So. It seems like my dream of a handler for plain text is out. Which is too bad honestly. But, we live with what the browsers give us I guess. As a finaly reference, the changes that we've accomplished so far are these:

Class or CodeNew Location
Soundclass in sounds.js
sound_*SOUNDS object in sounds.js
VictoryScreenVictoryScreen in ui/VictoryScreen.js
SCENE_EVENTS, SOUND_EVENTS, EVENT_NAMEexports in events.js
LetterButtonLetterButton in ui/LetterButton.js
StartSelectStartSelect in ui/StartSelect.js
LetterSelectLetterSelect in ui/LetterSelect.js
WordRowWordRow in ui/WordRow.js
GameStateWas renamed to MatchState and left alone
firstTime variableRefactored code into two event handlers to remove it

And now that the internals are a bit more manageable, it's time to expand on them!

Polish 1: Giving the user a guide ↩

It's always nice to have a bit of a clue when you're playing a game on where to go next. In a game where you have to guess the words, it's a lot harder if theres no rhyme or rhythm to it. Like, having related words maybe makes things easier. But even without semantic meaning, it's useful to at least put the words in a consistent order.

Consider the words ace, art, ape, part, tart. When you're looking at these and you've got no sense of what order they're in, then you can see this:

And have nothing to go on at all. Is it tar? rat? What could the 3rd word be? But, consider if instead it looked like this:

And you knew that the words shown were in alphabetical order. It would stand to reason that you can deduce that the 2nd word starts with an A. And that it's the second letter has to be at least c, and at most r, and any letter inbetween is fair game. It's simple to see that acp acr aren't words, but act is a real word and fits into place, so you could guess that!

So, just by doing a quick one line change when we setup the game, we can give the user a way to reason and make educated guesses on the word list, rather than blindly guessing over and over again. 

const word = potentialWords.randomWord();
const words = potentialWords.restrictedWalk(3, 8, word, 7);
words.sort();

Not bad, right?

Polish 2: Button breathing and the update refactor ↩

Let's take advantage of the change we did when we added in the radius method to the LetterButton. It's not really enough to do just that. Like, sure, we could toggle between two different sizes. But it'd be better to animate it wouldn't it?

In order to do that, we need to know what time it is (Since anything done over time means we need to actually track time itself). So, first things first, let's add the time since the last frame to the update loop. Looking at the first example from the MDN website we can adapt something like this: 

function game_loop(timestamp) {
    if (!ctx) {
        console.log('No canvas found?')
        return;
    }
    ctx.clearRect(0, 0, canvas.width, canvas.height);
    update_entities(timestamp);
    draw_entities(ctx);
    requestAnimationFrame(game_loop);
}

let lastTimestamp;
function update_entities(timestamp) {
    if (lastTimestamp === undefined) {
        lastTimestamp = timestamp;
    }
    const elaspedMilli = lastTimestamp - timestamp;
    entities.forEach((e) => {
        if (e.update) {
            e.update(cursor.x, cursor.y, cursor.click, cursor, elaspedMilli);
        }
    })
}

Since we're updating the update method, let's resolve the thing we left as a TODO during the code organization section. We don't need to pass the individual x y click fields from cursor if we're passing the whole thing. So let's change the call site to e.update(cursor, elaspedMilli) and then watch everything break! 

LetterButton.js:32  Uncaught TypeError: Cannot read properties of undefined (reading 'x')
    at LetterButton.update (LetterButton.js:32:36)
    at StartSelect.update (StartSelect.js:72:29)
    at index.html:124:23
    at Array.forEach (<anonymous>)
    at update_entities (index.html:122:22)
    at game_loop (index.html:111:13)
    at HTMLDocument.<anonymous> (index.html:212:13)
    at index.html:102:26

Since we moved all the ui entities to the ui module, it's easy to know where to go and do our updates! 

games/words/ui\LetterButton.js
    30:    update(mx, my, click, cursor) {

    games/words/ui\StartSelect.js
    69:    update(mx, my, click, cursor) {
    72:            this.buttons[i].update(mx, my, click, cursor);

    games/words/ui\LetterSelect.js
    43:    update(mx, my, click, cursor) {
    46:            this.buttons[i].update(mx, my, click, cursor);

Just the three places then! That's really not so bad. And since we just hae to copy and paste in update(cursor, elaspedMillis) in each place, it's really not much work at all. The work comes when it's time to do the radious fun!

Let's track the number of milliseconds and accumulate it inside of each letter. 

update(cursor, elaspedMillis) {
    if (!this.millis) {
        this.millis = 0;
    }
    this.millis = (this.millis + elaspedMillis) % 2000;
    ...

While I'm getting started, I'm going to just hardcode 2 seconds in as the maximum amount to accumulate to. We can move that to a constant in the class once we've got things working. Now, we can tweak the radius method to go from a hardcoded value to a computed one: 

radius() {
    const sinusoidal = (s) => {
        return Math.sin(s * 2 * Math.PI);
    }
                
    const radius = 29;
    if (this.state === BUTTON_STATE.HOVER) {
        const dx = sinusoidal(this.millis / 2000);
        const extension = 2;
        return 29 + extension * dx;
    }
    return radius;
}

Annnd uhm. Well. I think something is wrong:

I stared at this for a while. Tried out lerping the value, dividing things. Thinking about the amplitude and period of the function. Wondering where the heck I was going wrong. Logging things out to the console. The usual suspects. It took a while to realize that the problem isn't actually anything todo with the radius method at all. Nope. It's the code we snagged from MDN! 

let lastRenderTimestamp;
function update_entities(timestamp) {
    if (!lastRenderTimestamp) {
        lastRenderTimestamp = timestamp || 0;
    }
    const elaspedMilli = timestamp - lastRenderTimestamp;
    lastRenderTimestamp = timestamp;                       // <---------- this fixes the problem!
    entities.forEach((e) => {
        if (e.update) {
            e.update(cursor, elaspedMilli);
        }
    })
}

The code from MDN sets up an example where the elapsed time is from when the page first started requesting frames! Which doesn't really work with something that was expecting the delta time between frames. Once I corrected that I started getting something less jittery and awful:

With a basic example working, I went ahead and updated the code to remove the hard coded value and make the pulse continue once the button is activated: 

radius() {
    const radius = 29;
    if (this.state === BUTTON_STATE.HOVER) {
        const dx = sinusoidal(this.millis / this.hoverAnimationPeriodMillis);
        const extension = 2;
        return radius + extension * dx;
    }
                
    if (this.state === BUTTON_STATE.ACTIVATED) {
        const dx = sinusoidal(this.millis / this.hoverAnimationPeriodMillis);
        const extension = 1;
        return radius + extension * dx;
    }
                
    return radius;
}

Now this works. But before we move along. I think having all the buttons pulse at the same time is fine for the StartSelect. But the screen we'll be spending most of our time on is the one with the LetterSelect. So let's offset the delta time for each letter, just a little bit: 

update(cursor, elaspedMillis) {
    this.cursor = cursor;
    for (let i = 0; i < this.buttons.length; i++) {
        this.buttons[i].update(cursor, elaspedMillis + i);
    }

And the result?

Cool. I like the way the pulse feels in giving the game a bit more life. But, I think it's obvious to see what I'd like to tackle next here for the polishing.

Polish 3: Colors and theme configuration ↩

So. The colors the current game has are... questionable. The random dark blue on the activate button looks bad. The contrast between things isn't particularly great. It just, generally, doesn't look like something that has had any time put into it. Which, admittedly, I didn't spend much time on the colors in the last post because I was more interested in the letter selection and the trie work. I'm not a designer, but I think I can do better.

A few weeks ago I watched a pretty good video on website design. The main takeaway that I got from it was to start with black and white. Then add some highlight colors in, be sparing and intentional. Now, that was for website design. So I don't know how well it holds up for a small game. But I'm willing to give it a try! I suppose the first thing to do is to make all the colors into variables that I can control from one location so that it's easy to try out various techniques. Nearly all of our colors are from setting the fillStyle field when we draw, so all we have to do is collect all of those! 

$ rg fillStyle games/words/
games/words/ui\LetterSelect.js
66:        ctx.fillStyle = 'lightblue';

games/words/ui\WordRow.js
47:        ctx.fillStyle = letter_background_color;
58:                ctx.fillStyle = 'black';

games/words/ui\LetterButton.js
88:        ctx.fillStyle = this.state_color[this.state];
96:        ctx.fillStyle = 'black';

games/words/ui\StartSelect.js
31:        ctx.fillStyle = 'lightblue';
46:        ctx.fillStyle = 'black';

So, taking the state_color into account. I suppose we've got black, lightblue, rgb(200 200 200 / 50%) (letter_background_color), and then the three state_colors: which are lightcyan, cyan, blue. That's quite a few different colors! Let's move all of these to a colors file and give them decent names. I think it would make sense to name them based more on their file locations and what they're used for as oppose to something like "primary", "secondary", "tertiary" or "highlight". So: 

// ./ui/colors.js
export const LETTER_SELECT_STYLE = {
    FILL: 'lightblue',
    STROKE: 'lightyellow',
    PATH: {
        FILL: 'lightblue',
        STROKE: 'lightyellow',
        LINE_WIDTH: 10,
    }
};

While I know we're going to go greyscale in a moment, I couldn't help updating the code to make these apply and tweaking things a bit by adding in the line width for the letter select. Just having the thicker line does a lot I think:

But the colors still need work, so let's keep up the pace and define some more styles! Given that I've included the line width here, I suppose maybe I shouldn't call this file colors.js and instead call it something like styles.js. But that's a pretty minor tweak, the more interesting thing of course is how we should structure the LetterButton's styles because it's not just one static thing, there are multiple states for the button and we expect to change the colors for each: 

export const LETTER_BUTTON_STYLE = {
    IDLE: {
        BACKGROUND: {
            FILL: 'lightcyan',
            STROKE: 'lightyellow',
        },
        FONT: {
            FILL: 'black',
            STROKE: 'white',
        },
    },
    HOVER: {
        BACKGROUND: {
            FILL: 'cyan',
            STROKE: 'lightyellow',
        },
        FONT: {
            FILL: 'black',
            STROKE: 'white',
        },
    },
    ACTIVATED: {
        BACKGROUND: {
            FILL: 'blue',
            STROKE: 'lightyellow',
        },
        FONT: {
            FILL: 'black',
            STROKE: 'white',
        },
    },
};

Breaking out the styles like this makes it a lot easier to update the existing code as well. Before, we have just the color styles being added to the object keyed by the state flag. But now we can just do the whole object for that: 

constructor(x, y, letter) {
    ...
    this.state = BUTTON_STATE.IDLE;
    this.state_color = {};
    this.state_color[BUTTON_STATE.IDLE] = LETTER_BUTTON_STYLE.IDLE;
    this.state_color[BUTTON_STATE.HOVER] = LETTER_BUTTON_STYLE.HOVER;
    this.state_color[BUTTON_STATE.ACTIVATED] = LETTER_BUTTON_STYLE.ACTIVATED;
    ...
}
...
draw(ctx) {
    ctx.save();
    const style = this.state_color[this.state];
    ctx.fillStyle = style.BACKGROUND.FILL;
    ctx.strokeStyle = style.BACKGROUND.STROKE;
    ...

We can repeat this for the StartSelect since that's pretty similar. The current code doesn't draw any sort of background (like how LetterSelect has that circle) but if we wanted to tweak that, then it wouldn't be too hard to define a fill and stroke for that as well as the path for the line between the buttons that are activated: 

export const START_SELECT_STYLE = {
    FILL: 'lightblue',
    STROKE: 'lightyellow',
    PATH: {
        FILL: 'blue',
        STROKE: 'lightyellow',
        LINE_WIDTH: 10,
    },
    FONT: {
        FILL: 'black'
    }
};

We need a FONT section too, because the StartSelect is responsible for drawing in the text for Click and drag to spell. Which... Maybe we should move to its own little class instead? It doesn't really feel like our start select class is for just the start button, but rather, the entire start screen. Which is a bit different. Now that I think about it though, wouldn't that be the perfect thing to make to handle the setup code that we have in setup_start?

But I think I'm just looking for excuses to not have to tweak colors, so let's push past that urge to fiddle for now and just accept the changes we've done to the file to make things configured by our style constant and our new background color we've added for fun: 

// in index.html 
function setup_start() {
    ...
    for (let i = 0; i < words.length; i++) {
        const wordRow = new WordRow(word_x, y, word);
        wordRow.draw = () => {};                      // <---- THIS IS NEW
    }
    ...
}

// in StartSelect.js
draw(ctx) {
    ctx.save();
    ctx.beginPath();
    ctx.fillStyle = START_SELECT_STYLE.FILL;
    ctx.strokeStyle = START_SELECT_STYLE.FILL;
    ctx.fillRect(0, this.canvas.height - this.height, this.canvas.width, this.height); // <-- new
    ctx.fillStyle = START_SELECT_STYLE.PATH.FILL;
    ctx.strokeStyle = START_SELECT_STYLE.PATH.FILL;
    ctx.lineWidth = START_SELECT_STYLE.PATH.LINE_WIDTH;
    for (let i = 0; i < this.active_buttons.length; i++) {
        ...

I'm not including every change, but just enough to give the gist of it, as well as to highlight the new pieces of code that enable us to now render the screen like this:

As noted before, we'll swap to grayscaling everything in a bit, just need to get a central place to control it all first! To which, we need to update one more place. The WordRow class that renders on the main game screen while the user plays. We can define its current style behavior like so 

export const WORD_ROW_STYLE = {
    BACKGROUND: {
        FILL: 'rgb(200 200 200 / 50%)',
        STROKE: 'black'
    },
    LETTER: {
        FILL: 'black',
        STROKE: 'black'
    }
}

Then it's just a matter of importing the values into the file, tweaking the assignments, and we've got the exact same result as before! So. Let's move right along and make everything feel like an early motion picture, it's black and white time!

First up, let's deal with the CSS values that are controlling the space around the game. 

body {
    background-color: lightpink;
}
            
canvas {
    background-color: white;
}

My eyes prefer dark mode colors, so I'm going to move away from the light pink here for something a bit easier and more neutral on the eyes. 

body {
    background-color: black;
}
            
canvas {
    background-color: darkcyan;
}

This honestly looks a lot better to me already and I haven't even done anything I said I'd do about graying up the UI constants yet!

Now, let's do that. If I go ahead and convert the colors we wrote into the style exports earlier into some quick constants we can see at a glance that we've got quite a few colors in use. Maybe too many honestly. 

const BUTTON_IDLE = 'lightcyan';
const BUTTON_HOVER = 'cyan';
const BUTTON_DOWN = 'blue';
const OUTLINE_ACCENT = 'lightyellow';
const CONTROL_BACKGROUND = 'lightblue';
const FONT_COLOR = 'black';
const FONT_COLOR_INVERT = 'white';
const BOX_BACKGROUND = 'rgb(200 200 200 / 50%)';

Part of me wonders if CSS variables work in this context. To test, I added 

:root {
    --test: 'red';
}

to the index file, and then attempted to see if const FONT_COLOR = '--test'; would work. It uhm. Did not.

Which is too bad, since it'd be really nice to be able to define them on the index page so we could just do CSS related things that way. But, hey, at least we learned something! 

const BUTTON_IDLE = 'white';
const BUTTON_HOVER = 'gray';
const BUTTON_DOWN = 'darkgray';
const OUTLINE_ACCENT = 'white';
const CONTROL_BACKGROUND = 'lightgray';
const FONT_COLOR = 'black';
const FONT_COLOR_INVERT = 'white';
const BOX_BACKGROUND = 'rgb(200 200 200 / 50%)';

Washing things out at least tells us one important thing. The background looks better when it lets the word rows stand out a bit more. In fact, I honestly didn't mind the dark cyan we used before. So, let's go back to that since it's simple and easy to enjoy. And, while I'm at it, I'll just use one of those nice websites that give us various shades and color wheel type stuff based on the color we give it. Using a bunch of these and I land on something that's relatively easy on the eyes. 

const BUTTON_IDLE = '#abd0cf';
const BUTTON_HOVER = '#80b9b8';
const BUTTON_DOWN = '#002323';
const OUTLINE_ACCENT = 'white';
const CONTROL_BACKGROUND = '#359696';
const FONT_COLOR = 'black';
const FONT_COLOR_INVERT = 'white';
const BOX_BACKGROUND = 'rgb(171 208 207 / 50%)';

It might just be fatigue, but this type of color selection, tweaking, and staring doesn't really spark any joy in me. I prefer being given a design system from someone whose training is graphically inclined. If it feels passable enough, then I'm inclined to pass on by. So, before I run out of steam on writing this post and trying to improve this thing. Let's call this ok and shift along to the next polishing item of potential interest.

Polish 4: visual feedback on matches ↩

Right now, when you make a match, the word appears in the list and that's that:

It would be nice if there was some sort of something that made you feel like something had actually happened and you had got it right (or wrong). So let's do something! Since we've separated things out into classes and made them more easily able to reason about their own state without mixing in global variables this should be straightforward.

Currently, WordRow has no update method because it had no need for one. It's been getting by with the simple log of 

const shouldDisplayLetters = this.state === ROW_STATE.MATCHED

in its draw method to determine if it should be bothered to draw the letters or not. So, let's add some state: 

this.fadeInTimer = 0;
this.maxAnimationTimeMs = 1000;

I'm sure we'll tweak the timing, but for now a second will probably do and I'll just place it in the constructor so we set it once. We should only really need to run the timer once for now for each WordRow. And if I want to make the text go from invisible to visible, we can leverage the globalAlpha property to do just that: 

draw(ctx) {
    ...
    const shouldDisplayLetters = this.state === ROW_STATE.MATCHED;
    for (let j = 0; j < this.word.length; j++) {
        const x_offset = this.x + letter_box_width * j + j * padding;
        ctx.fillRect(this.x + x_offset, this.y, letter_box_width, letter_box_height);
        if (shouldDisplayLetters) {
            const letter = this.word[j];
            ctx.save();
            ctx.strokeStyle = WORD_ROW_STYLE.LETTER.STROKE;
            ctx.fillStyle = WORD_ROW_STYLE.LETTER.FILL;
            ctx.globalAlpha = this.fadeInTimer / this.maxAnimationTimeMs; 
            ctx.font = "48px serif";
            ...
}

update(cursor, elaspedMillis) {
    if (this.state !== ROW_STATE.MATCHED) {
        return;
    }

    this.fadeInTimer += elaspedMillis;
    if (this.fadeInTimer > this.maxAnimationTimeMs) {
        this.fadeInTimer = this.maxAnimationTimeMs;
    }
}

Remember that our update_entities method checks to see if a given entity has an update method before calling it, and then proceeds to do so. So by nature of us adding the update function we're not getting the time between frames and can use that to increment our timer. A simple linear transformation of the alpha property does make things feel a bit more... alive:

But it's also a bit slow. And doesn't really "pop". Perhaps because we're treating the word as, well, a word, and not like you just built it up out of letters. So what if we treated it more like a step function? Rather than a slow and smooth transition, what if we had each letter pop in one at a time as the clock ticked? 

const percentage = this.fadeInTimer / this.maxAnimationTimeMs;
ctx.globalAlpha = percentage >= (j / this.word.length) ? percentage : 0 ;

This feels better to me. The way the letters populate the word's row now feels like it's in line with how the user selected them. It's all subjective, but it's got a good feel to it I think. One more tweak though! It still doesn't feel like it's "poppy" enough to me. So, I tried doing some more experimentation, rather than just having the opacity linearly fill in, what if I change the color? 

const letter = this.word[j];
ctx.save();
const percentage = this.fadeInTimer / this.maxAnimationTimeMs;
if (percentage !== 1.0) {
    gradient.addColorStop(0, WORD_ROW_STYLE.LETTER.FILL);
    gradient.addColorStop(percentage, WORD_ROW_STYLE.LETTER.STROKE);
    gradient.addColorStop(1.0, "white");
    ctx.fillStyle = gradient;
} else {
    ctx.fillStyle = WORD_ROW_STYLE.LETTER.FILL;
}

ctx.globalAlpha = percentage >= (j / this.word.length) ? percentage : 0 ;

It's still a bit odd at the end, it snaps to the last color too fast and the entire gradient disappears suddenly. So... if instead, I make the first color stop follow the percentage like so: 

const percentage = this.fadeInTimer / this.maxAnimationTimeMs;
if (percentage !== 1.0) {
    gradient.addColorStop(percentage - percentage / (j + 1), WORD_ROW_STYLE.LETTER.FILL);
    gradient.addColorStop(percentage, WORD_ROW_STYLE.LETTER.STROKE);
    gradient.addColorStop(1.0, "white");
    ctx.fillStyle = gradient;
} else {
    ctx.fillStyle = WORD_ROW_STYLE.LETTER.FILL;
}

It feels a bit better. Though it still has the same problem as before. Where the last letter basically never gets a time to shine. Or in this case, for the gradient to flash across it before all the letters settle into place and become the usual static black. So, how do we fix that?

The first idea that comes to mind is to use a non-linear function. Rather than advancing the time up bit by but at a steady rate, what if we instead use an easing function? Potentially, this could hide the rough edge by pushing us past it in the first place so that the last letter gets a shine to it ahead of when it's finished being displayed by the percentage. There's just one problem of course: 

WordRow.js:75  Uncaught IndexSizeError: Failed to execute 'addColorStop' on 'CanvasGradient': 
       The provided value (-0.00138042) is outside the range (0.0, 1.0).
    at WordRow.draw (WordRow.js:75:30)
    at index.html:132:39
    at Array.forEach (<anonymous>)
    at draw_entities (index.html:132:22)
    at game_loop (index.html:112:13)

These types of bouncy functions that go past the range... well. They go past the range! And color stop very explicitly wants us to draw inside the lines. We could use an easing function that stays within the lines, but when I gave the "easeOutExpo" on that website a try, it didn't crash but it didn't look good either. So, I asked around a little bit, shared these gifs, and got some feedback from people. 

const percentage = this.fadeInTimer  / this.maxAnimationTimeMs;
if (percentage !== 1.0) {
    gradient.addColorStop(percentage - percentage / (j + 1), WORD_ROW_STYLE.LETTER.FILL);
    gradient.addColorStop(percentage, WORD_ROW_STYLE.LETTER.STROKE);
    gradient.addColorStop(1.0, "white");
    ctx.fillStyle = gradient;
} else {
    ctx.fillStyle = WORD_ROW_STYLE.LETTER.FILL;
}

ctx.strokeStyle = WORD_ROW_STYLE.LETTER.STROKE;
ctx.globalAlpha = percentage;

Some feedback was that the animation should be faster, so I tweaked maxAnimationTimeMs to be 250 rather than 1000. Additionally, the letters appearing faster so that it the player can see it's worked and get back to working on the circle of letters. Which was all decent advice. The last piece of feedback was something I was already planning on doing though. We need visual feedback where we just selected the letters. And we've got all the tools to do it!

The WordRow are able to notify any subscriber when they've got a match right? And we want the letter selection circle to do something when there's been a match. You can probably see where we're going to go here. In our setup_entities method, we don't just subscribe the match state to the events from the WordRow, but also the instance of the letter selection: 

const wordRow = new WordRow(word_x, y, word);
wordRow.subscribe(matchState);
wordRow.subscribe(letterSelections);
letterSelections.subscribe(wordRow);
entities.push(wordRow);

It might feel a bit odd that they've both subscribed to each other, but given that neither calls or emits events to each from within those methods that are going to cause recursion at a distance, we're okay. So, just like before with the word row, we need to setup a simple timer for state and we'll keep track of how long it should go. This time, we'll also put in a flag for whether or not it should be on or not, because we'll need to be able to toggle it for each match we get over the course of a round: 

this.matchedAnimationTimer = 0;
this.matchedAnimationTimerMaxMS = 500;
this.matchedAnimationEnabled = false;

Within the update method, we need to increment the timer if it's enabled and also handle resetting it: 

if (this.matchedAnimationEnabled) {
    if (this.matchedAnimationTimer > this.matchedAnimationTimerMaxMS) {
        this.matchedAnimationTimer = 0;
        this.matchedAnimationEnabled = false;
    }
    this.matchedAnimationTimer += elaspedMillis;
}

Our timer will increment once it's turned on, but we need to actually turn it on. That's done by implementing our on_match handler that we've setup the subscription for already: 

on_match(eventName, word) {
    this.matchedAnimationEnabled = true;
}

And lastly, we need to actually do something with the fact that the timer is running! Just to show that everything is working as expected, let's just make the background color change for the duration of the timer. That will tell us that the skeleton is working as expecting without us going into the weeds yet: 

draw(ctx) {
    ctx.save();
    ctx.beginPath();
    ctx.fillStyle = LETTER_SELECT_STYLE.FILL;
    ctx.strokeStyle = LETTER_SELECT_STYLE.STROKE;
    ctx.arc(this.x, this.y, this.backgroundRadius(), 0, 2 * Math.PI);
    if (this.matchedAnimationEnabled) {
        ctx.fillStyle = 'red';   
    }
    ctx.fill();
    ctx.stroke();      
    ctx.closePath();
    ...

Does it work?

Yes it does! Looks terrible, but ask any nerd who's every had to troubleshoot CSS and they'll tell you about how they'll set the background colors to garish and awful primaries that clash and conflict just to be able to spot where the borders and padding have all gone wrong. Anyway, let's ditch the red and make something else happen. For now, I'll stick with white and re-use the idea that gradients look cool when animated. 

ctx.arc(this.x, this.y, this.backgroundRadius(), 0, 2 * Math.PI);
if (this.matchedAnimationEnabled) {
    const percentage = this.matchedAnimationTimer / this.matchedAnimationTimerMaxMS % 1.0;
    const gradient = ctx.createRadialGradient(
        this.x, this.y, this.backgroundRadius() * percentage,
        this.x, this.y, this.backgroundRadius()
    );
    gradient.addColorStop(0, LETTER_SELECT_STYLE.FILL);
    if (percentage < 1.0) {
        gradient.addColorStop(percentage, 'white');
    }
    gradient.addColorStop(1, LETTER_SELECT_STYLE.FILL);
    ctx.fillStyle = gradient;
}
ctx.fill();
ctx.stroke();

I'm thinking that we'll change this hardcoded white to be a positive green for a successful match if we also want to create something like an event for when nothing matched. Because then we could do a red color. Though I'm waffling a bit because both the white and the green look cool:

Though honestly, I think the white is a bit more radiant, ba dum tsh. the green is more subtle, which maybe is better if I was playing in a dark room. For now, I'll stick with the white I think. And let's figure out if we can actually make use of the red. Examining our code, is it actually possible for us to tell that a word matched none of anything?

The reason I ask is because right now, we're relying on the individual WordRow instances to check if the word made by the LetterSelect matches their word and if it doesn't they ignore it. After all, each of those instances can't tell that the word doesn't match the other WordRows in existance. So, we need to step back one step higher. To the MatchState because that class is tracking all the words.

However, MatchState is relying on the individual WordRow instances to call on_match and trigger its method that's doing this: 

on_match(name, word) {
    if (name !== EVENT_NAME.MATCHED) {
        return;
    }

    if (this.words_to_find.includes(word)) {
        this.found_words.add(word);
        document.dispatchEvent(new Event(SOUND_EVENTS.YES_MATCH));
    }

    if (this.is_game_done()) {
        document.dispatchEvent(new Event(SCENE_EVENTS.ALLMATCHED));
    }
}

If you stop and think for a minute, specifically about the fact that a WordRow will never call on_match if there is no match then you may realize our impase here. Even if we were to remove the early return, it doesn't change that nothing is going to call us. And so, we need to do something about that. Recalling that the LetterSelect class is the one that takes the user input and fans it out to the buttons (via the subscriptions), we can reasonably say that hey! Why don't we just have the match state subscribe too!

This feels like we're making an italian meal, but I suppose clustered intricacies of state and their friends often do feel like handling spaghetti. But... it certainly should work: 

// in our setup_entities method
const letterSelections = new LetterSelect(origin_x, origin_y, letters, canvas);
letterSelections.subscribe(matchState);

// in our MatchState class 
on_select(name, word) {
    if (name !== EVENT_NAME.SELECTED) {
        return;
    }

    if (!this.words_to_find.includes(word)) {
        console.log('Do something :)');
    }
}

And making a bad incorrect match on the inputs? 

Do something :)

Wonderful. The place I want to do something is in the LetterSelect class again. I want to make the highlight be red for a fail, and white for a successful match. So... let's make a new event type: 

export const EVENT_NAME = {
    ACTIVATED: 'activated',
    RELEASED: 'released',
    SELECTED: 'selected',
    MATCHED: 'matched',
    NO_MATCH: 'nomatch',
};

Then setup the match handler within LetterSelect to reason about that as I've described: 

on_match(eventName, word) {
    if (eventName === EVENT_NAME.MATCHED) {
        this.matchedAnimationEnabled = true;
        this.matchedAnimationColor = 'white';
    }
    if (eventName === EVENT_NAME.NO_MATCH) {
        this.matchedAnimationEnabled = true;
        this.matchedAnimationColor = 'red';
    }
}

And we have to update the draw method to use that this.matchAnimationColor variable instead of the hardcoded color we've been using. The only thing to do after that? Make something trigger the match event for the LetterSelect. 

class MatchState {
    constructor(words) {
        this.words_to_find = words;
        this.found_words = new Set();
        this.nomatchers = [];
    }

    subscribe_to_nomatch(subscriber) {
        this.nomatchers.push(subscriber);
    }

    on_select(name, word) {
        if (name !== EVENT_NAME.SELECTED) {
            return;
        }

        if (!this.words_to_find.includes(word)) {
            document.dispatchEvent(new Event(SOUND_EVENTS.NO_MATCH));
            for (let i = 0; i < this.nomatchers.length; i++) {
                this.nomatchers[i].on_match(EVENT_NAME.NO_MATCH, word);
            }
        }
    }
}

I am breaking our convention a bit here with the specific subscribe_to_nomatch versus a generic subscribe event. But the MatchState feels rather... nexial I suppose. And I wonder if we start to extend things more, if it will need to emit more events to more types of subscribers. I suppose maybe I shouldn't care, after all, we always pass the name of the event down first and let subscribers care or not about that. And we could do a similar thing we've done in our update loop, checking to see that the entity (or in this case, subscriber) has a method defined on themselves that can be called or not.

Sigh. I suppose I've talked myself out of my non-generic method here. Not that it really changes much code. Just the setup site: 

const letterSelections = new LetterSelect(origin_x, origin_y, letters, canvas);
letterSelections.subscribe(matchState);
matchState.subscribe(letterSelections);
entities.length = 0;

Anyway, minor refactoring aside. This is more importantly, functional:

And now, looking at the list of things I said in the last blog post about what sort of things we could do, I find myself staring down the barrel of the one thing I know I should do but have been dreading this entire time.

Polish 5: The dictionary ↩

By far my least favorite thing about my little word puzzle clone is that its dictionary, frankly, sucks. I'm fully willing to take some of the blame that my restricted walk idea from the initial post might come up with waaaay too many letters sometimes, but it doesn't change the fact that a list of words like: 

['GURRY', 'GUSSY', 'GUY', 'RUG', 'SUSS']

In the debug console are simply things that no normal human with a functioning brainstem would guess. For one, I thought I had found a scrabble dictionary last time, proper nouns (last I checked) aren't allowed to be matches in scrabble!

As I've stated a few times, I very much don't want to do this task. However, I know which library can help us go from 178686 words to... well, hopefully something smaller if that's what needs to happen to remove what feels like garbage. The nltk library like before! Except, this time, we're going to filter the heck out of the dang thing!

By far the biggest issue is that there's all these obscure, useless words that are technically dictionary words, but that no person in their right mind would think of. Like, "ell", which is a unit of measurement in tailoring, is not suitable for a word meant to be a fun way to pass the time. XLVIII? Sure, that's a roman number, and that'll be in the dictionary (because where else are you going to put it?) But it's not something that someone is going to think of when you say "word search" since you know, it's a number. 

ROMAN_REGEX = re.compile(r"^(?=[MDCLXVI]+$)([MDCLXVI]+)$", re.IGNORECASE)
common_words = {
    'was', 'has', 'his', 'her', 'our', 'its', 'one', 'all', 'any', 'who', 'how', 'you',
    'your', 'they', 'them', 'what', 'when', 'where', 'this', 'that', 'into', 'onto',
    'over', 'under', 'some', 'more', 'each', 'much', 'many', 'most', 'least', 'just',
    'like', 'even', 'ever', 'with', 'from', 'then', 'than', 'back', 'very'
}

def is_good_word(word):
    return (
        word.isalpha() and            # No punctuation, digits
        word.islower() and            # Avoid proper nouns, acronyms
        len(word) > 2 and             # Minimum word length
        not ROMAN_REGEX.match(word) and 
        not looks_like_abbreviation(word)
    )

def looks_like_abbreviation(word):
    return (
        word.isupper() or                             # Already all caps
        (len(word) <= 5 and word.isalpha() and        # Shortish
        word.upper() == word and                     # All letters and all caps
        word.lower() not in common_words)            # Not a known lowercase word
    )

Doing this filtering and using the wordnet corpus this time felt like it helped a bit: 

words = set()
for synset in wordnet.all_synsets():
    for lemma in synset.lemmas():
        word = lemma.name()  # Can contain underscores and punctuation
        if not is_good_word(word):
            continue
    ...

And bringing 178,686 words down to around 62k did feel like an improvement. But ell was still here, as was some other random words that just didn't seem common enough. And so. Fed up with just using nltk, I decided to hit it with a bigger hammer: 

pip install wordfreq

The nice thing about wordfreq is that we can use it to get the frequency of any word we like. And, more specifically, we can use it to filter things out that simply aren't common enough to be known to people. 

# Keep only words with Zipf frequency >= 4.0 (about 1 per 10,000 words)
def is_common(word):
    return zipf_frequency(word.lower(), 'en') >= 4.0

The frequency we're using here has some definitions from the library page here, the important bit being

zipf_frequency is a variation on word_frequency that aims to return the word frequency on a human-friendly logarithmic scale. The Zipf scale was proposed by Marc Brysbaert, who created the SUBTLEX lists. The Zipf frequency of a word is the base-10 logarithm of the number of times it appears per billion words. A word with Zipf value 6 appears once per thousand words, for example, and a word with Zipf value 3 appears once per million words.

Well doesn't that sound lovely? Once I add my is_common filter into our is_good_word method we arrive at just 4848 words in our dictionary. A massive reduction, and... if I actually try playing my game without cheating with the debug list?

Holy crap it's working. Though also, wups. I forgot about the whole "make the victory screen not garbage" idea. I suppose we'll need to do that next huh. And here I thought I was going to be done with this blogpost. But I suppose it's true what I've heard about game development, the last 20% is the hardest and takes the longest. It was simple to test out the idea of making buttons with lines between them in a circle, as well as simple to get something displayed on the screen. But it took a lot more time to refactor, make things easier to tweak, and add and play with parameters to make things look nice.

Though a large part of that was probably just my sleeping half my weekend away multiple weeks in a row instead of sitting down and writing like I am now. Wups. Anyway, here's the python script I just used to make the dictionary in case you also run into the problem of there being too many weird nonsensical words in these dictionaries to think about: 

import nltk
from nltk.corpus import wordnet
import string
import re

# Make sure the WordNet corpus is available
nltk.download('wordnet')
from wordfreq import zipf_frequency

# Keep only words with Zipf frequency >= 4.0 (about 1 per 10,000 words)
def is_common(word):
    return zipf_frequency(word.lower(), 'en') >= 4.0

def looks_like_abbreviation(word):
    return (
        word.isupper() or                            # Already all caps
        (len(word) <= 5 and word.isalpha() and       # Shortish
        word.upper() == word and                     # All letters and all caps
        word.lower() not in common_words)            # Not a known lowercase word
    )

ROMAN_REGEX = re.compile(r"^(?=[MDCLXVI]+$)([MDCLXVI]+)$", re.IGNORECASE)
common_words = {
    'was', 'has', 'his', 'her', 'our', 'its', 'one', 'all', 'any', 'who', 'how', 'you',
    'your', 'they', 'them', 'what', 'when', 'where', 'this', 'that', 'into', 'onto',
    'over', 'under', 'some', 'more', 'each', 'much', 'many', 'most', 'least', 'just',
    'like', 'even', 'ever', 'with', 'from', 'then', 'than', 'back', 'very'
}

def is_good_word(word):
    return (
        is_common(word) and
        word.isalpha() and            # No punctuation, digits
        word.islower() and            # Avoid proper nouns, acronyms
        len(word) > 2 and             # Minimum word length
        not ROMAN_REGEX.match(word) and 
        not looks_like_abbreviation(word)
    )

# "apples" but not "apple"
def is_plural_junk(word):
    return (
        word.endswith('s') and
        not wordnet.synsets(word, pos=wordnet.NOUN)
    )

words = set()
for synset in wordnet.all_synsets():
    for lemma in synset.lemmas():
        word = lemma.name()  # Can contain underscores and punctuation
        if not is_good_word(word):
            continue
        if is_plural_junk(word):
            continue
        words.add(word.upper())  # Final output format: ALL CAPS

with open("clean_dictionary.txt", "w", newline='\n') as f:
    for word in sorted(words):
        f.write(word + '\n')

print(f"Saved {len(words)} words to clean_dictionary.txt")

Wrap up ↩

I will note that we are still missing some of the features I mused about at the end of the last post:

  • Pleasant imagery: such as backgrounds
  • Background music: something nice to listen to while you play, or a mute button to not.
  • Reset/Give up/Shuffle buttons: being able to give up would be nice, being able to shuffle the buttons around to spark ideas would be too
  • Scoring systems.
  • A proper victory screen and time to savor your well earned win.

That's five things... we did five sections of polish this time, after we did the code organization. Which took a while! So we've actually accomplished quite a bit. And I think this blog post is running a bit long in the tooth. My stamina is low, and it's 10:40pm on a Sunday night and I haven't touched a video game4 all day. So I'm going to call our work done for now, and I'll see you in the next post!

(Though I might potentially take a break to think about a different word problem that was on my mind recently.)