refactor: add world-local static plugin config, remove ContextLoadingSettings resource
#470
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
makspll
changed the title
ContextLoadingSettings resourceContextLoadingSettings resource
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the configuration system by removing the ContextLoadingSettings resource and replacing it with a world-local static plugin configuration system. The change simplifies HandlerCtxt and improves performance by avoiding resource lookups for configuration that is only set at app startup.
- Introduces
ScriptingPluginConfiguration<P>with thread-local storage accessed viaP::readonly_config(world.id()) - Removes
ContextLoadingSettingsresource and related code throughout the codebase - Updates all configuration access patterns to use the new static configuration system
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/script_tests.rs | Forces single-threaded test execution to avoid thread-local storage issues |
| crates/testing_crates/test_utils/src/test_plugin.rs | Adds static plugin config macro usage |
| crates/languages/bevy_mod_scripting_rhai/src/lib.rs | Adds static plugin config macro and required imports |
| crates/languages/bevy_mod_scripting_rhai/Cargo.toml | Adds bevy_platform dependency |
| crates/languages/bevy_mod_scripting_lua/src/lib.rs | Adds static plugin config macro and required imports |
| crates/bevy_mod_scripting_core/src/script/script_context.rs | Adds imports for new config system |
| crates/bevy_mod_scripting_core/src/lib.rs | Updates trait bounds and plugin initialization logic |
| crates/bevy_mod_scripting_core/src/handler.rs | Removes context loading settings parameter and uses static config |
| crates/bevy_mod_scripting_core/src/extractors.rs | Removes ResScope and related context loading settings code |
| crates/bevy_mod_scripting_core/src/event.rs | Adds imports for new config system |
| crates/bevy_mod_scripting_core/src/context.rs | Removes ContextLoadingSettings and updates loaders to use static config |
| crates/bevy_mod_scripting_core/src/config.rs | New file implementing the static plugin configuration system |
| crates/bevy_mod_scripting_core/src/commands.rs | Updates script loading to use static config instead of handler context |
| crates/bevy_mod_scripting_core/src/bindings/world.rs | Adds world ID access methods |
| crates/bevy_mod_scripting_core/src/bindings/script_system.rs | Updates dynamic handler to use static config |
| crates/bevy_mod_scripting_core/src/bindings/schedule.rs | Adds imports for new config system |
| crates/bevy_mod_scripting_core/src/asset.rs | Updates asset handling to use static config instead of resource |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Bencher Report| Branch | refactor/world-local-plugin-config |
| Testbed | linux-gha |
⚠️ WARNING: No Threshold found!Without a Threshold, no Alerts will ever be generated.
Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsflag.
Click to view all benchmark results
| Benchmark | Latency | nanoseconds (ns) |
|---|---|---|
| component/access Lua | 📈 view plot | 3,523.30 ns |
| component/access Rhai | 📈 view plot | 5,563.10 ns |
| component/get Lua | 📈 view plot | 2,181.70 ns |
| component/get Rhai | 📈 view plot | 4,298.50 ns |
| conversions/Mut::from | 📈 view plot | 80.39 ns |
| conversions/Ref::from | 📈 view plot | 76.17 ns |
| conversions/ScriptValue::List | 📈 view plot | 295.62 ns |
| conversions/ScriptValue::Map | 📈 view plot | 886.95 ns |
| conversions/ScriptValue::Reference::from_into | 📈 view plot | 25.89 ns |
| conversions/Val::from_into | 📈 view plot | 275.31 ns |
| function/call 4 args Lua | 📈 view plot | 1,532.90 ns |
| function/call 4 args Rhai | 📈 view plot | 1,340.60 ns |
| function/call Lua | 📈 view plot | 225.07 ns |
| function/call Rhai | 📈 view plot | 454.49 ns |
| loading/empty Lua | 📈 view plot | 54,151.00 ns |
| loading/empty Rhai | 📈 view plot | 306,170.00 ns |
| math/vec mat ops Lua | 📈 view plot | 6,734.30 ns |
| math/vec mat ops Rhai | 📈 view plot | 5,859.20 ns |
| query/10 entities Lua | 📈 view plot | 17,948.00 ns |
| query/10 entities Rhai | 📈 view plot | 18,538.00 ns |
| query/100 entities Lua | 📈 view plot | 38,491.00 ns |
| query/100 entities Rhai | 📈 view plot | 30,754.00 ns |
| query/1000 entities Lua | 📈 view plot | 245,120.00 ns |
| query/1000 entities Rhai | 📈 view plot | 160,810.00 ns |
| reflection/10 Lua | 📈 view plot | 5,329.10 ns |
| reflection/10 Rhai | 📈 view plot | 15,223.00 ns |
| reflection/100 Lua | 📈 view plot | 46,407.00 ns |
| reflection/100 Rhai | 📈 view plot | 718,030.00 ns |
| resource/access Lua | 📈 view plot | 3,175.80 ns |
| resource/access Rhai | 📈 view plot | 5,079.60 ns |
| resource/get Lua | 📈 view plot | 1,834.60 ns |
| resource/get Rhai | 📈 view plot | 3,853.00 ns |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Removes
ContextLoadingSettingsand instead moves its innards into aScriptingPluginConfiguration<P>struct,which can be accessed via
P::readonly_config(world.id())This should:
HandlerCtxtbringing us closer to a world where we can put it in an ArcMigration Guide
Switch all references of
ContextLoadingSettingstoP::readonly_config(world.id()).