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

Alter Global Scripts handling #3197

Merged
merged 2 commits into from Oct 31, 2023
Merged

Conversation

volundmush
Copy link
Contributor

Brief overview of PR changes/additions

Currently, Global Scripts are started at the end of evennia._init(), which is called by both the portal and the server. The GlobalScriptContainer is also having trouble distinguishing "scripts defined in its settings.py section" from "ALL Scripts which have no db_obj attached" and tries to start them all...

Not only that, but the scripts are starting before at_server_init() and all startup hooks are called, so they might fire off before the game has finished loading critical resources.

Even if it's running on the portal.

To combat this, I moved the GLOBAL_SCRIPTS.start() to the very end of the server startup process, after calling startup hooks.

Motivation for adding to Evennia

The above is definitely not something we want happening.

Other info (issues closed, discussion etc)

Copy link
Member

@Griatch Griatch left a comment

Choose a reason for hiding this comment

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

Overall, I think this looks fine. The unit tests don't pass though.

@Griatch
Copy link
Member

Griatch commented Jun 4, 2023

@volundmush Fixing the unit tests should be what remains for this PR.

@Griatch
Copy link
Member

Griatch commented Sep 3, 2023

@volundmush Ping. This and the other PRs you made often require rather small additions/unit test fixes.

@volundmush
Copy link
Contributor Author

It looks like blongden got to this first - sorry I lost my momentum there. This PR has been superseded.

@volundmush volundmush closed this Sep 8, 2023
@Griatch
Copy link
Member

Griatch commented Sep 8, 2023

@volundmush Not sure about that ... Blongden made a fix to make sure global scripts does not start up in the portal; I think this PR covers a different case.

@blongden
Copy link
Contributor

blongden commented Sep 9, 2023

@volundmush Not sure about that ... Blongden made a fix to make sure global scripts does not start up in the portal; I think this PR covers a different case.

Agreed. This incorporates a nicer fix to the same issue, moving the scripts start into the server rather than keeping it in init.

@Griatch
Copy link
Member

Griatch commented Sep 20, 2023

@volundmush Is this ready to re-review?

Copy link
Member

@Griatch Griatch left a comment

Choose a reason for hiding this comment

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

LGTM! And unit tests pass now too 👍

@Griatch Griatch merged commit 4986888 into evennia:main Oct 31, 2023
7 checks passed
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.

None yet

3 participants