Add option to limit expired moments

[tmedwards]

Add a boolean configuration setting to limit the number of expired moments kept. Boolean to keep users from setting stupid values.

This will keep very long running games from slowing down and reduce save bloat at the cost of breaking the visited family of functions.

Tasks

• [ ] Add the Config.history.limitExpired boolean property (default: false). When enabled, expired moments will be limited to some TBD amount—probably the default max states or twice that; i.e., 40 or 80.
• [ ] Make the visited family of functions throw with an appropriate error message when the above setting is enabled.
• [ ] Loudly warn, in its documentation, that enabling the setting will break the visited family of functions and that it should not be changed mid-project, since it drops history.

Maybe

• [ ] Make State.expired immutable.
• [ ] Add warnings about the setting to the documentation of the visited family of functions?

[AlyxMS]

If Config.history.limitExpired can only be boolean and State.expired is immutable, then it's impossible to set a precise amount the user want.

My preference is have State.expired be immutable(or even all of the undocumented stuff immutable) but let history.limitExpired be a number instead of boolean. Safety can be implemented to check against invalid values(like negative or float) and the user can be warned against stupidly low values in the docs.

[tmedwards]

What purpose would that serve?

The expired history serves exactly one purpose, to allow the various history APIs to continue to function while the maximum in-play history is limited.

Once you start pruning the expired history, you accept that, at least some of, the history APIs become unreliable/broken. In this case, that would be the visited family of functions.

The rest of the history APIs—e.g., previous(), <<return>>, etc—can cope with some loss of the expired history. Their requirements are a sizable enough buffer to look back a, relatively, short distance.

There's not much of a need for user tuning here, just a buffer to allow some of the APIs to continue to work.

It's a Goldilocks thing. Too small a buffer and the remaining APIs could become unreliable too. Too large and you're wasting resources, which is what the setting is supposed to address.

[AlyxMS]

Don't know. Maybe the user want to track some statistics based on the passage names of the last 1000 visits. Maybe someone's passage need the player to visit the same passage 100 times in a row and the only way out depends on previous().

I know these can be implemented/achieved in a better way by the user. Point is this is an arbitrary limit(40/80 or infinite) when it could've easily been a tunable number. The explaination about have it be just small or have it be infinite can be in the docs and guide the users there. Maybe a "don't touch unless you are experiencing this issue and have pinpointed the cause to this" too.

I sincerely doubt there will be a flood of report/complaints about game being broken because of incorrect use of a feature buried in the Config API section of the docs. (I don't doubt there will be a few but I don't think having it boolean instead of number will have much of an impact on the amount of reports.)

I'm just a weird "more settings more better" kind of person and dislike arbitrary limits. If you do decide to implement it like you planned I think it's a reasonable decision. I simply think "to keep users from setting stupid values" isn't a good reason to make it boolean. If the amount of problems having a documented feature that breaks core functions is the main concern, perhaps it shouldn't be implemented at all. Having it a number instead of boolean will not reduce the amount of reports much, but not having this feature will eliminate it.

This feature is going to realistically help maybe 0.1% of users. But much more than 0.1% of them will have this feature on because it's there(and potentially have problems). Perhaps just let the 0.1% mess with State.expired for now and fix it completely in SC3.

[ChapelR]

The more I think about this the more I think this idea is unneeded. The edge cases this feature would help are so rare, and I think for most users, deactivating session storage as a means to squeeze more performance out makes more sense than allowing users to get rid of the expired history.

On a philosophical level, maybe users should have this setting, but on a practical level, no one who is writing sane, functional code should really need it.

[tmedwards]

Here's the problem.

In a moment of weakness, (on Discord) I provided a solution to a user for an issue they were having. I say in a moment of weakness because I exposed an engine implementation detail that I'm normally loath to do—I blame lack of sleep.

Stupid, yes, but no big deal and the whole interaction should have gotten buried in the channel as normal. That should have ended it, done, finito.

It didn't end there though. Several hours later it was brought back up. That got a few people's attentions and it snowballed from there. Code was refined, explanations were made.

I'm not blaming anyone but myself, to be clear. The point is that the cat is out of the bag. I know damn well that code like the following is going to start making the cargo cult rounds:

$(document).on(":passagedisplay", () => {
    State.expired.splice(0, Math.max(State.expired.length - 100, 0));
});

And it doesn't matter how often I tell people not to fucking do it:

Also. Please do not start doing this as a general thing. It's a dirty hack that I gave to someone who was experiencing issues with their published game.

If you're not having flaming pitchforks thrown at you by your players, do not preemptively break your game's history.

They're going to anyway. You know they are.

I don't want to add this setting, or anything like it, but I also don't want to see State.expired splicing hacks. Especially when people are never going to tell you they're using the damn hack in the first place.

I just want to turn back time at this point.

[tmedwards]

I think I'm just going to delete the #sugarcube channel and forget this shitshow ever happened. 😏

[AlyxMS]

I've deleted my discord messages containing that code and explanation, hope it helps 😟

[tmedwards]

You didn't need to do that. As I said, I'm not blaming anyone but myself for this fiasco.

I do appreciate the effort though.