Skip to content

Commit 3db90fd

Browse files
makspllmakspll
committed
AHA
1 parent deddd15 commit 3db90fd

File tree

3 files changed

+76
-41
lines changed

3 files changed

+76
-41
lines changed

‎crates/bevy_mod_scripting_core/src/bindings/script_system.rs

Lines changed: 71 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ use bevy::{
2626
entity::Entity,
2727
query::{Access, FilteredAccess, FilteredAccessSet, QueryState},
2828
reflect::AppTypeRegistry,
29-
schedule::{IntoSystemConfigs, SystemSet},
29+
schedule::{IntoSystemConfigs, IntoSystemSetConfigs, SystemSet},
3030
system::{IntoSystem, System},
3131
world::{unsafe_world_cell::UnsafeWorldCell, World},
3232
},
@@ -49,7 +49,7 @@ impl std::fmt::Debug for ScriptSystemSet {
4949
}
5050

5151
impl ScriptSystemSet {
52-
/// Creates a new script system set with a unique id
52+
/// Creates a new script system set
5353
pub fn new(id: impl Into<Cow<'static, str>>) -> Self {
5454
Self(id.into())
5555
}
@@ -139,7 +139,7 @@ impl ScriptSystemBuilder {
139139
world: WorldGuard,
140140
schedule: &ReflectSchedule,
141141
) -> Result<ReflectSystem, InteropError> {
142-
let o = world.scope_schedule(schedule, |world, schedule| {
142+
world.scope_schedule(schedule, |world, schedule| {
143143
// this is different to a normal event handler
144144
// the system doesn't listen to events
145145
// it immediately calls a singular script with a predefined payload
@@ -156,8 +156,7 @@ impl ScriptSystemBuilder {
156156
// this is quite important, by default systems are placed in a set defined by their TYPE, i.e. in this case
157157
// all script systems would be the same
158158
// let set = ScriptSystemSet::next();
159-
let mut config = IntoSystemConfigs::<IsDynamicScriptSystem<P>>::into_configs(self);
160-
159+
let mut set_config = IntoSystemSetConfigs::into_configs(set);
161160
// apply ordering
162161
for (other, is_before) in before_systems
163162
.into_iter()
@@ -167,50 +166,28 @@ impl ScriptSystemBuilder {
167166
for default_set in other.default_system_sets() {
168167
if is_before {
169168
bevy::log::info!("before {default_set:?}");
170-
config = config.before(*default_set);
169+
system_config = system_config.before(*default_set);
171170
} else { bevy::log::info!("before {default_set:?}");
172171
bevy::log::info!("after {default_set:?}");
173-
config = config.after(*default_set);
172+
system_config = system_config.after(*default_set);
174173
}
175174
}
176175
}
177176

178-
schedule.add_systems(config);
177+
schedule.add_systems(system_config);
178+
// TODO: the node id seems to always be system.len()
179+
// if this is slow, we can always just get the node id that way
180+
// and let the schedule initialize itself right before it gets run
181+
// for now I want to avoid not having the right ID as that'd be a pain
179182
schedule.initialize(world)?;
180-
181-
let dot = bevy_system_reflection::schedule_to_dot_graph(schedule);
182-
183-
std::fs::write(format!("/home/makspll/git/bevy_mod_scripting/{system_name}_post_update.dot"), dot).expect("Unable to write file");
184-
185183
// now find the system
186184
let (node_id, system) = schedule
187185
.systems()?
188186
.find(|(_, b)| b.name().deref() == system_name)
189187
.ok_or_else(|| InteropError::invariant("After adding the system, it was not found in the schedule, could not return a reference to it"))?;
190188

191-
192-
193189
Ok(ReflectSystem::from_system(system.as_ref(), node_id))
194-
195-
})?;
196-
197-
// #[allow(clippy::expect_used)]
198-
// world.with_global_access(|world| {
199-
200-
// let mut app = App::new();
201-
// std::mem::swap(app.world_mut(), world);
202-
203-
// let dot = bevy_mod_debugdump::schedule_graph_dot(&mut app, PostUpdate, &Settings{
204-
// ..Default::default()
205-
// });
206-
// // save to ./graph.dot
207-
// std::fs::write("/home/makspll/git/bevy_mod_scripting/post_update.dot", dot).expect("Unable to write file");
208-
209-
// // swap worlds back
210-
// std::mem::swap(app.world_mut(), world);
211-
// }).expect("");
212-
213-
o
190+
})?
214191
}
215192
}
216193

@@ -407,7 +384,7 @@ impl<P: IntoScriptPluginParams> System for DynamicScriptSystem<P> {
407384
}
408385

409386
fn is_send(&self) -> bool {
410-
true
387+
!self.is_exclusive()
411388
}
412389

413390
fn is_exclusive(&self) -> bool {
@@ -640,3 +617,61 @@ impl<P: IntoScriptPluginParams> System for DynamicScriptSystem<P> {
640617
vec![ScriptSystemSet::new(self.name.clone()).intern()]
641618
}
642619
}
620+
621+
622+
#[cfg(test)]
623+
mod test {
624+
use bevy::{app::{App, MainScheduleOrder, Update}, asset::AssetPlugin, diagnostic::DiagnosticsPlugin, ecs::schedule::{ScheduleLabel, Schedules}};
625+
use test_utils::make_test_plugin;
626+
627+
use super::*;
628+
629+
630+
make_test_plugin!(crate);
631+
632+
fn test_system_rust(world: &mut World) {}
633+
634+
#[test]
635+
fn test_script_system_with_existing_system_dependency_can_execute() {
636+
let mut app = App::new();
637+
638+
#[derive(ScheduleLabel, Clone, Debug, Hash, PartialEq, Eq)]
639+
struct TestSchedule;
640+
641+
app.add_plugins((AssetPlugin::default(), DiagnosticsPlugin,TestPlugin::default()));
642+
app.init_schedule(TestSchedule);
643+
let mut main_schedule_order = app.world_mut().resource_mut::<MainScheduleOrder>();
644+
main_schedule_order.insert_after(Update, TestSchedule);
645+
app.add_systems(TestSchedule, test_system_rust);
646+
647+
648+
// run the app once
649+
app.finish();
650+
app.cleanup();
651+
app.update();
652+
653+
// find existing rust system
654+
let test_system = app.world_mut().resource_scope::<Schedules,_>(|_, schedules| {
655+
let (node_id, system) = schedules.get(TestSchedule)
656+
.unwrap()
657+
.systems()
658+
.unwrap()
659+
.find(|(_, system)| {
660+
system.name().contains("test_system_rust")
661+
})
662+
.unwrap();
663+
664+
ReflectSystem::from_system(system.as_ref(), node_id)
665+
});
666+
667+
668+
// now dynamically add script system via builder
669+
let mut builder = ScriptSystemBuilder::new("test".into(), "empty_script".into());
670+
builder.before_system(test_system);
671+
672+
let _ = builder.build::<TestPlugin>(WorldAccessGuard::new_exclusive(app.world_mut()), &ReflectSchedule::from_label(TestSchedule)).unwrap();
673+
674+
// now re-run app, expect no panicks
675+
app.update();
676+
}
677+
}

‎crates/bevy_system_reflection/src/lib.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -196,20 +196,20 @@ pub fn schedule_to_dot_graph(schedule: &Schedule) -> String {
196196
for edge in graph.hierarchy {
197197
let from = node_id_map.get(&edge.from).cloned().unwrap_or_else(|| {
198198
let mut unknown = writer.node_auto();
199-
unknown.set_label(&format!("unknown_child {:?}", edge.from.0));
199+
unknown.set_label(&format!("unknown_parent {:?}", edge.from.0));
200200
let id = unknown.id();
201201
node_id_map.insert(edge.from, id.clone());
202202
id
203203
});
204204
let to = node_id_map.get(&edge.to).cloned().unwrap_or_else(|| {
205205
let mut unknown = writer.node_auto();
206-
unknown.set_label(&format!("unknown_parent {:?}", edge.to.0));
206+
unknown.set_label(&format!("unknown_child {:?}", edge.to.0));
207207
let id = unknown.id();
208208
node_id_map.insert(edge.to, id.clone());
209209
id
210210
});
211211
writer
212-
.edge(from, to)
212+
.edge(to, from)
213213
.attributes()
214214
.set_color(dot_writer::Color::Red)
215215
.set_label("child of")
@@ -235,7 +235,7 @@ pub fn schedule_to_dot_graph(schedule: &Schedule) -> String {
235235
.edge(from, to)
236236
.attributes()
237237
.set_color(dot_writer::Color::Blue)
238-
.set_label("depends on")
238+
.set_label("runs before")
239239
.set_arrow_head(dot_writer::ArrowType::Normal);
240240
}
241241
}

‎crates/languages/bevy_mod_scripting_lua/tests/lua_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ fn discover_all_tests() -> Vec<Test> {
106106
let mut test_files = Vec::new();
107107
visit_dirs(&test_root, &mut |entry| {
108108
let path = entry.path();
109-
if path.extension().unwrap() == "lua" {
109+
if path.extension().unwrap() == "lua" && path.to_string_lossy().contains("adds_system") {
110110
// only take the path from the assets bit
111111
let relative = path.strip_prefix(&assets_root).unwrap();
112112
test_files.push(Test {

0 commit comments

Comments
 (0)