Skip to content

feat: Add on_script_reloaded callback. #421

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

shanecelis
Copy link
Contributor

Summary

I added an on_script_reloaded function that is called before and after on_script_loaded on reloads. This addresses the issue I filed #419.

I updated the core-callbacks doc with example code. Here's the example code shown there:

mode = 1
function on_script_reloaded(save, value)
    if save then
        print("Before I go, take this.")
        return mode
    else
        print("I'm back. Where was I?")
        mode = value
    end
end

I'm not certain about the name. on_script_reload might be the name I'd choose with a blank slate.

Alternatives

One could designate a special variable to capture. But I think a function is preferable because a reload can interrupt at any time and keeping that value up to date all the time instead of when it's required seems inefficient.

One could break this into two functions, but they don't do anything unless both are implemented so it seemed wise to keep them as a single function.

Testing

I exercised it manually with errors in both the saving of state and restoring state, and both were reported as the usual mechanism.

Mistake

I started this branch on top of my other pull request #418. Oops. Luckily #418 is not controversial or large.

@shanecelis
Copy link
Contributor Author

A further alternative

I guess as an alternative going with the two functions, one could do this:

mode = 1
function on_script_reload()
    print("Before I go, take this.")
    return mode
end

function on_script_loaded(value)
    if value then
        print("I'm back. Where was I?")
        mode = value
    end
end

My criticism above doesn't apply because on_script_loaded() is useful without on_script_reload(). We'd then only have this set of function calls when reloading:

  1. on_script_loaded()
  2. INITIATE RELOAD
  3. value = on_script_reload()
  4. CONTEXT CLEARED
  5. on_script_loaded(value)

The preceding design looks like this for comparison:

  1. on_script_loaded()
  2. INITIATE RELOAD
  3. value = on_script_reload(true)
  4. CONTEXT CLEARED
  5. on_script_loaded()
  6. on_script_reload(false, value)

@shanecelis
Copy link
Contributor Author

NOTE: This currently only works when context sharing is enabled. I made an attempt to support non-shared contexts, but it seemed potentially infeasible. I'd love to hear @makspll's take on whether that seems right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant