Skip to content

Autosave scheduling checks a world variable instead of the action queue, so ::reload settings pushes the deadline out #1251

Description

@HarleyGilpin

AutoSave guards its scheduling on World.contains("auto_save"):

settingsReload {
    val minutes = Settings["storage.autoSave.minutes", 0]
    if (World.contains("auto_save") && minutes <= 0) {
        World.clearQueue("auto_save")
    } else if (!World.contains("auto_save") && minutes > 0) {
        autoSave()
    }
}

World is declared object World : Entity, VariableStore, Runnable, KoinComponent, so contains resolves to VariableStore.contains(key) = variables.contains(key). That reads a world variable named auto_save. Nothing anywhere sets one, so it is always false. The queue check is World.containsQueue (World.kt:48), which reads the actions map that World.queue actually writes to.

Two consequences.

With minutes > 0 the second branch fires on every reload, because !World.contains(...) is always true. autoSave() then calls World.queue("auto_save", TimeUnit.MINUTES.toTicks(minutes)), and actions is keyed by name:

fun queue(name: String, initialDelay: Int = 0, block: () -> Unit) {
    actions[name] = (GameLoop.tick + initialDelay) to block
}

So the existing entry is replaced with a fresh deadline rather than duplicated. Every ::reload settings restarts the countdown. Reload more often than storage.autoSave.minutes and autosave never runs at all. On a dev server where you're reloading settings while testing, that's easy to hit without noticing, and the first sign is a rolled back save.

With minutes <= 0 the first branch can't fire either, so World.clearQueue("auto_save") is unreachable. Turning autosave off at runtime doesn't take effect immediately: the already queued action still fires once more and saves everyone, and only then does the chain stop, because the autoSave() call at the end of the block returns early on minutes <= 0.

Swapping both calls to containsQueue fixes the first and makes the second reachable. Self-rescheduling from inside the queued block still works, since World.run() removes the entry before invoking it:

iterator.remove()
try {
    block.invoke()
}

so containsQueue("auto_save") is false at the point autoSave() re-queues.

A test would need World.containsQueue rather than the private actions map: with minutes > 0 assert the queue is populated, set storage.autoSave.minutes to 0, call SettingsReload.now(), and assert it's been cleared. That assertion fails today because the clearQueue branch never runs.

Found while working on #1249, which touches AutoSave's worldDespawn block but leaves the scheduling alone.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions