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

Run init-script only once after a sector has loaded #3011

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

Conversation

tobbi
Copy link
Member

@tobbi tobbi commented Jul 11, 2024

Fixes #698

@Narre
Copy link
Contributor

Narre commented Jul 11, 2024

This is a great option to have, especially for infinitely looping scripts that are only meant to trigger once, such as a loop that randomises the time between lightning strikes, however, I think there remains the need for scripts that are triggered every time on sector entry (such as my STAGE project, that needs to check for changes made in other sectors when a sector is entered). I have a few possible solutions in mind.

  • init script is triggered every time the sector is entered, much as it works now, but its execution is terminated when the sector is unloaded (so it gets triggered again upon re-entry but the old instance does not continue running)
  • there is an option in the sector settings that determines whether the init script should only trigger once upon the sector being loaded, or every time
  • there are two separate scripts, the init script that's only triggered on first initialisation, and an enter script that's triggered every time (in this case, I think it would be better for retrocompatibility if existing init scripts in old levels are interpreted as enter scripts, so they work how they did in the version they were written in)

@tobbi
Copy link
Member Author

tobbi commented Jul 11, 2024

Basically you want an option that you can check to specify whether it's run every time. I think I need to also reset the flag when the sector is reset.

@tobbi
Copy link
Member Author

tobbi commented Jul 12, 2024

Discussion question:
Should I keep the default of "run every time" for backward compatibility reasons, but make an option of "run only once".

Can I get some input on this?

@Narre @tylerandari13 @Vankata453

@Vankata453
Copy link
Member

Yeah, I believe a script to run only on sector initialization should be a separate option.

@Narre
Copy link
Contributor

Narre commented Jul 12, 2024

I agree, with both options available, the default should be the old behaviour to avoid needlessly breaking older levels.

@tobbi
Copy link
Member Author

tobbi commented Jul 15, 2024

This is ready for review. I haven't tested the actual behaviour but to quote "I, Robot": "My logic is undeniable".
https://www.youtube.com/watch?v=LH-G8c3TUac

@tylerandari13
Copy link
Contributor

Every time you load a sector it replaces all the instances with new instances. Is this also gone with this PR or is that untouched?

@tobbi
Copy link
Member Author

tobbi commented Jul 19, 2024

Every time you load a sector it replaces all the instances with new instances. Is this also gone with this PR or is that untouched?

What do you mean by "instances" in this case?

@tylerandari13
Copy link
Contributor

Every time you load a sector it replaces all the instances with new instances. Is this also gone with this PR or is that untouched?

What do you mean by "instances" in this case?

Basically the exposed objects. (theyre referred to as instances by Squirrel)

@@ -287,8 +289,10 @@ Sector::activate(const Vector& player_pos)
}

// Run init script
if (!m_init_script.empty() && !Editor::is_active()) {
if (!m_init_script.empty() && !Editor::is_active() &&
((m_init_script_run_once && !m_init_script_run) || !m_init_script_run_once)) {
Copy link
Member

@mrkubax10 mrkubax10 Jul 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose second line could be replaced with (!m_init_script_run_once || !m_init_script_run).

@Frostwithasideofsalt
Copy link
Member

What's the status on this PR?

@tobbi
Copy link
Member Author

tobbi commented Oct 30, 2024

Nothing was done since the last comments. It's another PR I need to tackle.

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.

InitScript bug when teleporting
6 participants