Custom improvements/changes to SC2, maybe of interest for v3?

[Arkerthan]

I work on a larger game and we added some improvements mainly to increase speed, Main changes are:

1. Reduce the number of clone() calls on passage change. For maxHistory = 1 vanilla SC makes 3 clone() calls, we got it down to 1. No idea if this is possible for higher maxHistory, though I am pretty sure at least one clone() could be removed.
2. Disabled save compression for auto saves
3. Increase the loading speed of the saves menu by storing each slot separately and store an extra object holding the saves description so only that needs to be loaded to display the saves menu.

Not sure if any of this interests you or how far along you are with v3 and it's only really important on games with a large state.

In any case, link to our repo if interested: https://gitgud.io/Arkerthan/sugarcube-2

There's also a list of all our changes in the README.

[tmedwards]

Cloning

1. Reduce the number of clone() calls on passage change. For maxHistory = 1 vanilla SC makes 3 clone() calls, we got it down to 1. No idea if this is possible for higher maxHistory, though I am pretty sure at least one clone() could be removed.

Removing the instance from historyDeltaEncode() should probably be okay with the codebase as it currently stands.

Removing the instance from momentCreate() means that references are preserved through navigation, which is a problem—in general, at least. If any of code, even accidentally, begins to depend on that behavior, then it will break on session/save load as references cannot currently be preserved when roundtripping through serialization. You may be able to make that tradeoff for your project, but it's not something that I can, or should, force on everyone.

 

Save compression

2. Disabled save compression for auto saves

Synchronous browser storage will likely always be a very limited commodity—certainly Web Storage is likely to be. Trading space usage for computation time may be the right tradeoff for your project, but I think I'll stick with space efficiency for now.

As point of consideration. You may be better served by dropping the delta coding step altogether, rather than compression on the autosave. No idea on the how that would play out for your project, but that may be a better tradeoff.

NOTE: I'm unsure how you're providing access to your project, but as an FYI. If you're giving players the files for local play, then you should know that different browsers handle local Web Storage differently. E.g., Firefox gives each path its own storage instance, while Chrome—probably all Blink-based browsers—gives you a single storage instance for all local play.

I described this in more detail once at tfgames.site, relevant parts quoted below:

Web Storage instances are segmented by origin and some browsers differ in how the file:// schema demarcates origins.

Chrome does work as you've described, one origin for local files—most, if not all, browsers based on Chromium probably do.

Not sure about Safari, but it's likely in the Chromium camp.

Firefox is probably the odd duck out. It treats the file:// schema similar to how it treats the http:// schema, specifically it grants separate origins based on the full directory path—i.e., file:///C:/games/alfa/game.html and file:///C:/games/bravo/game.html have separate origins, thus separate Web Storage instances.

The gist here is that Chrome users, if playing locally, are more likely to need the space savings—even if it's only on the one autosave, since your sharing with anything else they're storing locally. Your players may never run into trouble because of it, but I'd keep an eye out just in case.

 

Save manifest

3. Increase the loading speed of the saves menu by storing each slot separately and store an extra object holding the saves description so only that needs to be loaded to display the saves menu.

Having separate manifest and data objects for saves isn't a bad idea. The current system is, mostly, a holdover from v1, when developers' aspirations were more modest.

 

SugarCube v3

Not sure if any of this interests you or how far along you are with v3 and it's only really important on games with a large state.

Per v3:

1. How game state is recorded is changing significantly, so cloning won't be a concern in the future.
2. While plans are to use IndexedDB as the primary storage provider, compression is likely to remain—most people aren't playing on potatoes, so the computational hit shouldn't be an issue; especially in light of the state changes.
3. Saves are also changing, somewhat because of state changes, but I do like the idea of having a separate manifest so I'll probably end up doing something to that effect.

[tmedwards]

Posting this here since I don't really want to create accounts at every site who feels the need to reinvent the damn login wheel.

Here's a tip or two for other changes in your custom version.

→ Ignore expired array

If your project doesn't use anything that uses the expired moment array—i.e., the visited family of functions or State.hasPlayed()—then I'd simply comment out/remove that whole if statement in stateMarshal(). There's no point in adding an .expired property at all if it's always going to be an empty array.

        // I.e., comment out or remove this whole `if` statement block.
        if (_expired.length > 0) {
            stateObj.expired = [..._expired];
        }

I'd also find the following, in the historyCreate() function:

        if (Config.history.maxStates > 0) {
            while (historySize() > Config.history.maxStates) {
                _expired.push(_history.shift().title);
            }
        }

And make the following change:

        if (Config.history.maxStates > 0) {
            while (historySize() > Config.history.maxStates) {
                _history.shift();
            }
        }

I.e., do shift expired moments off the history array, but do not append them to the expired moments array. Basically, there's no need to push moment passage names onto the expired array at all if you're only going to discard them later.

It would be cleaner to remove all expired moment related code, but the above will get you the benefits without having to muck around with anything else.

 

→ Use render() instead of a local wikifier, making it possible to render DOM...

Use render() instead of a local wikifier, making it possible to render DOM directly in special passages

I assume that I'm looking at the current <Passage>.render() method from the fc branch. If not, then this may be off.

You've replaced a call to the Wikifier with a call to <Passage>.render(), which itself calls the Wikifier. The balance of the changes being something like the following.

Original code:

• Trims the processed text.
• Renders directly to the page element.

Changed code:

• Does not trim the processed text.
• Creates a document fragment and renders to that, only later appending its contents to the page element.
• Checks an if conditional—I'm assuming that it will always fail in your case, so you aren't also updating the excerpt.

To illustrate what I mean, you went from this:

                new Wikifier(el, Story.get(ids[i]).processText().trim());

To essentially this:

                const frag = document.createDocumentFragment();
                new Wikifier(frag, Story.get(ids[i]).processText());

                if (Config.passages.descriptions == null) {
                    this._excerpt = Passage.getExcerptFromNode(frag);
                }

                el.append(frag);

That seems like a net loss to me, so I'm unsure how that's a good change for any project, let alone yours. The only possibly non-negative change there is the dropping of the trim, and there are probably better ways to get that if that's what you need—though I'd probably suggest checking why that's a necessity and fixing that.

[Arkerthan]

Big thanks for answering in such detail and going through our changes.

Browser handle local storage differently.

Really interesting, thanks.

The only way to play our game is with local play. While we support all modern browsers, we actually recommend Firefox because it is possible to increase local storage limits. 500kB+ (compressed!) saves are relatively common. At that point we decided it's more important to make auto saves happen at a decently fast speed over space savings. Firefox behaving differently with local storage didn't factor in that decision though, might make sense to reevaluate this.

There's no point in adding an .expired property at all if it's always going to be an empty array.

Thanks for the tip, wasn't around when it was done, just copied it over to a proper repository instead of a big patch file, so I didn't actually know what it did.

I assume that I'm looking at the current <Passage>.render() method from the fc branch. If not, then this may be off.

We extend the Passage class and overwrite render() so it simply executes a DOM generating function and returns that. This means we are not creating any Wikifier. So yes, only makes sense for our project.