Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add setting to reduce impact of expired moments #285

Open
tmedwards opened this issue Nov 29, 2023 · 2 comments
Open

Add setting to reduce impact of expired moments #285

tmedwards opened this issue Nov 29, 2023 · 2 comments
Assignees
Labels
enhancement New feature or request todo Make this happen

Comments

@tmedwards
Copy link
Owner

tmedwards commented Nov 29, 2023

Add a boolean Config.history setting for expired moments to either:

  • Drop them outright rather than recording them in an Array.
  • Change their storage to a Map and record the previous passage seperately.

NOTE: Both options would be irreversible—i.e., saves made with either enabled would be unable to be properly loaded with it disabled.

Rationale

Even with Config.history.maxStates set to a low value, games with a massive amount of turns can still begin to experience slowdown due to searching through the massive expired moments Array.

Option 1: Drop Expired Moments

Add a boolean Config.history setting to drop expired moments rather than recording them—e.g., Config.history.dropExpired.

Enabling this setting should:

  • Change how the previous passage is recorded so that the previous() function continues to work as intended.
  • Make dependent code throw an error so that end users never depend upon code that cannot work as intended with the setting enabled—e.g., the <<back>> & <<return>> macros, the visited family of functions, etc.

Option 2: Change Storage To Map

Add a boolean Config.history setting to record expired moments within a Map—e.g., Config.history.expireToMap.

Enabling this setting should:

  • Change how the previous passage is recorded so that the previous() function continues to work as intended.
  • Make dependent code throw an error so that end users never depend upon code that cannot work as intended with the setting enabled—e.g., the <<back>> & <<return>> macros, the lastVisited() function, etc.
@tmedwards tmedwards added enhancement New feature or request todo Make this happen labels Nov 29, 2023
@tmedwards tmedwards self-assigned this Nov 29, 2023
@tmedwards tmedwards changed the title Add a setting to drop expired moments Add setting to reduce impact of expired moments Dec 4, 2023
@david-donachie
Copy link

Presumably this would affect how hasVisited() (still works with a map, not with a drop), visited() (can only ever return 1, unless the map stores a counter?) and visitedTags() (presumably reliable with a map, and not a drop), work.

What would you think about something like a State.history.trim(maxVisits) method, which just cuts the history to have no more than X instances of a given passage? That might not really be a gain over using a map to record the number of visits per passage

@tmedwards
Copy link
Owner Author

To clarify, and assuming that the previous passage is recorded separately. Simply dropping expired moments will break the (entire) visited family of functions and the <<return>> macro. Switching to a Map([[name, visits]]) would only break the lastVisited() function as it's the only one that depends upon order.

The <<back>> macro and Jump To dialog are affected simply by expiring moments. Changing how expired moments are treated doesn't further affect them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request todo Make this happen
Projects
None yet
Development

No branches or pull requests

2 participants