diff --git a/Cargo.toml b/Cargo.toml index ae14124a4..2b87107cf 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -93,6 +93,7 @@ members = [ "crates/ladfile", "crates/lad_backends/mdbook_lad_preprocessor", "crates/ladfile_builder", + "crates/bevy_system_reflection", ] resolver = "2" exclude = ["crates/bevy_api_gen", "crates/macro_tests"] diff --git a/assets/lua_to_rhai_test_conversion_prompt.md b/assets/lua_to_rhai_test_conversion_prompt.md new file mode 100644 index 000000000..176e92456 --- /dev/null +++ b/assets/lua_to_rhai_test_conversion_prompt.md @@ -0,0 +1,240 @@ +// perfect application of AI +// helps proompting LLMs to do good for this project + +# Convert lua to rhai script +Convert the current test to a rhai test. below are examples and instructions on the conversions necessary: + +## Dynamic function calls +Functions which are not native to rhai MUST be explicitly called using the below syntax: + +```rhai +type.function.call(arguments...) +``` + +native rhai functions MUST NOT be converted using the syntax above. + +Below is a list of some of the native functions their argument names and some constants available in rhai: + +```csv +function,arguments +is_odd, +is_even, +min,"a, b" +max,"a, b" +to_float, +to_decimal, +abs,value +sign,value +is_zero, +sin,angle +cos,angle +tan,angle +sinh,angle +cosh,angle +tanh,angle +hypot,"x, y" +asin,value +acos,value +atan,value +atan,"x, y" +asinh,value +acosh,value +atanh,value +sqrt,value +exp,value +ln,value +log,value +floor, +ceiling, +round, +round,decimal_points +int, +fraction, +round_up,decimal_points +round_down,decimal_points +round_half_up,decimal_points +round_half_down,decimal_points +to_int, +to_decimal, +to_float, +to_degrees, +to_radians, +is_nan, +is_finite, +is_infinite, +parse_int,"string, [radix]" +parse_float,string +parse_decimal,string +to_binary,value +to_octal,value +to_hex,value +PI, +E, +``` + +## Operators +Operators are different in lua and rhai, below is a list of operators supported: + +``` +Operators Assignment operators Supported types +(see standard types) ++, += + + INT + FLOAT (if not no_float) + Decimal (requires decimal) + char + string + +-, *, /, %, **, -=, *=, /=, %=, **= + + INT + FLOAT (if not no_float) + Decimal (requires decimal) + +<<, >> <<=, >>= + + INT + +&, |, ^ &=, |=, ^= + + INT (bit-wise) + bool (non-short-circuiting) + +&&, || + + bool (short-circuits) + +==, != + + INT + FLOAT (if not no_float) + Decimal (requires decimal) + bool + char + string + BLOB + numeric range + () + +>, >=, <, <= + + INT + FLOAT (if not no_float) + Decimal (requires decimal) + char + string + () +``` + +## Function syntax +Functions in rhai look like this: + +```rhai +fn function_name(arg1, arg2) { + return value; +} +``` + +## Semicolons +Every statement must end in a semicolon + + +Below is a new section on Rhai strings that you can add to the prompt: + +## Rhai Strings + +Rhai supports different string types such as raw strings (enclosed by matching `#` and double-quotes), multi-line literal strings (enclosed by backticks to preserve exact formatting), and strings with interpolation (using `${…}` inside multi-line literals). These variants allow you to easily include complex content like newlines, quotes, and even embedded expressions while keeping the original formatting. Here are three examples: + +````rhai +// Raw string example: +let raw_str = #"Hello, raw string! \n No escape sequences here."#; +```` + +````rhai +// Multi-line literal string example: +let multi_line = ` +This is a multi-line literal string, +which preserves whitespaces, newlines, and "quotes" exactly. +`; +```` + +````rhai +// String interpolation example: +let value = 42; +let interpolated = `The answer is ${value}, which is computed dynamically.`; +```` + +## Null Checks +null checks can be performed by checking `type_of(value) == "()"` + +## Examples +Below is an example lua test and its equivalent rhai script: + +### Lua +```lua +local entity_a = world.spawn() +local entity_b = world.spawn() +local entity_c = world.spawn() +local entity_d = world._get_entity_with_test_component("CompWithFromWorldAndComponentData") + +local component_with = world.get_type_by_name("CompWithFromWorldAndComponentData") +local component_without = world.get_type_by_name("CompWithDefaultAndComponentData") + +world.add_default_component(entity_a, component_with) +world.add_default_component(entity_b, component_with) +world.add_default_component(entity_c, component_with) + +world.add_default_component(entity_b, component_without) + +local found_entities = {} +for i,result in pairs(world.query():component(component_with):without(component_without):build()) do + table.insert(found_entities, result:entity()) +end + +assert(#found_entities == 3, "Expected 3 entities, got " .. #found_entities) + +expected_entities = { + entity_c, + entity_d, + entity_a, +} + +for i, entity in ipairs(found_entities) do + assert(entity:index() == expected_entities[i]:index(), "Expected entity " .. expected_entities[i]:index() .. " but got " .. entity:index()) +end +``` + +### Rhai +```rhai +let entity_a = world.spawn_.call(); +let entity_b = world.spawn_.call(); +let entity_c = world.spawn_.call(); +let entity_d = world._get_entity_with_test_component.call("CompWithFromWorldAndComponentData"); + +let component_with = world.get_type_by_name.call("CompWithFromWorldAndComponentData"); +let component_without = world.get_type_by_name.call("CompWithDefaultAndComponentData"); + +world.add_default_component.call(entity_a, component_with); +world.add_default_component.call(entity_b, component_with); +world.add_default_component.call(entity_c, component_with); + +world.add_default_component.call(entity_b, component_without); + +let found_entities = []; +for (result, i) in world.query.call().component.call(component_with).without.call(component_without).build.call() { + found_entities.push(result.entity.call()); +} + +assert(found_entities.len == 3, "Expected 3 entities, got " + found_entities.len); + +let expected_entities = [ + entity_d, + entity_a, + entity_c, +]; + +for (entity, i) in found_entities { + assert(entity.index.call() == expected_entities[i].index.call(), "Expected entity " + expected_entities[i].index.call() + " but got " + entity.index.call()); +} +``` \ No newline at end of file diff --git a/assets/tests/add_system/added_systems_run_in_parallel.lua b/assets/tests/add_system/added_systems_run_in_parallel.lua new file mode 100644 index 000000000..2adf8104d --- /dev/null +++ b/assets/tests/add_system/added_systems_run_in_parallel.lua @@ -0,0 +1,43 @@ + +function on_test() + local post_update_schedule = world.get_schedule_by_name("PostUpdate") + + local test_system = post_update_schedule:get_system_by_name("on_test_post_update") + + local system_a = world.add_system( + post_update_schedule, + system_builder("custom_system_a", script_id) + :after(test_system) + ) + + local system_b = world.add_system( + post_update_schedule, + system_builder("custom_system_b", script_id) + :after(test_system) + ) + + -- generate a schedule graph and verify it's what we expect + local dot_graph = post_update_schedule:render_dot() + + local expected_dot_graph = [[ +digraph { + node_0 [label="bevy_mod_scripting_core::bindings::allocator::garbage_collector"]; + node_1 [label="on_test_post_update"]; + node_2 [label="script_integration_test_harness::dummy_before_post_update_system"]; + node_3 [label="script_integration_test_harness::dummy_post_update_system"]; + node_4 [label="custom_system_a"]; + node_5 [label="custom_system_b"]; + node_6 [label="SystemSet GarbageCollection"]; + node_7 [label="SystemSet ScriptSystem(custom_system_a)"]; + node_8 [label="SystemSet ScriptSystem(custom_system_b)"]; + node_0 -> node_6 [color=red, label="child of", arrowhead=diamond]; + node_4 -> node_7 [color=red, label="child of", arrowhead=diamond]; + node_5 -> node_8 [color=red, label="child of", arrowhead=diamond]; + node_1 -> node_4 [color=blue, label="runs before", arrowhead=normal]; + node_1 -> node_5 [color=blue, label="runs before", arrowhead=normal]; + node_2 -> node_3 [color=blue, label="runs before", arrowhead=normal]; +} + ]] + + assert_str_eq(dot_graph, expected_dot_graph, "Expected the schedule graph to match the expected graph") +end diff --git a/assets/tests/add_system/added_systems_run_in_parallel.rhai b/assets/tests/add_system/added_systems_run_in_parallel.rhai new file mode 100644 index 000000000..4ab2ddc17 --- /dev/null +++ b/assets/tests/add_system/added_systems_run_in_parallel.rhai @@ -0,0 +1,41 @@ +fn on_test() { + let post_update_schedule = world.get_schedule_by_name.call("PostUpdate"); + + let test_system = post_update_schedule.get_system_by_name.call("on_test_post_update"); + + let system_a = world.add_system.call( + post_update_schedule, + system_builder.call("custom_system_a", script_id) + .after.call(test_system) + ); + + let system_b = world.add_system.call( + post_update_schedule, + system_builder.call("custom_system_b", script_id) + .after.call(test_system) + ); + + // generate a schedule graph and verify it's what we expect + let dot_graph = post_update_schedule.render_dot.call(); + + let expected_dot_graph = ` +digraph { + node_0 [label="bevy_mod_scripting_core::bindings::allocator::garbage_collector"]; + node_1 [label="on_test_post_update"]; + node_2 [label="script_integration_test_harness::dummy_before_post_update_system"]; + node_3 [label="script_integration_test_harness::dummy_post_update_system"]; + node_4 [label="custom_system_a"]; + node_5 [label="custom_system_b"]; + node_6 [label="SystemSet GarbageCollection"]; + node_7 [label="SystemSet ScriptSystem(custom_system_a)"]; + node_8 [label="SystemSet ScriptSystem(custom_system_b)"]; + node_0 -> node_6 [color=red, label="child of", arrowhead=diamond]; + node_4 -> node_7 [color=red, label="child of", arrowhead=diamond]; + node_5 -> node_8 [color=red, label="child of", arrowhead=diamond]; + node_1 -> node_4 [color=blue, label="runs before", arrowhead=normal]; + node_1 -> node_5 [color=blue, label="runs before", arrowhead=normal]; + node_2 -> node_3 [color=blue, label="runs before", arrowhead=normal]; +}`; + + assert_str_eq.call(dot_graph, expected_dot_graph, "Expected the schedule graph to match the expected graph"); +} \ No newline at end of file diff --git a/assets/tests/add_system/adds_system_in_correct_order.lua b/assets/tests/add_system/adds_system_in_correct_order.lua index 36e0e1e88..44a844d77 100644 --- a/assets/tests/add_system/adds_system_in_correct_order.lua +++ b/assets/tests/add_system/adds_system_in_correct_order.lua @@ -20,6 +20,13 @@ function on_test() system_builder("custom_system_before", script_id) :before(test_system) ) + + local script_system_between = world.add_system( + post_update_schedule, + system_builder("custom_system_between", script_id) + :after(test_system) + :before(system_after) + ) end @@ -39,10 +46,16 @@ function custom_system_after() runs[#runs + 1] = "custom_system_after" end +function custom_system_between() + print("custom_system_between") + runs[#runs + 1] = "custom_system_between" +end + -- runs in the `Last` bevy schedule function on_test_last() - assert(#runs == 3, "Expected 3 runs, got: " .. #runs) + assert(#runs == 4, "Expected 4 runs, got: " .. #runs) assert(runs[1] == "custom_system_before", "Expected custom_system_before to run first, got: " .. runs[1]) assert(runs[2] == "on_test_post_update", "Expected on_test_post_update to run second, got: " .. runs[2]) - assert(runs[3] == "custom_system_after", "Expected custom_system_after to run third, got: " .. runs[3]) + assert(runs[3] == "custom_system_between", "Expected custom_system_between to run third, got: " .. runs[3]) + assert(runs[4] == "custom_system_after", "Expected custom_system_after to run second, got: " .. runs[4]) end \ No newline at end of file diff --git a/assets/tests/add_system/adds_system_in_correct_order.rhai b/assets/tests/add_system/adds_system_in_correct_order.rhai index e989b50d7..6bfc1a6e4 100644 --- a/assets/tests/add_system/adds_system_in_correct_order.rhai +++ b/assets/tests/add_system/adds_system_in_correct_order.rhai @@ -4,11 +4,19 @@ fn on_test() { let post_update_schedule = world.get_schedule_by_name.call("PostUpdate"); let test_system = post_update_schedule.get_system_by_name.call("on_test_post_update"); - let builder_after = system_builder.call("custom_system_after", script_id).after.call(test_system); + let builder_after = system_builder.call("custom_system_after", script_id) + .after.call(test_system); let system_after = world.add_system.call(post_update_schedule, builder_after); - let builder_before = system_builder.call("custom_system_before", script_id).before.call(test_system); + let builder_before = system_builder.call("custom_system_before", script_id) + .before.call(test_system); let system_before = world.add_system.call(post_update_schedule, builder_before); + + let builder_between = system_builder.call("custom_system_between", script_id) + .after.call(test_system) + .before.call(system_after); + + let system_between = world.add_system.call(post_update_schedule, builder_between); } fn custom_system_before() { @@ -26,9 +34,15 @@ fn custom_system_after() { runs.push("custom_system_after"); } +fn custom_system_between() { + print("custom_system_between"); + runs.push("custom_system_between"); +} + fn on_test_last() { - assert(runs.len() == 3, "Expected 3 runs, got: " + runs.len().to_string()); + assert(runs.len() == 4, "Expected 4 runs, got: " + runs.len().to_string()); assert(runs[0] == "custom_system_before", "Expected custom_system_before to run first, got: " + runs[0]); assert(runs[1] == "on_test_post_update", "Expected on_test_post_update to run second, got: " + runs[1]); - assert(runs[2] == "custom_system_after", "Expected custom_system_after to run third, got: " + runs[2]); + assert(runs[2] == "custom_system_between", "Expected custom_system_between to run third, got: " + runs[2]); + assert(runs[3] == "custom_system_after", "Expected custom_system_after to run third, got: " + runs[3]); } \ No newline at end of file diff --git a/assets/tests/add_system/system_can_access_unspecified_resource_in_exclusive_system__RETURNS.lua b/assets/tests/add_system/system_can_access_unspecified_resource_in_exclusive_system__RETURNS.lua new file mode 100644 index 000000000..544513648 --- /dev/null +++ b/assets/tests/add_system/system_can_access_unspecified_resource_in_exclusive_system__RETURNS.lua @@ -0,0 +1,32 @@ + +runs = {} + +function on_test() + local post_update_schedule = world.get_schedule_by_name("PostUpdate") + + world.add_system( + post_update_schedule, + system_builder("my_exclusive_system", script_id):exclusive() + ) + + return true +end + +function my_exclusive_system() + print("my_exclusive_system") + runs[#runs + 1] = "my_non_exclusive_system" + + local ResourceType = world.get_type_by_name("TestResource") + local res = world.get_resource(ResourceType) + assert(res ~= nil, "Expected to get resource but got nil") +end + + +function on_test_post_update() + return true +end + +function on_test_last() + assert(#runs == 1, "Expected 1 runs, got: " .. #runs) + return true +end \ No newline at end of file diff --git a/assets/tests/add_system/system_can_access_unspecified_resource_in_exclusive_system__RETURNS.rhai b/assets/tests/add_system/system_can_access_unspecified_resource_in_exclusive_system__RETURNS.rhai new file mode 100644 index 000000000..88413e440 --- /dev/null +++ b/assets/tests/add_system/system_can_access_unspecified_resource_in_exclusive_system__RETURNS.rhai @@ -0,0 +1,30 @@ +let runs = []; + +fn on_test() { + let post_update_schedule = world.get_schedule_by_name.call("PostUpdate"); + + world.add_system.call( + post_update_schedule, + system_builder.call("my_exclusive_system", script_id).exclusive.call() + ); + + return true; +}; + +fn my_exclusive_system() { + print("my_exclusive_system"); + runs.push("my_non_exclusive_system"); + + let ResourceType = world.get_type_by_name.call("TestResource"); + let res = world.get_resource.call(ResourceType); + assert(type_of(res) != "()", "Expected to get resource but got nil"); +}; + +fn on_test_post_update() { + return true; +}; + +fn on_test_last() { + assert(runs.len == 1, "Expected 1 runs, got: " + runs.len); + return true; +}; \ No newline at end of file diff --git a/assets/tests/add_system/system_cannot_access_unspecified_resource_in_non_exclusive_system__RETURNS.lua b/assets/tests/add_system/system_cannot_access_unspecified_resource_in_non_exclusive_system__RETURNS.lua new file mode 100644 index 000000000..0d94f730a --- /dev/null +++ b/assets/tests/add_system/system_cannot_access_unspecified_resource_in_non_exclusive_system__RETURNS.lua @@ -0,0 +1,34 @@ + +runs = {} + +function on_test() + local post_update_schedule = world.get_schedule_by_name("PostUpdate") + + world.add_system( + post_update_schedule, + system_builder("my_non_exclusive_system", script_id) + ) + + return true +end + +function my_non_exclusive_system() + print("my_non_exclusive_system") + runs[#runs + 1] = "my_non_exclusive_system" + + local ResourceType = world.get_type_by_name("TestResource") + assert_throws(function() + local res = world.get_resource(ResourceType) + local blah = res.blahblah + end, ".*annot claim access to.*") +end + + +function on_test_post_update() + return true +end + +function on_test_last() + assert(#runs == 1, "Expected 1 runs, got: " .. #runs) + return true +end \ No newline at end of file diff --git a/assets/tests/add_system/system_cannot_access_unspecified_resource_in_non_exclusive_system__RETURNS.rhai b/assets/tests/add_system/system_cannot_access_unspecified_resource_in_non_exclusive_system__RETURNS.rhai new file mode 100644 index 000000000..f82bb1378 --- /dev/null +++ b/assets/tests/add_system/system_cannot_access_unspecified_resource_in_non_exclusive_system__RETURNS.rhai @@ -0,0 +1,27 @@ +let runs = []; + +fn on_test() { + let post_update_schedule = world.get_schedule_by_name.call("PostUpdate"); + world.add_system.call(post_update_schedule, system_builder.call("my_non_exclusive_system", script_id)); + return true; +} + +fn my_non_exclusive_system() { + print("my_non_exclusive_system"); + runs.push("my_non_exclusive_system"); + + let ResourceType = world.get_type_by_name.call("TestResource"); + assert_throws(|| { + let res = world.get_resource.call(ResourceType); + let blah = res.blahblah + }, ".*annot claim access to.*"); +} + +fn on_test_post_update() { + return true; +} + +fn on_test_last() { + assert(runs.len == 1, "Expected 1 runs, got: " + runs.len); + return true; +} \ No newline at end of file diff --git a/assets/tests/add_system/system_with_parameters__RETURNS.lua b/assets/tests/add_system/system_with_parameters__RETURNS.lua new file mode 100644 index 000000000..700658214 --- /dev/null +++ b/assets/tests/add_system/system_with_parameters__RETURNS.lua @@ -0,0 +1,60 @@ + +runs = {} +local ResourceTypeA = world.get_type_by_name("TestResource") +local ResourceTypeB = world.get_type_by_name("TestResourceWithVariousFields") +local ComponentA = world.get_type_by_name("CompWithFromWorldAndComponentData") +local ComponentB = world.get_type_by_name("CompWithDefaultAndComponentData") + +function on_test() + local post_update_schedule = world.get_schedule_by_name("PostUpdate") + + local entity = world.spawn() + local entity2 = world.spawn() + + + world.add_default_component(entity, ComponentA) + world.add_default_component(entity, ComponentB) + world.add_default_component(entity2, ComponentA) + world.add_default_component(entity2, ComponentB) + + world.add_system( + post_update_schedule, + system_builder("my_parameterised_system", script_id) + :resource(ResourceTypeA) + :query(world.query():component(ComponentA):component(ComponentB)) + :resource(ResourceTypeB) + ) + + return true +end + +function my_parameterised_system(resourceA,query,resourceB) + print("my_parameterised_system") + runs[#runs + 1] = "my_non_exclusive_system" + + assert(resourceA ~= nil, "Expected to get resource but got nil") + assert(query ~= nil, "Expected to get query but got nil") + assert(resourceB ~= nil, "Expected to get resource but got nil") + + assert(#resourceA.bytes == 6, "Expected 6 bytes, got: " .. #resourceA.bytes) + assert(resourceB.string == "Initial Value", "Expected 'Initial Value', got: " .. resourceB.string) + assert(#query == 2, "Expected 3 results, got: " .. #query) + for i,result in pairs(query) do + components = result:components() + assert(#components == 2, "Expected 2 components, got " .. #components) + local componentA = components[1] + local componentB = components[2] + assert(componentA._1 == "Default", "Expected 'Default', got: " .. componentA._1) + assert(componentB._1 == "Default", "Expected 'Default', got: " .. componentA._1) + end +end + + +function on_test_post_update() + return true +end + +function on_test_last() + assert(#runs == 1, "Expected 1 runs, got: " .. #runs) + return true +end \ No newline at end of file diff --git a/assets/tests/add_system/system_with_parameters__RETURNS.rhai b/assets/tests/add_system/system_with_parameters__RETURNS.rhai new file mode 100644 index 000000000..bbd01872c --- /dev/null +++ b/assets/tests/add_system/system_with_parameters__RETURNS.rhai @@ -0,0 +1,56 @@ +let runs = []; + +let ResourceTypeA = world.get_type_by_name.call("TestResource"); +let ResourceTypeB = world.get_type_by_name.call("TestResourceWithVariousFields"); +let ComponentA = world.get_type_by_name.call("CompWithFromWorldAndComponentData"); +let ComponentB = world.get_type_by_name.call("CompWithDefaultAndComponentData"); + +fn on_test() { + let post_update_schedule = world.get_schedule_by_name.call("PostUpdate"); + + let entity = world.spawn_.call(); + let entity2 = world.spawn_.call(); + + world.add_default_component.call(entity, ComponentA); + world.add_default_component.call(entity, ComponentB); + world.add_default_component.call(entity2, ComponentA); + world.add_default_component.call(entity2, ComponentB); + + let built_system = system_builder.call("my_parameterised_system", script_id) + .resource.call(ResourceTypeA) + .query.call(world.query.call().component.call(ComponentA).with_.call(ComponentB)) + .resource.call(ResourceTypeB); + + world.add_system.call(post_update_schedule, built_system); + + return true; +} + +fn my_parameterised_system(resourceA, query, resourceB) { + print("my_parameterised_system"); + runs.push("my_non_exclusive_system"); + + assert(type_of(resourceA) != (), "Expected to get resource but got nil"); + assert(type_of(query) != (), "Expected to get query but got nil"); + assert(type_of(resourceB) != (), "Expected to get resource but got nil"); + + assert(resourceA.bytes.len() == 6, "Expected 6 bytes, got: " + resourceA.bytes.len()); + assert(resourceB.string == "Initial Value", "Expected 'Initial Value', got: " + resourceB.string); + assert(query.len() == 2, "Expected 3 results, got: " + query.len()); + for result in query { + components = result.components.call(); + let componentA = components[0]; + let componentB = components[1]; + assert(componentA["_0"] == "Default", "Expected 'Default', got: " + componentA["_0"]); + assert(componentB["_0"] == "Default", "Expected 'Default', got: " + componentB["_0"]); + } +} + +fn on_test_post_update() { + return true; +} + +fn on_test_last() { + assert(runs.len == 1, "Expected 1 runs, got: " + runs.len); + return true; +} \ No newline at end of file diff --git a/assets/tests/query/query_can_access_components.lua b/assets/tests/query/query_can_access_components.lua new file mode 100644 index 000000000..72dc54ef5 --- /dev/null +++ b/assets/tests/query/query_can_access_components.lua @@ -0,0 +1,26 @@ +local entity_a = world.spawn() + +local componentA = world.get_type_by_name("CompWithFromWorldAndComponentData") +local componentB = world.get_type_by_name("CompWithDefaultAndComponentData") +local componentC = world.get_type_by_name("TestComponent") + +world.add_default_component(entity_a, componentA) +world.add_default_component(entity_a, componentB) +world.insert_component(entity_a, componentC, construct(componentC, { + strings = { [1] = "asd" } +})) + +local query_result = world.query():component(componentA):component(componentA):component(componentC):build() + +assert(#query_result == 1, "Expected 1 result, got " .. #query_result) +for i,result in pairs(query_result) do + assert(result:entity():index() == entity_a:index(), "Expected entity_a, got " .. result:entity():index()) + components = result:components() + assert(#components == 3, "Expected 3 components, got " .. #components) + A = components[1] + B = components[2] + C = components[3] + assert(A._1 == "Default", "Expected 'Default', got: " .. A._1) + assert(B._1 == "Default", "Expected 'Default', got: " .. B._1) + assert(C.strings[1] == "asd", "Expected 'asd', got: " .. C.strings[1]) +end diff --git a/assets/tests/query/query_can_access_components.rhai b/assets/tests/query/query_can_access_components.rhai new file mode 100644 index 000000000..335c17fb6 --- /dev/null +++ b/assets/tests/query/query_can_access_components.rhai @@ -0,0 +1,25 @@ +let entity_a = world.spawn_.call(); +let componentA = world.get_type_by_name.call("CompWithFromWorldAndComponentData"); +let componentB = world.get_type_by_name.call("CompWithDefaultAndComponentData"); +let componentC = world.get_type_by_name.call("TestComponent"); + +world.add_default_component.call(entity_a, componentA); +world.add_default_component.call(entity_a, componentB); +world.insert_component.call(entity_a, componentC, construct.call(componentC, #{ + strings: ["asd"] +})); + +let query_result = world.query.call().component.call(componentA).component.call(componentA).component.call(componentC).build.call(); + +assert(query_result.len == 1, "Expected 1 result, got " + query_result.len); +for (result, i) in query_result { + assert(result.entity.call().index.call() == entity_a.index.call(), "Expected entity_a, got " + result.entity.call().index.call()); + let components = result.components.call(); + assert(components.len == 3, "Expected 3 components, got " + components.len); + let A = components[0]; + let B = components[1]; + let C = components[2]; + assert(A["_0"] == "Default", "Expected 'Default', got: " + A["_0"]); + assert(B["_0"] == "Default", "Expected 'Default', got: " + B["_0"]); + assert(C.strings[0] == "asd", "Expected 'asd', got: " + C.strings[0]); +} \ No newline at end of file diff --git a/crates/bevy_mod_scripting_core/Cargo.toml b/crates/bevy_mod_scripting_core/Cargo.toml index c5f5ce68d..ad74afc23 100644 --- a/crates/bevy_mod_scripting_core/Cargo.toml +++ b/crates/bevy_mod_scripting_core/Cargo.toml @@ -29,9 +29,7 @@ mlua = { version = "0.10", default-features = false, optional = true } rhai = { version = "1.21", default-features = false, features = [ "sync", ], optional = true } - bevy = { workspace = true, default-features = false, features = ["bevy_asset"] } - thiserror = "1.0.31" parking_lot = "0.12.1" dashmap = "6" @@ -42,6 +40,8 @@ profiling = { workspace = true } bevy_mod_scripting_derive = { workspace = true } fixedbitset = "0.5" petgraph = "0.6" +bevy_mod_debugdump = "0.12" +bevy_system_reflection = { path = "../bevy_system_reflection", version = "0.1.0" } [dev-dependencies] test_utils = { workspace = true } diff --git a/crates/bevy_mod_scripting_core/src/bindings/access_map.rs b/crates/bevy_mod_scripting_core/src/bindings/access_map.rs index b913957ee..f93facdd2 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/access_map.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/access_map.rs @@ -4,7 +4,7 @@ use std::thread::ThreadId; use bevy::{ ecs::{component::ComponentId, world::unsafe_world_cell::UnsafeWorldCell}, prelude::Resource, - utils::hashbrown::HashMap, + utils::hashbrown::{HashMap, HashSet}, }; use parking_lot::Mutex; use smallvec::SmallVec; @@ -226,17 +226,100 @@ impl From for ReflectAllocationId { /// A map of access claims pub struct AccessMap(Mutex); +/// A trait for controlling system world access at runtime. +/// +/// This trait provides methods to claim and release read, write, and global access +/// to various parts of the world. Implementations of this trait manage internal state +/// to ensure safe and concurrent access to resources. Methods include scope-based locking, +/// as well as introspection of access state via code location information. +pub trait DynamicSystemMeta { + /// Executes the provided closure within a temporary access scope. + /// + /// Any accesses claimed within the scope are rolled back once the closure returns. + fn with_scope O>(&self, f: F) -> O; + + /// Returns `true` if the world is exclusively locked. + /// + /// When exclusively locked, no additional individual or global accesses may be claimed. + fn is_locked_exclusively(&self) -> bool; + + /// Retrieves the code location where the global lock was claimed (if any). + /// + /// This is useful for debugging conflicts involving the global access lock. + fn global_access_location(&self) -> Option>; + + /// Attempts to claim read access for the given key. + /// + /// Returns `true` if the read access is successfully claimed. The claim will fail if + /// the key is currently locked for write or if a global lock is active. + #[track_caller] + fn claim_read_access(&self, key: K) -> bool; + + /// Attempts to claim write access for the given key. + /// + /// Returns `true` if the write access is successfully claimed. Write access fails if any + /// read or write access is active for the key or if a global lock is held. + #[track_caller] + fn claim_write_access(&self, key: K) -> bool; + + /// Attempts to claim a global access lock. + /// + /// Returns `true` if the global access is successfully claimed. Global access precludes any + /// individual accesses until it is released. + #[track_caller] + fn claim_global_access(&self) -> bool; + + /// Releases an access claimed for the provided key. + /// + /// # Panics + /// + /// Panics if the access is released by a thread different from the one that claimed it. + fn release_access(&self, key: K); + + /// Releases an active global access lock. + /// + /// # Panics + /// + /// Panics if the global access is released from a thread other than the one that claimed it. + fn release_global_access(&self); + + /// Returns a list of active accesses. + /// + /// The list is provided as key and corresponding access count pairs. + fn list_accesses(&self) -> Vec<(K, AccessCount)>; + + /// Returns the number of active individual accesses. + /// + /// In the case of a global lock, this method considers that as a single active access. + fn count_accesses(&self) -> usize; + + /// Releases all active accesses. + /// + /// Both individual and global accesses will be removed. + fn release_all_accesses(&self); + + /// Returns the location where the specified key was first accessed. + /// + /// This is useful for debugging and tracing access failures. + fn access_location(&self, key: K) -> Option>; + + /// Returns the location of the first access among all keys. + /// + /// This can assist in identifying the origin of access conflicts. + fn access_first_location(&self) -> Option>; +} + #[derive(Debug, Default, Clone)] struct AccessMapInner { individual_accesses: HashMap, global_lock: AccessCount, } +const GLOBAL_KEY: u64 = 0; + #[profiling::all_functions] -impl AccessMap { - /// Increments the scope and executes operations which will be unrolled at the end. - /// Any accesses added inside the scope are rolled back after f() returns. - pub fn with_scope O>(&self, f: F) -> O { +impl DynamicSystemMeta for AccessMap { + fn with_scope O>(&self, f: F) -> O { // Snapshot the current inner state. let backup = { let inner = self.0.lock(); @@ -254,23 +337,19 @@ impl AccessMap { result } - /// Checks if the map is locked exclusively. - pub fn is_locked_exclusively(&self) -> bool { + fn is_locked_exclusively(&self) -> bool { let inner = self.0.lock(); // If global_lock cannot be written, then it is locked exclusively. !inner.global_lock.can_write() } - /// Retrieves the location of the global lock if any. - pub fn global_access_location(&self) -> Option> { + fn global_access_location(&self) -> Option> { let inner = self.0.lock(); inner.global_lock.as_location() } - /// Tries to claim read access. Returns false if somebody else is writing to the same key, - /// or if a global lock prevents the access. #[track_caller] - pub fn claim_read_access(&self, key: K) -> bool { + fn claim_read_access(&self, key: K) -> bool { let mut inner = self.0.lock(); if !inner.global_lock.can_write() { @@ -278,6 +357,11 @@ impl AccessMap { } let key = key.as_index(); + if key == GLOBAL_KEY { + bevy::log::error!("Trying to claim read access to global key, this is not allowed"); + return false; + } + let entry = inner.individual_accesses.entry(key).or_default(); if entry.can_read() { entry.read_by.push(ClaimOwner { @@ -290,10 +374,8 @@ impl AccessMap { } } - /// Tries to claim write access. Returns false if somebody else is reading or writing to the same key, - /// or if a global lock prevents the access. #[track_caller] - pub fn claim_write_access(&self, key: K) -> bool { + fn claim_write_access(&self, key: K) -> bool { let mut inner = self.0.lock(); if !inner.global_lock.can_write() { @@ -301,6 +383,11 @@ impl AccessMap { } let key = key.as_index(); + if key == GLOBAL_KEY { + bevy::log::error!("Trying to claim write access to global key, this is not allowed"); + return false; + } + let entry = inner.individual_accesses.entry(key).or_default(); if entry.can_write() { entry.read_by.push(ClaimOwner { @@ -314,10 +401,8 @@ impl AccessMap { } } - /// Tries to claim global access. This prevents any other access from happening simultaneously. - /// Returns false if any access is currently active. #[track_caller] - pub fn claim_global_access(&self) -> bool { + fn claim_global_access(&self) -> bool { let mut inner = self.0.lock(); if !inner.individual_accesses.is_empty() || !inner.global_lock.can_write() { @@ -331,11 +416,7 @@ impl AccessMap { true } - /// Releases an access. - /// - /// # Panics - /// if the access is released from a different thread than it was claimed from. - pub fn release_access(&self, key: K) { + fn release_access(&self, key: K) { let mut inner = self.0.lock(); let key = key.as_index(); @@ -354,8 +435,7 @@ impl AccessMap { } } - /// Releases a global access. - pub fn release_global_access(&self) { + fn release_global_access(&self) { let mut inner = self.0.lock(); inner.global_lock.written = false; if let Some(claim) = inner.global_lock.read_by.pop() { @@ -367,8 +447,7 @@ impl AccessMap { } } - /// Lists all active accesses. - pub fn list_accesses(&self) -> Vec<(K, AccessCount)> { + fn list_accesses(&self) -> Vec<(K, AccessCount)> { let inner = self.0.lock(); inner .individual_accesses @@ -377,8 +456,7 @@ impl AccessMap { .collect() } - /// Counts the number of active individual accesses. - pub fn count_accesses(&self) -> usize { + fn count_accesses(&self) -> usize { if self.is_locked_exclusively() { 1 } else { @@ -387,8 +465,7 @@ impl AccessMap { } } - /// Releases all accesses. - pub fn release_all_accesses(&self) { + fn release_all_accesses(&self) { let mut inner = self.0.lock(); inner.individual_accesses.clear(); // Release global access if held. @@ -396,11 +473,7 @@ impl AccessMap { inner.global_lock.read_by.clear(); } - /// Returns the location where the key was first accessed. - pub fn access_location( - &self, - key: K, - ) -> Option> { + fn access_location(&self, key: K) -> Option> { let inner = self.0.lock(); if key.as_index() == 0 { // it blocked by individual access @@ -419,8 +492,7 @@ impl AccessMap { } } - /// Returns the location of the first access among all entries. - pub fn access_first_location(&self) -> Option> { + fn access_first_location(&self) -> Option> { let inner = self.0.lock(); inner .individual_accesses @@ -429,6 +501,199 @@ impl AccessMap { } } +/// An inverse of [`AccessMap`], It limits the accesses allowed to be claimed to those in a pre-specified subset. +pub struct SubsetAccessMap { + inner: AccessMap, + subset: Box bool + Send + Sync + 'static>, +} + +impl SubsetAccessMap { + /// Creates a new subset access map with the provided subset of ID's as well as a exception function. + pub fn new( + subset: impl IntoIterator, + exception: impl Fn(u64) -> bool + Send + Sync + 'static, + ) -> Self { + let set = subset + .into_iter() + .map(|k| k.as_index()) + .collect::>(); + Self { + inner: Default::default(), + subset: Box::new(move |id| set.contains(&id) || exception(id)), + } + } + + fn in_subset(&self, key: u64) -> bool { + (self.subset)(key) + } +} + +impl DynamicSystemMeta for SubsetAccessMap { + fn with_scope O>(&self, f: F) -> O { + self.inner.with_scope(f) + } + + fn is_locked_exclusively(&self) -> bool { + self.inner.is_locked_exclusively() + } + + fn global_access_location(&self) -> Option> { + self.inner.global_access_location() + } + + fn claim_read_access(&self, key: K) -> bool { + if !self.in_subset(key.as_index()) { + return false; + } + self.inner.claim_read_access(key) + } + + fn claim_write_access(&self, key: K) -> bool { + if !self.in_subset(key.as_index()) { + return false; + } + self.inner.claim_write_access(key) + } + + fn claim_global_access(&self) -> bool { + if !self.in_subset(0) { + return false; + } + self.inner.claim_global_access() + } + + fn release_access(&self, key: K) { + self.inner.release_access(key); + } + + fn release_global_access(&self) { + self.inner.release_global_access(); + } + + fn list_accesses(&self) -> Vec<(K, AccessCount)> { + self.inner.list_accesses() + } + + fn count_accesses(&self) -> usize { + self.inner.count_accesses() + } + + fn release_all_accesses(&self) { + self.inner.release_all_accesses(); + } + + fn access_location(&self, key: K) -> Option> { + self.inner.access_location(key) + } + + fn access_first_location(&self) -> Option> { + self.inner.access_first_location() + } +} + +/// A polymorphic enum for access map types. +/// +/// Equivalent to `dyn DynamicSystemMeta` for most purposes +pub enum AnyAccessMap { + /// A map which allows any and all accesses to be claimed + UnlimitedAccessMap(AccessMap), + /// A map which only allows accesses to keys in a pre-specified subset + SubsetAccessMap(SubsetAccessMap), +} + +impl DynamicSystemMeta for AnyAccessMap { + fn with_scope O>(&self, f: F) -> O { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.with_scope(f), + AnyAccessMap::SubsetAccessMap(map) => map.with_scope(f), + } + } + + fn is_locked_exclusively(&self) -> bool { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.is_locked_exclusively(), + AnyAccessMap::SubsetAccessMap(map) => map.is_locked_exclusively(), + } + } + + fn global_access_location(&self) -> Option> { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.global_access_location(), + AnyAccessMap::SubsetAccessMap(map) => map.global_access_location(), + } + } + + fn claim_read_access(&self, key: K) -> bool { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.claim_read_access(key), + AnyAccessMap::SubsetAccessMap(map) => map.claim_read_access(key), + } + } + + fn claim_write_access(&self, key: K) -> bool { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.claim_write_access(key), + AnyAccessMap::SubsetAccessMap(map) => map.claim_write_access(key), + } + } + + fn claim_global_access(&self) -> bool { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.claim_global_access(), + AnyAccessMap::SubsetAccessMap(map) => map.claim_global_access(), + } + } + + fn release_access(&self, key: K) { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.release_access(key), + AnyAccessMap::SubsetAccessMap(map) => map.release_access(key), + } + } + + fn release_global_access(&self) { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.release_global_access(), + AnyAccessMap::SubsetAccessMap(map) => map.release_global_access(), + } + } + + fn list_accesses(&self) -> Vec<(K, AccessCount)> { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.list_accesses(), + AnyAccessMap::SubsetAccessMap(map) => map.list_accesses(), + } + } + + fn count_accesses(&self) -> usize { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.count_accesses(), + AnyAccessMap::SubsetAccessMap(map) => map.count_accesses(), + } + } + + fn release_all_accesses(&self) { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.release_all_accesses(), + AnyAccessMap::SubsetAccessMap(map) => map.release_all_accesses(), + } + } + + fn access_location(&self, key: K) -> Option> { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.access_location(key), + AnyAccessMap::SubsetAccessMap(map) => map.access_location(key), + } + } + + fn access_first_location(&self) -> Option> { + match self { + AnyAccessMap::UnlimitedAccessMap(map) => map.access_first_location(), + AnyAccessMap::SubsetAccessMap(map) => map.access_first_location(), + } + } +} + /// A trait for displaying a code location nicely pub trait DisplayCodeLocation { /// Displays the location @@ -452,15 +717,15 @@ impl DisplayCodeLocation for Option> { /// A macro for claiming access to a value for reading macro_rules! with_access_read { ($access_map:expr, $id:expr, $msg:expr, $body:block) => {{ - if !$access_map.claim_read_access($id) { + if !$crate::bindings::access_map::DynamicSystemMeta::claim_read_access($access_map, $id) { Err($crate::error::InteropError::cannot_claim_access( $id, - $access_map.access_location($id), + $crate::bindings::access_map::DynamicSystemMeta::access_location($access_map, $id), $msg, )) } else { let result = $body; - $access_map.release_access($id); + $crate::bindings::access_map::DynamicSystemMeta::release_access($access_map, $id); Ok(result) } }}; @@ -470,15 +735,15 @@ macro_rules! with_access_read { /// A macro for claiming access to a value for writing macro_rules! with_access_write { ($access_map:expr, $id:expr, $msg:expr, $body:block) => { - if !$access_map.claim_write_access($id) { + if !$crate::bindings::access_map::DynamicSystemMeta::claim_write_access($access_map, $id) { Err($crate::error::InteropError::cannot_claim_access( $id, - $access_map.access_location($id), + $crate::bindings::access_map::DynamicSystemMeta::access_location($access_map, $id), $msg, )) } else { let result = $body; - $access_map.release_access($id); + $crate::bindings::access_map::DynamicSystemMeta::release_access($access_map, $id); Ok(result) } }; @@ -488,17 +753,19 @@ macro_rules! with_access_write { /// A macro for claiming global access macro_rules! with_global_access { ($access_map:expr, $msg:expr, $body:block) => { - if !$access_map.claim_global_access() { + if !$crate::bindings::access_map::DynamicSystemMeta::claim_global_access($access_map) { Err($crate::error::InteropError::cannot_claim_access( $crate::bindings::access_map::ReflectAccessId::for_global(), - $access_map - .access_location($crate::bindings::access_map::ReflectAccessId::for_global()), + $crate::bindings::access_map::DynamicSystemMeta::access_location( + $access_map, + $crate::bindings::access_map::ReflectAccessId::for_global(), + ), $msg, )) } else { #[allow(clippy::redundant_closure_call)] let result = (|| $body)(); - $access_map.release_global_access(); + $crate::bindings::access_map::DynamicSystemMeta::release_global_access($access_map); Ok(result) } }; @@ -509,17 +776,41 @@ mod test { use super::*; #[test] - fn test_list_accesses() { + fn access_map_list_accesses() { let access_map = AccessMap::default(); - access_map.claim_read_access(0); - access_map.claim_write_access(1); + access_map.claim_read_access(1); + access_map.claim_write_access(2); let accesses = access_map.list_accesses::(); assert_eq!(accesses.len(), 2); - let access_0 = accesses.iter().find(|(k, _)| *k == 0).unwrap(); - let access_1 = accesses.iter().find(|(k, _)| *k == 1).unwrap(); + let access_0 = accesses.iter().find(|(k, _)| *k == 1).unwrap(); + let access_1 = accesses.iter().find(|(k, _)| *k == 2).unwrap(); + + assert_eq!(access_0.1.readers(), 1); + assert_eq!(access_1.1.readers(), 1); + + assert!(!access_0.1.written); + assert!(access_1.1.written); + } + + #[test] + fn subset_access_map_list_accesses() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 1 || id == 2), + }; + + subset_access_map.claim_read_access(1); + subset_access_map.claim_write_access(2); + + let accesses = subset_access_map.list_accesses::(); + + assert_eq!(accesses.len(), 2); + let access_0 = accesses.iter().find(|(k, _)| *k == 1).unwrap(); + let access_1 = accesses.iter().find(|(k, _)| *k == 2).unwrap(); assert_eq!(access_0.1.readers(), 1); assert_eq!(access_1.1.readers(), 1); @@ -529,58 +820,119 @@ mod test { } #[test] - fn test_read_access_blocks_write() { + fn access_map_read_access_blocks_write() { + let access_map = AccessMap::default(); + + assert!(access_map.claim_read_access(1)); + assert!(!access_map.claim_write_access(1)); + access_map.release_access(1); + assert!(access_map.claim_write_access(1)); + } + + #[test] + fn subset_access_map_read_access_blocks_write() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 1), + }; + + assert!(subset_access_map.claim_read_access(1)); + assert!(!subset_access_map.claim_write_access(1)); + subset_access_map.release_access(1); + assert!(subset_access_map.claim_write_access(1)); + } + + #[test] + fn access_map_write_access_blocks_read() { let access_map = AccessMap::default(); - assert!(access_map.claim_read_access(0)); - assert!(!access_map.claim_write_access(0)); - access_map.release_access(0); - assert!(access_map.claim_write_access(0)); + assert!(access_map.claim_write_access(1)); + assert!(!access_map.claim_read_access(1)); + access_map.release_access(1); + assert!(access_map.claim_read_access(1)); } #[test] - fn test_write_access_blocks_read() { + fn subset_access_map_write_access_blocks_read() { let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 1), + }; - assert!(access_map.claim_write_access(0)); - assert!(!access_map.claim_read_access(0)); - access_map.release_access(0); - assert!(access_map.claim_read_access(0)); + assert!(subset_access_map.claim_write_access(1)); + assert!(!subset_access_map.claim_read_access(1)); + subset_access_map.release_access(1); + assert!(subset_access_map.claim_read_access(1)); } #[test] - fn test_global_access_blocks_all() { + fn access_map_global_access_blocks_all() { let access_map = AccessMap::default(); assert!(access_map.claim_global_access()); - assert!(!access_map.claim_read_access(0)); - assert!(!access_map.claim_write_access(0)); + assert!(!access_map.claim_read_access(1)); + assert!(!access_map.claim_write_access(1)); access_map.release_global_access(); - assert!(access_map.claim_write_access(0)); - access_map.release_access(0); - assert!(access_map.claim_read_access(0)); + assert!(access_map.claim_write_access(1)); + access_map.release_access(1); + assert!(access_map.claim_read_access(1)); + } + + #[test] + fn subset_access_map_global_access_blocks_all() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 1 || id == 0), + }; + + assert!(subset_access_map.claim_global_access()); + assert!(!subset_access_map.claim_read_access(1)); + assert!(!subset_access_map.claim_write_access(1)); + subset_access_map.release_global_access(); + assert!(subset_access_map.claim_write_access(1)); + subset_access_map.release_access(1); + assert!(subset_access_map.claim_read_access(1)); } #[test] - fn any_access_blocks_global() { + fn access_map_any_access_blocks_global() { let access_map = AccessMap::default(); - assert!(access_map.claim_read_access(0)); + assert!(access_map.claim_read_access(1)); assert!(!access_map.claim_global_access()); - access_map.release_access(0); + access_map.release_access(1); - assert!(access_map.claim_write_access(0)); + assert!(access_map.claim_write_access(1)); assert!(!access_map.claim_global_access()); } + #[test] + fn subset_map_any_access_blocks_global() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 0 || id == 1), + }; + + assert!(subset_access_map.claim_read_access(1)); + assert!(!subset_access_map.claim_global_access()); + subset_access_map.release_access(1); + + assert!(subset_access_map.claim_write_access(1)); + assert!(!subset_access_map.claim_global_access()); + } + #[test] #[should_panic] - fn releasing_read_access_from_wrong_thread_panics() { + fn access_map_releasing_read_access_from_wrong_thread_panics() { let access_map = AccessMap::default(); - access_map.claim_read_access(0); + access_map.claim_read_access(1); std::thread::spawn(move || { - access_map.release_access(0); + access_map.release_access(1); }) .join() .unwrap(); @@ -588,19 +940,53 @@ mod test { #[test] #[should_panic] - fn releasing_write_access_from_wrong_thread_panics() { + fn subset_map_releasing_read_access_from_wrong_thread_panics() { let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 1), + }; - access_map.claim_write_access(0); + subset_access_map.claim_read_access(1); std::thread::spawn(move || { - access_map.release_access(0); + subset_access_map.release_access(1); }) .join() .unwrap(); } #[test] - fn test_as_and_from_index_for_access_id_non_overlapping() { + #[should_panic] + fn access_map_releasing_write_access_from_wrong_thread_panics() { + let access_map = AccessMap::default(); + + access_map.claim_write_access(1); + std::thread::spawn(move || { + access_map.release_access(1); + }) + .join() + .unwrap(); + } + + #[test] + #[should_panic] + fn subset_map_releasing_write_access_from_wrong_thread_panics() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 1), + }; + + subset_access_map.claim_write_access(1); + std::thread::spawn(move || { + subset_access_map.release_access(1); + }) + .join() + .unwrap(); + } + + #[test] + fn as_and_from_index_for_access_id_non_overlapping() { let global = ReflectAccessId::for_global(); let first_component = ReflectAccessId { @@ -637,10 +1023,10 @@ mod test { } #[test] - fn test_with_scope_unrolls_individual_accesses() { + fn access_map_with_scope_unrolls_individual_accesses() { let access_map = AccessMap::default(); // Claim a read access outside the scope - assert!(access_map.claim_read_access(0)); + assert!(access_map.claim_read_access(3)); // Inside with_scope, claim additional accesses access_map.with_scope(|| { @@ -653,22 +1039,52 @@ mod test { // After with_scope returns, accesses claimed inside (keys 1 and 2) are unrolled. let accesses = access_map.list_accesses::(); - // Only the access claimed outside (key 0) remains. + // Only the access claimed outside (key 3) remains. + assert_eq!(accesses.len(), 1); + let (k, count) = &accesses[0]; + assert_eq!(*k, 3); + // The outside access remains valid. + assert!(count.readers() > 0); + } + + #[test] + fn subset_map_with_scope_unrolls_individual_accesses() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 1 || id == 2 || id == 3), + }; + + // Claim a read access outside the scope + assert!(subset_access_map.claim_read_access(3)); + + // Inside with_scope, claim additional accesses + subset_access_map.with_scope(|| { + assert!(subset_access_map.claim_read_access(1)); + assert!(subset_access_map.claim_write_access(2)); + // At this point, individual_accesses contains keys 0, 1 and 2. + let accesses = subset_access_map.list_accesses::(); + assert_eq!(accesses.len(), 3); + }); + + // After with_scope returns, accesses claimed inside (keys 1 and 2) are unrolled. + let accesses = subset_access_map.list_accesses::(); + // Only the access claimed outside (key 3) remains. assert_eq!(accesses.len(), 1); let (k, count) = &accesses[0]; - assert_eq!(*k, 0); + assert_eq!(*k, 3); // The outside access remains valid. assert!(count.readers() > 0); } #[test] - fn test_with_scope_unrolls_global_accesses() { + fn access_map_with_scope_unrolls_global_accesses() { let access_map = AccessMap::default(); access_map.with_scope(|| { assert!(access_map.claim_global_access()); // At this point, global_access is claimed. - assert!(!access_map.claim_read_access(0)); + assert!(!access_map.claim_read_access(1)); }); let accesses = access_map.list_accesses::(); @@ -676,7 +1092,25 @@ mod test { } #[test] - fn count_accesses_counts_globals() { + fn subset_map_with_scope_unrolls_global_accesses() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 0 || id == 1), + }; + + subset_access_map.with_scope(|| { + assert!(subset_access_map.claim_global_access()); + // At this point, global_access is claimed. + assert!(!subset_access_map.claim_read_access(1)); + }); + + let accesses = subset_access_map.list_accesses::(); + assert_eq!(accesses.len(), 0); + } + + #[test] + fn access_map_count_accesses_counts_globals() { let access_map = AccessMap::default(); // Initially, no accesses are active. @@ -700,7 +1134,35 @@ mod test { } #[test] - fn location_is_tracked_for_all_types_of_accesses() { + fn subset_map_count_accesses_counts_globals() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 0 || id == 1 || id == 2), + }; + + // Initially, no accesses are active. + assert_eq!(subset_access_map.count_accesses(), 0); + + // Claim global access. When global access is active, + // count_accesses should return 1. + assert!(subset_access_map.claim_global_access()); + assert_eq!(subset_access_map.count_accesses(), 1); + subset_access_map.release_global_access(); + + // Now claim individual accesses. + assert!(subset_access_map.claim_read_access(1)); + assert!(subset_access_map.claim_write_access(2)); + // Since two separate keys were claimed, count_accesses should return 2. + assert_eq!(subset_access_map.count_accesses(), 2); + + // Cleanup individual accesses. + subset_access_map.release_access(1); + subset_access_map.release_access(2); + } + + #[test] + fn access_map_location_is_tracked_for_all_types_of_accesses() { let access_map = AccessMap::default(); assert!(access_map.claim_global_access()); @@ -719,4 +1181,61 @@ mod test { assert!(access_map.access_location(2).is_some()); access_map.release_access(2); } + + #[test] + fn subset_map_location_is_tracked_for_all_types_of_accesses() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 0 || id == 1 || id == 2), + }; + + assert!(subset_access_map.claim_global_access()); + assert!(subset_access_map + .access_location(ReflectAccessId::for_global()) + .is_some()); + subset_access_map.release_global_access(); + + // Claim a read access + assert!(subset_access_map.claim_read_access(1)); + assert!(subset_access_map.access_location(1).is_some()); + subset_access_map.release_access(1); + + // Claim a write access + assert!(subset_access_map.claim_write_access(2)); + assert!(subset_access_map.access_location(2).is_some()); + subset_access_map.release_access(2); + } + + #[test] + fn subset_map_prevents_access_to_out_of_subset_access() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 1), + }; + + assert!(!subset_access_map.claim_read_access(2)); + assert!(!subset_access_map.claim_write_access(2)); + assert!(!subset_access_map.claim_global_access()); + } + + #[test] + fn subset_map_retains_subset_in_scope() { + let access_map = AccessMap::default(); + let subset_access_map = SubsetAccessMap { + inner: access_map, + subset: Box::new(|id| id == 1), + }; + + subset_access_map.with_scope(|| { + assert!(subset_access_map.claim_read_access(1)); + assert!(!subset_access_map.claim_read_access(2)); + assert!(!subset_access_map.claim_write_access(2)); + }); + + assert!(subset_access_map.claim_read_access(1)); + assert!(!subset_access_map.claim_read_access(2)); + assert!(!subset_access_map.claim_write_access(2)); + } } diff --git a/crates/bevy_mod_scripting_core/src/bindings/function/into.rs b/crates/bevy_mod_scripting_core/src/bindings/function/into.rs index 4f50fd492..d3d1dfbbb 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/function/into.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/function/into.rs @@ -19,6 +19,14 @@ use super::{ pub trait IntoScript { /// Convert this value into a [`ScriptValue`]. fn into_script(self, world: WorldGuard) -> Result; + + /// Convert this value into a [`ScriptValue`], returning an error as a ScriptValue if an error occurs. + fn into_script_inline_error(self, world: WorldGuard) -> ScriptValue + where + Self: Sized, + { + self.into_script(world).unwrap_or_else(ScriptValue::Error) + } } impl IntoScript for ScriptValue { diff --git a/crates/bevy_mod_scripting_core/src/bindings/globals/mod.rs b/crates/bevy_mod_scripting_core/src/bindings/globals/mod.rs index 22ed1f019..5ee9b1372 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/globals/mod.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/globals/mod.rs @@ -209,7 +209,7 @@ mod test { assert_eq!(registry.len(), 1); assert_eq!( - (registry.get("foo").unwrap().maker.clone().unwrap())(WorldGuard::new( + (registry.get("foo").unwrap().maker.clone().unwrap())(WorldGuard::new_exclusive( &mut World::new() )) .unwrap(), @@ -220,7 +220,7 @@ mod test { assert_eq!(registry.len(), 1); assert_eq!( - (registry.get("foo").unwrap().maker.clone().unwrap())(WorldGuard::new( + (registry.get("foo").unwrap().maker.clone().unwrap())(WorldGuard::new_exclusive( &mut World::new() )) .unwrap(), diff --git a/crates/bevy_mod_scripting_core/src/bindings/mod.rs b/crates/bevy_mod_scripting_core/src/bindings/mod.rs index 9dd2de5e7..2b9c91d07 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/mod.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/mod.rs @@ -8,6 +8,7 @@ pub mod pretty_print; pub mod query; pub mod reference; pub mod schedule; +pub mod script_system; pub mod script_value; pub mod world; diff --git a/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs b/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs index 1c5203598..16ce6458d 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/pretty_print.rs @@ -681,7 +681,7 @@ mod test { #[test] fn test_type_id() { let mut world = setup_world(); - let world = WorldGuard::new(&mut world); + let world = WorldGuard::new_exclusive(&mut world); let type_id = TypeId::of::(); assert_eq!(type_id.display_with_world(world.clone()), "usize"); @@ -700,7 +700,7 @@ mod test { #[test] fn test_reflect_base_type() { let mut world = setup_world(); - let world = WorldGuard::new(&mut world); + let world = WorldGuard::new_exclusive(&mut world); let type_id = TypeId::of::(); @@ -736,7 +736,7 @@ mod test { fn test_reflect_reference() { let mut world = setup_world(); - let world = WorldGuard::new(&mut world); + let world = WorldGuard::new_exclusive(&mut world); let type_id = TypeId::of::(); @@ -769,7 +769,7 @@ mod test { #[test] fn test_hashmap() { let mut world = setup_world(); - let world = WorldGuard::new(&mut world); + let world = WorldGuard::new_exclusive(&mut world); let mut map = std::collections::HashMap::new(); map.insert("hello".to_owned(), ScriptValue::Bool(true)); diff --git a/crates/bevy_mod_scripting_core/src/bindings/query.rs b/crates/bevy_mod_scripting_core/src/bindings/query.rs index d40896c51..138737bd8 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/query.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/query.rs @@ -3,7 +3,12 @@ use super::{ReflectReference, WorldAccessGuard}; use crate::{error::InteropError, with_global_access}; use bevy::{ - ecs::{component::ComponentId, entity::Entity}, + ecs::{ + component::ComponentId, + entity::Entity, + query::{QueryData, QueryState}, + world::World, + }, prelude::{EntityRef, QueryBuilder}, reflect::{ParsedPath, Reflect, TypeRegistration}, }; @@ -131,7 +136,7 @@ impl std::fmt::Display for ScriptTypeRegistration { #[reflect(opaque)] /// A builder for a query. pub struct ScriptQueryBuilder { - components: Vec, + pub(crate) components: Vec, with: Vec, without: Vec, } @@ -171,6 +176,27 @@ impl ScriptQueryBuilder { self.without.push(without); self } + + /// Builds the query into a query state as used in systems. + pub fn as_query_state(&self, world: &mut World) -> QueryState { + let mut dynamic_query = QueryBuilder::::new(world); + // we don't actually want to fetch the data for components now, only figure out + // which entities match the query + // so we might be being slightly overkill + for c in &self.components { + dynamic_query.ref_id(c.component_id()); + } + + for w in &self.with { + dynamic_query.with_id(w.component_id()); + } + + for without_id in &self.without { + dynamic_query.without_id(without_id.component_id()); + } + + dynamic_query.build() + } } #[derive(Clone, Reflect)] @@ -190,26 +216,9 @@ impl WorldAccessGuard<'_> { &self, query: ScriptQueryBuilder, ) -> Result, InteropError> { - with_global_access!(self.inner.accesses, "Could not query", { + with_global_access!(&self.inner.accesses, "Could not query", { let world = unsafe { self.as_unsafe_world_cell()?.world_mut() }; - let mut dynamic_query = QueryBuilder::::new(world); - - // we don't actually want to fetch the data for components now, only figure out - // which entities match the query - // so we might be being slightly overkill - for c in &query.components { - dynamic_query.ref_id(c.component_id()); - } - - for w in query.with { - dynamic_query.with_id(w.component_id()); - } - - for without_id in query.without { - dynamic_query.without_id(without_id.component_id()); - } - - let mut built_query = dynamic_query.build(); + let mut built_query = query.as_query_state::(world); let query_result = built_query.iter(world); Ok(query_result diff --git a/crates/bevy_mod_scripting_core/src/bindings/reference.rs b/crates/bevy_mod_scripting_core/src/bindings/reference.rs index 5b9e334ee..e1266c8bd 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/reference.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/reference.rs @@ -25,15 +25,13 @@ use std::{any::TypeId, fmt::Debug}; /// An accessor to a `dyn PartialReflect` struct, stores a base ID of the type and a reflection path /// safe to build but to reflect on the value inside you need to ensure aliasing rules are upheld #[derive(Debug, Clone, PartialEq, Eq, Reflect)] -#[reflect(Default)] +#[reflect(Default, opaque)] pub struct ReflectReference { - #[reflect(ignore)] /// The base type and id of the value we want to access pub base: ReflectBaseType, // TODO: experiment with Fixed capacity vec, boxed array etc, compromise between heap allocation and runtime cost // needs benchmarks first though /// The path from the top level type to the actual value we want to access - #[reflect(ignore)] pub reflect_path: ParsedPath, } @@ -243,7 +241,7 @@ impl ReflectReference { ) -> Result { let access_id = ReflectAccessId::for_reference(self.base.base_id.clone()); with_access_read!( - world.inner.accesses, + &world.inner.accesses, access_id, "could not access reflect reference", { unsafe { self.reflect_unsafe(world.clone()) }.map(f)? } @@ -260,7 +258,7 @@ impl ReflectReference { ) -> Result { let access_id = ReflectAccessId::for_reference(self.base.base_id.clone()); with_access_write!( - world.inner.accesses, + &world.inner.accesses, access_id, "Could not access reflect reference mutably", { unsafe { self.reflect_mut_unsafe(world.clone()) }.map(f)? } @@ -635,7 +633,7 @@ mod test { .spawn(Component(vec!["hello".to_owned(), "world".to_owned()])) .id(); - let world_guard = WorldGuard::new(&mut world); + let world_guard = WorldGuard::new_exclusive(&mut world); let mut component_ref = ReflectReference::new_component_ref::(entity, world_guard.clone()) @@ -715,7 +713,7 @@ mod test { world.insert_resource(Resource(vec!["hello".to_owned(), "world".to_owned()])); - let world_guard = WorldGuard::new(&mut world); + let world_guard = WorldGuard::new_exclusive(&mut world); let mut resource_ref = ReflectReference::new_resource_ref::(world_guard.clone()) .expect("could not create resource reference"); @@ -794,7 +792,7 @@ mod test { let value = Component(vec!["hello".to_owned(), "world".to_owned()]); - let world_guard = WorldGuard::new(&mut world); + let world_guard = WorldGuard::new_exclusive(&mut world); let allocator = world_guard.allocator(); let mut allocator_write = allocator.write(); let mut allocation_ref = ReflectReference::new_allocated(value, &mut allocator_write); diff --git a/crates/bevy_mod_scripting_core/src/bindings/schedule.rs b/crates/bevy_mod_scripting_core/src/bindings/schedule.rs index 8f798f798..62bc2b763 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/schedule.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/schedule.rs @@ -1,144 +1,21 @@ //! Dynamic scheduling from scripts -use std::{ - any::TypeId, - borrow::Cow, - collections::HashMap, - hash::Hash, - ops::Deref, - sync::{ - atomic::{AtomicUsize, Ordering}, - Arc, - }, -}; - +use super::{script_system::ScriptSystemBuilder, WorldAccessGuard}; +use crate::{error::InteropError, IntoScriptPluginParams}; use bevy::{ app::{ First, FixedFirst, FixedLast, FixedMain, FixedPostUpdate, FixedPreUpdate, FixedUpdate, Last, PostStartup, PostUpdate, PreStartup, PreUpdate, RunFixedMainLoop, Startup, Update, }, ecs::{ - entity::Entity, - intern::Interned, - schedule::{ - InternedScheduleLabel, IntoSystemConfigs, NodeId, Schedule, ScheduleLabel, Schedules, - SystemSet, - }, - system::{IntoSystem, Resource, System, SystemInput, SystemState}, + schedule::{Schedule, ScheduleLabel, Schedules}, + system::Resource, world::World, }, - reflect::Reflect, }; +use bevy_system_reflection::{ReflectSchedule, ReflectSystem}; use parking_lot::RwLock; - -use crate::{ - error::InteropError, - event::CallbackLabel, - extractors::{HandlerContext, WithWorldGuard}, - handler::handle_script_errors, - script::ScriptId, - IntoScriptPluginParams, -}; - -use super::{WorldAccessGuard, WorldGuard}; - -#[derive(Reflect, Debug, Clone)] -/// A reflectable system. -pub struct ReflectSystem { - name: Cow<'static, str>, - type_id: TypeId, - node_id: ReflectNodeId, -} - -#[derive(Reflect, Clone, Debug)] -#[reflect(opaque)] -pub(crate) struct ReflectNodeId(pub(crate) NodeId); - -impl ReflectSystem { - /// Creates a reflect system from a system specification - pub fn from_system( - system: &dyn System, - node_id: NodeId, - ) -> Self { - ReflectSystem { - name: system.name().clone(), - type_id: system.type_id(), - node_id: ReflectNodeId(node_id), - } - } - - /// gets the short identifier of the system, i.e. just the function name - pub fn identifier(&self) -> &str { - // if it contains generics it might contain more than - if self.name.contains("<") { - self.name - .split("<") - .next() - .unwrap_or_default() - .split("::") - .last() - .unwrap_or_default() - } else { - self.name.split("::").last().unwrap_or_default() - } - } - - /// gets the path of the system, i.e. the fully qualified function name - pub fn path(&self) -> &str { - self.name.as_ref() - } -} - -/// A reflectable schedule. -#[derive(Reflect, Clone, Debug)] -pub struct ReflectSchedule { - /// The name of the schedule. - type_path: &'static str, - label: ReflectableScheduleLabel, -} - -#[derive(Reflect, Clone, Debug)] -#[reflect(opaque)] -struct ReflectableScheduleLabel(InternedScheduleLabel); - -impl Deref for ReflectableScheduleLabel { - type Target = InternedScheduleLabel; - - fn deref(&self) -> &Self::Target { - &self.0 - } -} - -impl From for ReflectableScheduleLabel { - fn from(label: InternedScheduleLabel) -> Self { - Self(label) - } -} - -impl ReflectSchedule { - /// Retrieves the name of the schedule. - pub fn type_path(&self) -> &'static str { - self.type_path - } - - /// Retrieves the short identifier of the schedule - pub fn identifier(&self) -> &'static str { - self.type_path.split("::").last().unwrap_or_default() - } - - /// Retrieves the label of the schedule - pub fn label(&self) -> &InternedScheduleLabel { - &self.label - } - - /// Creates a new reflect schedule from a schedule label - pub fn from_label(label: T) -> Self { - ReflectSchedule { - type_path: std::any::type_name::(), - label: label.intern().into(), - } - } -} +use std::{any::TypeId, collections::HashMap, sync::Arc}; #[derive(Default, Clone, Resource)] /// A Send + Sync registry of bevy schedules. @@ -230,177 +107,6 @@ impl ScheduleRegistry { } } -#[derive(Debug, Clone, Copy, Hash, PartialEq, Eq)] -/// a system set for script systems. -pub struct ScriptSystemSet(usize); - -impl ScriptSystemSet { - /// Creates a new script system set with a unique id - pub fn next() -> Self { - static CURRENT_ID: AtomicUsize = AtomicUsize::new(0); - Self(CURRENT_ID.fetch_add(1, Ordering::Relaxed)) - } -} - -impl SystemSet for ScriptSystemSet { - fn dyn_clone(&self) -> bevy::ecs::label::Box { - Box::new(*self) - } - - fn as_dyn_eq(&self) -> &dyn bevy::ecs::label::DynEq { - self - } - - fn dyn_hash(&self, mut state: &mut dyn ::core::hash::Hasher) { - self.hash(&mut state); - } -} - -/// A builder for systems living in scripts -#[derive(Reflect)] -pub struct ScriptSystemBuilder { - name: CallbackLabel, - script_id: ScriptId, - before: Vec, - after: Vec, -} - -impl ScriptSystemBuilder { - /// Creates a new script system builder - pub fn new(name: CallbackLabel, script_id: ScriptId) -> Self { - Self { - before: vec![], - after: vec![], - name, - script_id, - } - } - - /// Adds a system to run before the script system - pub fn before(&mut self, system: ReflectSystem) -> &mut Self { - self.before.push(system); - self - } - - /// Adds a system to run after the script system - pub fn after(&mut self, system: ReflectSystem) -> &mut Self { - self.after.push(system); - self - } - - /// Selects the most granual system set it can for the given system node id or None - fn get_individual_system_system_set( - node_id: NodeId, - schedule: &Schedule, - ) -> Option> { - // if let Some(system) = schedule.graph().get_system_at(node_id) { - // // this only works for normal bevy rust systems - // if let Some(system_set) = system.default_system_sets().first() { - // return Some(system_set.dyn_clone()); - // } - // } - if let Ok(systems) = schedule.systems() { - for (system_id, system) in systems { - if system_id == node_id { - return system.default_system_sets().first().cloned(); - } - } - } - - None - } - - /// Builds the system and inserts it into the given schedule - #[allow(deprecated)] - pub fn build( - self, - world: WorldGuard, - schedule: &ReflectSchedule, - ) -> Result { - world.scope_schedule(schedule, |world, schedule| { - // this is different to a normal event handler - // the system doesn't listen to events - // it immediately calls a singular script with a predefined payload - let system_name = format!("script_system_{}", &self.name); - let _system = move |world: &mut World, - system_state: &mut SystemState< - WithWorldGuard>, - >| { - let mut with_guard = system_state.get_mut(world); - - { - let (guard, handler_ctxt) = with_guard.get_mut(); - let name = self.name.clone(); - bevy::log::debug_once!("First call to script system {}", name); - match handler_ctxt.call_dynamic_label( - &name, - &self.script_id, - Entity::from_raw(0), - vec![], - guard.clone(), - ) { - Ok(_) => {} - Err(err) => { - handle_script_errors( - guard, - vec![err.with_script(self.script_id.clone())].into_iter(), - ); - } - }; - } - - system_state.apply(world); - }; - - let function_system = IntoSystem::into_system(_system.clone()).with_name(system_name); - - // dummy node id for now - let mut reflect_system = - ReflectSystem::from_system(&function_system, NodeId::System(0)); - - // this is quite important, by default systems are placed in a set defined by their TYPE, i.e. in this case - // all script systems would be the same - let set = ScriptSystemSet::next(); - let mut config = IntoSystemConfigs::into_configs(function_system.in_set(set)); - - // apply ordering - for (other, is_before) in self - .before - .iter() - .map(|b| (b, true)) - .chain(self.after.iter().map(|a| (a, false))) - { - match Self::get_individual_system_system_set(other.node_id.0, schedule) { - Some(set) => { - if is_before { - config = config.before(set); - } else { - config = config.after(set); - } - } - None => { - bevy::log::warn!( - "Could not find system set for system {:?}", - other.identifier() - ); - } - } - } - - schedule.configure_sets(set); - schedule.add_systems(config); - - schedule.initialize(world)?; - - let node_id = NodeId::System(schedule.systems_len()); - - reflect_system.node_id = ReflectNodeId(node_id); - - Ok(reflect_system) - })? - } -} - #[profiling::all_functions] /// Impls to do with dynamically querying systems and schedules impl WorldAccessGuard<'_> { @@ -422,7 +128,7 @@ impl WorldAccessGuard<'_> { })?; let mut removed_schedule = schedules - .remove(*label.label) + .remove(*label.label()) .ok_or_else(|| InteropError::missing_schedule(label.identifier()))?; let result = f(world, &mut removed_schedule); @@ -436,7 +142,7 @@ impl WorldAccessGuard<'_> { })?; assert!( - removed_schedule.label() == *label.label, + removed_schedule.label() == *label.label(), "removed schedule label doesn't match the original" ); schedules.insert(removed_schedule); @@ -449,7 +155,7 @@ impl WorldAccessGuard<'_> { pub fn systems(&self, schedule: &ReflectSchedule) -> Result, InteropError> { self.with_resource(|schedules: &Schedules| { let schedule = schedules - .get(*schedule.label) + .get(*schedule.label()) .ok_or_else(|| InteropError::missing_schedule(schedule.identifier()))?; let systems = schedule.systems()?; @@ -487,7 +193,10 @@ mod tests { use bevy::{ app::{App, Update}, - ecs::system::IntoSystem, + ecs::{ + schedule::{NodeId, Schedules}, + system::IntoSystem, + }, }; use test_utils::make_test_plugin; diff --git a/crates/bevy_mod_scripting_core/src/bindings/script_system.rs b/crates/bevy_mod_scripting_core/src/bindings/script_system.rs new file mode 100644 index 000000000..feaf42be5 --- /dev/null +++ b/crates/bevy_mod_scripting_core/src/bindings/script_system.rs @@ -0,0 +1,723 @@ +//! everything to do with dynamically added script systems + +use super::{ + access_map::ReflectAccessId, + function::{from::Val, into::IntoScript, script_function::AppScriptFunctionRegistry}, + schedule::AppScheduleRegistry, + script_value::ScriptValue, + AppReflectAllocator, ReflectBaseType, ReflectReference, ScriptQueryBuilder, ScriptQueryResult, + ScriptResourceRegistration, WorldAccessGuard, WorldGuard, +}; +use crate::{ + bindings::pretty_print::DisplayWithWorld, + context::ContextLoadingSettings, + error::{InteropError, ScriptError}, + event::CallbackLabel, + extractors::get_all_access_ids, + handler::CallbackSettings, + runtime::RuntimeContainer, + script::{ScriptId, Scripts}, + IntoScriptPluginParams, +}; +use bevy::{ + ecs::{ + archetype::{ArchetypeComponentId, ArchetypeGeneration}, + component::{ComponentId, Tick}, + entity::Entity, + query::{Access, FilteredAccess, FilteredAccessSet, QueryState}, + reflect::AppTypeRegistry, + schedule::{IntoSystemConfigs, SystemSet}, + system::{IntoSystem, System}, + world::{unsafe_world_cell::UnsafeWorldCell, World}, + }, + reflect::{OffsetAccess, ParsedPath, Reflect}, + utils::hashbrown::HashSet, +}; +use bevy_system_reflection::{ReflectSchedule, ReflectSystem}; +use std::{any::TypeId, borrow::Cow, hash::Hash, marker::PhantomData, ops::Deref}; +#[derive(Clone, Hash, PartialEq, Eq)] +/// a system set for script systems. +pub struct ScriptSystemSet(Cow<'static, str>); + +impl std::fmt::Debug for ScriptSystemSet { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.write_str("ScriptSystem(")?; + f.write_str(self.0.as_ref())?; + f.write_str(")")?; + Ok(()) + } +} + +impl ScriptSystemSet { + /// Creates a new script system set + pub fn new(id: impl Into>) -> Self { + Self(id.into()) + } +} + +impl SystemSet for ScriptSystemSet { + fn dyn_clone(&self) -> bevy::ecs::label::Box { + Box::new(self.clone()) + } + + fn as_dyn_eq(&self) -> &dyn bevy::ecs::label::DynEq { + self + } + + fn dyn_hash(&self, mut state: &mut dyn ::core::hash::Hasher) { + self.hash(&mut state); + } +} + +#[derive(Clone)] +enum ScriptSystemParamDescriptor { + Res(ScriptResourceRegistration), + EntityQuery(ScriptQueryBuilder), +} + +/// A builder for systems living in scripts +#[derive(Reflect, Clone)] +#[reflect(opaque)] +pub struct ScriptSystemBuilder { + pub(crate) name: CallbackLabel, + pub(crate) script_id: ScriptId, + before: Vec, + after: Vec, + system_params: Vec, + is_exclusive: bool, +} + +impl ScriptSystemBuilder { + /// Creates a new script system builder + pub fn new(name: CallbackLabel, script_id: ScriptId) -> Self { + Self { + before: vec![], + after: vec![], + name, + script_id, + system_params: vec![], + is_exclusive: false, + } + } + + /// Adds a component access to the system + pub fn query(&mut self, query: ScriptQueryBuilder) -> &mut Self { + self.system_params + .push(ScriptSystemParamDescriptor::EntityQuery(query)); + self + } + + /// Adds a resource access to the system + pub fn resource(&mut self, resource: ScriptResourceRegistration) -> &mut Self { + self.system_params + .push(ScriptSystemParamDescriptor::Res(resource)); + self + } + + /// Sets the system to be exclusive, i.e. it will be able to access everything but cannot be parallelized. + pub fn exclusive(&mut self, exclusive: bool) -> &mut Self { + self.is_exclusive = exclusive; + self + } + + /// Adds a system to run before the script system + pub fn before_system(&mut self, system: ReflectSystem) -> &mut Self { + self.before.push(system); + self + } + + /// Adds a system to run after the script system + pub fn after_system(&mut self, system: ReflectSystem) -> &mut Self { + self.after.push(system); + self + } + + /// Builds the system and inserts it into the given schedule + #[allow(deprecated)] + pub fn build( + self, + world: WorldGuard, + schedule: &ReflectSchedule, + ) -> Result { + world.scope_schedule(schedule, |world, schedule| { + // this is different to a normal event handler + // the system doesn't listen to events + // it immediately calls a singular script with a predefined payload + let before_systems = self.before.clone(); + let after_systems = self.after.clone(); + let system_name = self.name.to_string(); + + // let system: DynamicScriptSystem

= + // IntoSystem::<(), (), (IsDynamicScriptSystem

, ())>::into_system(self); + + // dummy node id for now + // let mut reflect_system = ReflectSystem::from_system(&system, NodeId::System(0)); + + // this is quite important, by default systems are placed in a set defined by their TYPE, i.e. in this case + // all script systems would be the same + // let set = ScriptSystemSet::next(); + let mut system_config = IntoSystemConfigs::>::into_configs(self); // apply ordering + for (other, is_before) in before_systems + .into_iter() + .map(|b| (b, true)) + .chain(after_systems.into_iter().map(|a| (a, false))) + { + for default_set in other.default_system_sets() { + if is_before { + bevy::log::info!("before {default_set:?}"); + system_config = system_config.before(*default_set); + } else { bevy::log::info!("before {default_set:?}"); + bevy::log::info!("after {default_set:?}"); + system_config = system_config.after(*default_set); + } + } + } + + schedule.add_systems(system_config); + // TODO: the node id seems to always be system.len() + // if this is slow, we can always just get the node id that way + // and let the schedule initialize itself right before it gets run + // for now I want to avoid not having the right ID as that'd be a pain + schedule.initialize(world)?; + // now find the system + let (node_id, system) = schedule + .systems()? + .find(|(_, b)| b.name().deref() == system_name) + .ok_or_else(|| InteropError::invariant("After adding the system, it was not found in the schedule, could not return a reference to it"))?; + Ok(ReflectSystem::from_system(system.as_ref(), node_id)) + })? + } +} + +struct DynamicHandlerContext<'w, P: IntoScriptPluginParams> { + scripts: &'w Scripts

, + callback_settings: &'w CallbackSettings

, + context_loading_settings: &'w ContextLoadingSettings

, + runtime_container: &'w RuntimeContainer

, +} + +impl<'w, P: IntoScriptPluginParams> DynamicHandlerContext<'w, P> { + #[allow( + clippy::expect_used, + reason = "cannot avoid panicking inside init_param due to Bevy API structure" + )] + pub fn init_param(world: &mut World, system: &mut FilteredAccessSet) { + let mut access = FilteredAccess::::matches_nothing(); + let scripts_res_id = world + .resource_id::>() + .expect("Scripts resource not found"); + let callback_settings_res_id = world + .resource_id::>() + .expect("CallbackSettings resource not found"); + let context_loading_settings_res_id = world + .resource_id::>() + .expect("ContextLoadingSettings resource not found"); + let runtime_container_res_id = world + .resource_id::>() + .expect("RuntimeContainer resource not found"); + + access.add_resource_read(scripts_res_id); + access.add_resource_read(callback_settings_res_id); + access.add_resource_read(context_loading_settings_res_id); + access.add_resource_read(runtime_container_res_id); + + system.add(access); + } + + #[allow( + clippy::expect_used, + reason = "cannot avoid panicking inside get_param due to Bevy API structure" + )] + pub fn get_param(system: &UnsafeWorldCell<'w>) -> Self { + unsafe { + Self { + scripts: system.get_resource().expect("Scripts resource not found"), + callback_settings: system + .get_resource() + .expect("CallbackSettings resource not found"), + context_loading_settings: system + .get_resource() + .expect("ContextLoadingSettings resource not found"), + runtime_container: system + .get_resource() + .expect("RuntimeContainer resource not found"), + } + } + } + + /// Call a dynamic label on a script + pub fn call_dynamic_label( + &self, + label: &CallbackLabel, + script_id: &ScriptId, + entity: Entity, + payload: Vec, + guard: WorldGuard<'_>, + ) -> Result { + // find script + let script = match self.scripts.scripts.get(script_id) { + Some(script) => script, + None => return Err(InteropError::missing_script(script_id.clone()).into()), + }; + + // call the script + let handler = self.callback_settings.callback_handler; + let pre_handling_initializers = &self + .context_loading_settings + .context_pre_handling_initializers; + let runtime = &self.runtime_container.runtime; + + let mut context = script.context.lock(); + + CallbackSettings::

::call( + handler, + payload, + entity, + script_id, + label, + &mut context, + pre_handling_initializers, + runtime, + guard, + ) + } +} + +/// TODO: inline world guard into the system state, we should be able to re-use it +struct ScriptSystemState { + type_registry: AppTypeRegistry, + function_registry: AppScriptFunctionRegistry, + schedule_registry: AppScheduleRegistry, + allocator: AppReflectAllocator, + subset: HashSet, + callback_label: CallbackLabel, + system_params: Vec, +} + +/// Equivalent of [`SystemParam`] but for dynamic systems, these are the kinds of things +/// that scripts can ask for access to and get passed in through dynamic script systems. +pub enum ScriptSystemParam { + /// An exclusive resource access + Res { + /// The component ID of the resource + component_id: ComponentId, + /// The type ID of the resource + type_id: TypeId, + }, + /// A query which returns entities + /// Boxed to reduce stack size + EntityQuery { + /// The internal state of the query + query: Box>, + /// the components in correct order describing the necessary references + components: Vec<(ComponentId, TypeId)>, + }, +} + +/// A system specified, created, and added by a script +pub struct DynamicScriptSystem { + name: Cow<'static, str>, + exclusive: bool, + /// The set of component accesses for this system. This is used to determine + /// - soundness issues (e.g. multiple [`SystemParam`]s mutably accessing the same component) + /// - ambiguities in the schedule (e.g. two systems that have some sort of conflicting access) + pub(crate) component_access_set: FilteredAccessSet, + /// This [`Access`] is used to determine which systems can run in parallel with each other + /// in the multithreaded executor. + /// + /// We use a [`ArchetypeComponentId`] as it is more precise than just checking [`ComponentId`]: + /// for example if you have one system with `Query<&mut T, With>` and one system with `Query<&mut T, With>` + /// they conflict if you just look at the [`ComponentId`] of `T`; but if there are no archetypes with + /// both `A`, `B` and `T` then in practice there's no risk of conflict. By using [`ArchetypeComponentId`] + /// we can be more precise because we can check if the existing archetypes of the [`World`] + /// cause a conflict + pub(crate) archetype_component_access: Access, + pub(crate) last_run: Tick, + target_script: ScriptId, + archetype_generation: ArchetypeGeneration, + system_param_descriptors: Vec, + state: Option, + _marker: std::marker::PhantomData P>, +} + +/// A marker type distinguishing between vanilla and script system types +pub struct IsDynamicScriptSystem

(PhantomData P>); + +impl IntoSystem<(), (), IsDynamicScriptSystem

> + for ScriptSystemBuilder +{ + type System = DynamicScriptSystem

; + + fn into_system(builder: Self) -> Self::System { + Self::System { + name: builder.name.to_string().into(), + exclusive: builder.is_exclusive, + archetype_generation: ArchetypeGeneration::initial(), + system_param_descriptors: builder.system_params, + last_run: Default::default(), + target_script: builder.script_id, + state: None, + component_access_set: Default::default(), + archetype_component_access: Default::default(), + _marker: Default::default(), + } + } +} + +impl System for DynamicScriptSystem

{ + type In = (); + + type Out = (); + + fn name(&self) -> std::borrow::Cow<'static, str> { + self.name.clone() + } + + fn component_access(&self) -> &bevy::ecs::query::Access { + self.component_access_set.combined_access() + } + + fn archetype_component_access( + &self, + ) -> &bevy::ecs::query::Access { + &self.archetype_component_access + } + + fn is_send(&self) -> bool { + !self.is_exclusive() + } + + fn is_exclusive(&self) -> bool { + self.exclusive + } + + fn has_deferred(&self) -> bool { + false + } + + unsafe fn run_unsafe( + &mut self, + _input: bevy::ecs::system::SystemIn<'_, Self>, + world: bevy::ecs::world::unsafe_world_cell::UnsafeWorldCell, + ) -> Self::Out { + let _change_tick = world.increment_change_tick(); + + #[allow( + clippy::panic, + reason = "cannot avoid panicking inside run_unsafe due to Bevy API structure" + )] + let state = match &mut self.state { + Some(state) => state, + None => panic!("System state not initialized!"), + }; + + let mut payload = Vec::with_capacity(state.system_params.len()); + let guard = if self.exclusive { + // safety: we are an exclusive system, therefore the cell allows us to do this + let world = unsafe { world.world_mut() }; + WorldAccessGuard::new_exclusive(world) + } else { + WorldAccessGuard::new_non_exclusive( + world, + state.subset.clone(), + state.type_registry.clone(), + state.allocator.clone(), + state.function_registry.clone(), + state.schedule_registry.clone(), + ) + }; + + // TODO: cache references which don't change once we have benchmarks + for param in &mut state.system_params { + match param { + ScriptSystemParam::Res { + component_id, + type_id, + } => { + let res_ref = ReflectReference { + base: super::ReflectBaseType { + type_id: *type_id, + base_id: super::ReflectBase::Resource(*component_id), + }, + reflect_path: ParsedPath::from(Vec::::default()), + }; + payload.push(res_ref.into_script_inline_error(guard.clone())); + } + ScriptSystemParam::EntityQuery { query, components } => { + // TODO: is this the right way to use this world cell for queries? + let entities = query.iter_unchecked(world).collect::>(); + let results = entities + .into_iter() + .map(|entity| { + Val(ScriptQueryResult { + entity, + components: components + .iter() + .map(|(component_id, type_id)| ReflectReference { + base: ReflectBaseType { + type_id: *type_id, + base_id: super::ReflectBase::Component( + entity, + *component_id, + ), + }, + reflect_path: Vec::::default().into(), + }) + .collect(), + }) + }) + .collect::>(); + + payload.push(results.into_script_inline_error(guard.clone())) + } + } + } + + // now that we have everything ready, we need to run the callback on the targetted scripts + // let's start with just calling the one targetted script + + let handler_ctxt = DynamicHandlerContext::

::get_param(&world); + + let result = handler_ctxt.call_dynamic_label( + &state.callback_label, + &self.target_script, + Entity::from_raw(0), + payload, + guard.clone(), + ); + + // TODO: emit error events via commands, maybe accumulate in state instead and use apply + match result { + Ok(_) => {} + Err(err) => { + bevy::log::error!( + "Error in dynamic script system `{}`: {}", + self.name, + err.display_with_world(guard) + ) + } + } + } + + fn initialize(&mut self, world: &mut bevy::ecs::world::World) { + // we need to register all the: + // - resources, simple just need the component ID's + // - queries, more difficult the queries need to be built, and archetype access registered on top of component access + + // start with resources + let mut subset = HashSet::default(); + let mut system_params = Vec::with_capacity(self.system_param_descriptors.len()); + for param in &self.system_param_descriptors { + match param { + ScriptSystemParamDescriptor::Res(res) => { + let component_id = res.resource_id; + let type_id = res.type_registration().type_id(); + + let system_param = ScriptSystemParam::Res { + component_id, + type_id, + }; + system_params.push(system_param); + + let mut access = FilteredAccess::::matches_nothing(); + + access.add_resource_write(component_id); + self.component_access_set.add(access); + let raid = ReflectAccessId::for_component_id(component_id); + #[allow( + clippy::panic, + reason = "WIP, to be dealt with in validate params better, but panic will still remain" + )] + if subset.contains(&raid) { + panic!("Duplicate resource access in system: {:?}.", raid); + } + subset.insert(raid); + } + ScriptSystemParamDescriptor::EntityQuery(query) => { + let components: Vec<_> = query + .components + .iter() + .map(|c| (c.component_id, c.type_registration().type_id())) + .collect(); + let query = query.as_query_state::(world); + + // Safety: we are not removing + self.component_access_set + .add(query.component_access().clone()); + + let new_raids = get_all_access_ids(query.component_access().access()) + .into_iter() + .map(|(a, _)| a) + .collect::>(); + + #[allow( + clippy::panic, + reason = "WIP, to be dealt with in validate params better, but panic will still remain" + )] + if !subset.is_disjoint(&new_raids) { + panic!("Non-disjoint query in dynamic system parameters."); + } + + system_params.push(ScriptSystemParam::EntityQuery { + query: query.into(), + components, + }); + subset.extend(new_raids); + } + } + } + + // TODO: access to internal resources, i.e. handler state + DynamicHandlerContext::

::init_param(world, &mut self.component_access_set); + + self.state = Some(ScriptSystemState { + type_registry: world.get_resource_or_init::().clone(), + function_registry: world + .get_resource_or_init::() + .clone(), + schedule_registry: world.get_resource_or_init::().clone(), + allocator: world.get_resource_or_init::().clone(), + subset, + callback_label: self.name.to_string().into(), + system_params, + }) + } + + fn update_archetype_component_access( + &mut self, + world: bevy::ecs::world::unsafe_world_cell::UnsafeWorldCell, + ) { + let archetypes = world.archetypes(); + + let old_generation = + std::mem::replace(&mut self.archetype_generation, archetypes.generation()); + + if let Some(state) = &mut self.state { + for archetype in &archetypes[old_generation..] { + for param in &mut state.system_params { + if let ScriptSystemParam::EntityQuery { query, .. } = param { + // SAFETY: The assertion above ensures that the param_state was initialized from `world`. + unsafe { + query.new_archetype(archetype, &mut self.archetype_component_access) + }; + } + } + } + } + } + + fn check_change_tick(&mut self, change_tick: bevy::ecs::component::Tick) { + let last_run = &mut self.last_run; + let this_run = change_tick; + + let age = this_run.get().wrapping_sub(last_run.get()); + if age > Tick::MAX.get() { + *last_run = Tick::new(this_run.get().wrapping_sub(Tick::MAX.get())); + } + } + + fn get_last_run(&self) -> bevy::ecs::component::Tick { + self.last_run + } + + fn set_last_run(&mut self, last_run: bevy::ecs::component::Tick) { + self.last_run = last_run; + } + + fn apply_deferred(&mut self, _world: &mut World) {} + + fn queue_deferred(&mut self, _world: bevy::ecs::world::DeferredWorld) {} + + unsafe fn validate_param_unsafe( + &mut self, + _world: bevy::ecs::world::unsafe_world_cell::UnsafeWorldCell, + ) -> bool { + true + } + + fn default_system_sets(&self) -> Vec { + vec![ScriptSystemSet::new(self.name.clone()).intern()] + } + + fn type_id(&self) -> TypeId { + TypeId::of::() + } + + fn validate_param(&mut self, world: &World) -> bool { + let world_cell = world.as_unsafe_world_cell_readonly(); + self.update_archetype_component_access(world_cell); + // SAFETY: + // - We have exclusive access to the entire world. + // - `update_archetype_component_access` has been called. + unsafe { self.validate_param_unsafe(world_cell) } + } +} + +#[cfg(test)] +mod test { + use bevy::{ + app::{App, MainScheduleOrder, Update}, + asset::AssetPlugin, + diagnostic::DiagnosticsPlugin, + ecs::schedule::{ScheduleLabel, Schedules}, + }; + use test_utils::make_test_plugin; + + use super::*; + + make_test_plugin!(crate); + + fn test_system_rust(_world: &mut World) {} + + #[test] + fn test_script_system_with_existing_system_dependency_can_execute() { + let mut app = App::new(); + + #[derive(ScheduleLabel, Clone, Debug, Hash, PartialEq, Eq)] + struct TestSchedule; + + app.add_plugins(( + AssetPlugin::default(), + DiagnosticsPlugin, + TestPlugin::default(), + )); + app.init_schedule(TestSchedule); + let mut main_schedule_order = app.world_mut().resource_mut::(); + main_schedule_order.insert_after(Update, TestSchedule); + app.add_systems(TestSchedule, test_system_rust); + + // run the app once + app.finish(); + app.cleanup(); + app.update(); + + // find existing rust system + let test_system = app + .world_mut() + .resource_scope::(|_, schedules| { + let (node_id, system) = schedules + .get(TestSchedule) + .unwrap() + .systems() + .unwrap() + .find(|(_, system)| system.name().contains("test_system_rust")) + .unwrap(); + + ReflectSystem::from_system(system.as_ref(), node_id) + }); + + // now dynamically add script system via builder + let mut builder = ScriptSystemBuilder::new("test".into(), "empty_script".into()); + builder.before_system(test_system); + + let _ = builder + .build::( + WorldAccessGuard::new_exclusive(app.world_mut()), + &ReflectSchedule::from_label(TestSchedule), + ) + .unwrap(); + + // now re-run app, expect no panicks + app.update(); + } +} diff --git a/crates/bevy_mod_scripting_core/src/bindings/world.rs b/crates/bevy_mod_scripting_core/src/bindings/world.rs index 9f9938d9d..f8bfe8219 100644 --- a/crates/bevy_mod_scripting_core/src/bindings/world.rs +++ b/crates/bevy_mod_scripting_core/src/bindings/world.rs @@ -6,13 +6,16 @@ //! we need wrapper types which have owned and ref variants. use super::{ - access_map::{AccessCount, AccessMap, ReflectAccessId}, + access_map::{ + AccessCount, AccessMapKey, AnyAccessMap, DynamicSystemMeta, ReflectAccessId, + ReflectAccessKind, SubsetAccessMap, + }, function::{ namespace::Namespace, script_function::{AppScriptFunctionRegistry, DynamicScriptFunction, FunctionCallContext}, }, pretty_print::DisplayWithWorld, - schedule::{AppScheduleRegistry, ReflectSchedule}, + schedule::AppScheduleRegistry, script_value::ScriptValue, AppReflectAllocator, ReflectBase, ReflectBaseType, ReflectReference, ScriptComponentRegistration, ScriptResourceRegistration, ScriptTypeRegistration, @@ -38,6 +41,7 @@ use bevy::{ DynamicVariant, ParsedPath, PartialReflect, TypeRegistryArc, }, }; +use bevy_system_reflection::ReflectSchedule; use std::{ any::TypeId, borrow::Cow, @@ -62,13 +66,12 @@ pub struct WorldAccessGuard<'w> { /// stored separate from the contents of the guard invalid: Rc, } - /// Used to decrease the stack size of [`WorldAccessGuard`] pub(crate) struct WorldAccessGuardInner<'w> { /// Safety: cannot be used unless the scope depth is less than the max valid scope cell: UnsafeWorldCell<'w>, // TODO: this is fairly hefty, explore sparse sets, bit fields etc - pub(crate) accesses: AccessMap, + pub(crate) accesses: AnyAccessMap, /// Cached for convenience, since we need it for most operations, means we don't need to lock the type registry every time type_registry: TypeRegistryArc, /// The script allocator for the world @@ -124,7 +127,7 @@ impl<'w> WorldAccessGuard<'w> { world: &'w mut World, f: impl FnOnce(WorldGuard<'static>) -> O, ) -> O { - let guard = WorldAccessGuard::new(world); + let guard = WorldAccessGuard::new_exclusive(world); // safety: we invalidate the guard after the closure is called, meaning the world cannot be accessed at all after the 'w lifetime ends let static_guard: WorldAccessGuard<'static> = unsafe { std::mem::transmute(guard) }; let o = f(static_guard.clone()); @@ -147,6 +150,40 @@ impl<'w> WorldAccessGuard<'w> { o } + /// Creates a new [`WorldAccessGuard`] from a possibly non-exclusive access to the world. + /// + /// It requires specyfing the exact accesses that are allowed to be given out by the guard. + /// Those accesses need to be safe to be given out to the script, as the guard will assume that it is safe to give them out in any way. + /// + /// # Safety + /// - The caller must ensure that the accesses in subset are not aliased by any other access + /// - If an access is allowed in this subset, but alised by someone else, + /// either by being converted to mutable or non mutable reference, this guard will be unsafe. + pub unsafe fn new_non_exclusive( + world: UnsafeWorldCell<'w>, + subset: impl IntoIterator, + type_registry: AppTypeRegistry, + allocator: AppReflectAllocator, + function_registry: AppScriptFunctionRegistry, + schedule_registry: AppScheduleRegistry, + ) -> Self { + Self { + inner: Rc::new(WorldAccessGuardInner { + cell: world, + accesses: AnyAccessMap::SubsetAccessMap(SubsetAccessMap::new( + subset, + // allocations live beyond the world, and can be safely accessed + |id| ReflectAccessId::from_index(id).kind == ReflectAccessKind::Allocation, + )), + type_registry: type_registry.0, + allocator, + function_registry, + schedule_registry, + }), + invalid: Rc::new(false.into()), + } + } + /// Creates a new [`WorldAccessGuard`] for the given mutable borrow of the world. /// /// Creating a guard requires that some resources exist in the world, namely: @@ -155,7 +192,7 @@ impl<'w> WorldAccessGuard<'w> { /// - [`AppScriptFunctionRegistry`] /// /// If these resources do not exist, they will be initialized. - pub fn new(world: &'w mut World) -> Self { + pub fn new_exclusive(world: &'w mut World) -> Self { let type_registry = world.get_resource_or_init::().0.clone(); let allocator = world.get_resource_or_init::().clone(); @@ -168,7 +205,7 @@ impl<'w> WorldAccessGuard<'w> { Self { inner: Rc::new(WorldAccessGuardInner { cell: world.as_unsafe_world_cell(), - accesses: Default::default(), + accesses: AnyAccessMap::UnlimitedAccessMap(Default::default()), allocator, type_registry, function_registry, @@ -305,7 +342,7 @@ impl<'w> WorldAccessGuard<'w> { f: F, ) -> Result { with_global_access!( - self.inner.accesses, + &self.inner.accesses, "Could not claim exclusive world access", { // safety: we have global access for the duration of the closure @@ -328,7 +365,7 @@ impl<'w> WorldAccessGuard<'w> { let access_id = ReflectAccessId::for_resource::(&cell)?; with_access_read!( - self.inner.accesses, + &self.inner.accesses, access_id, format!("Could not access resource: {}", std::any::type_name::()), { @@ -356,7 +393,7 @@ impl<'w> WorldAccessGuard<'w> { let cell = self.as_unsafe_world_cell()?; let access_id = ReflectAccessId::for_resource::(&cell)?; with_access_write!( - self.inner.accesses, + &self.inner.accesses, access_id, format!("Could not access resource: {}", std::any::type_name::()), { @@ -381,7 +418,7 @@ impl<'w> WorldAccessGuard<'w> { let cell = self.as_unsafe_world_cell()?; let access_id = ReflectAccessId::for_component::(&cell)?; with_access_read!( - self.inner.accesses, + &self.inner.accesses, access_id, format!("Could not access component: {}", std::any::type_name::()), { @@ -401,7 +438,7 @@ impl<'w> WorldAccessGuard<'w> { let access_id = ReflectAccessId::for_component::(&cell)?; with_access_write!( - self.inner.accesses, + &self.inner.accesses, access_id, format!("Could not access component: {}", std::any::type_name::()), { @@ -883,7 +920,7 @@ impl WorldAccessGuard<'_> { ) })?; - with_global_access!(self.inner.accesses, "Could not insert element", { + with_global_access!(&self.inner.accesses, "Could not insert element", { let cell = self.as_unsafe_world_cell()?; let type_registry = self.type_registry(); let type_registry = type_registry.read(); @@ -1218,7 +1255,7 @@ mod test { #[test] fn test_construct_struct() { let mut world = setup_world(|_, _| {}); - let world = WorldAccessGuard::new(&mut world); + let world = WorldAccessGuard::new_exclusive(&mut world); let registry = world.type_registry(); let registry = registry.read(); @@ -1237,7 +1274,7 @@ mod test { #[test] fn test_construct_tuple_struct() { let mut world = setup_world(|_, _| {}); - let world = WorldAccessGuard::new(&mut world); + let world = WorldAccessGuard::new_exclusive(&mut world); let registry = world.type_registry(); let registry = registry.read(); @@ -1280,7 +1317,7 @@ mod test { }); ::get_type_registration(); - let world = WorldAccessGuard::new(&mut world); + let world = WorldAccessGuard::new_exclusive(&mut world); let registry = world.type_registry(); let registry = registry.read(); @@ -1315,7 +1352,7 @@ mod test { #[test] fn test_construct_enum() { let mut world = setup_world(|_, _| {}); - let world = WorldAccessGuard::new(&mut world); + let world = WorldAccessGuard::new_exclusive(&mut world); let registry = world.type_registry(); let registry = registry.read(); @@ -1364,7 +1401,7 @@ mod test { #[test] fn test_scoped_handle_invalidate_doesnt_invalidate_parent() { let mut world = setup_world(|_, _| {}); - let world = WorldAccessGuard::new(&mut world); + let world = WorldAccessGuard::new_exclusive(&mut world); let scoped_world = world.scope(); // can use scoped & normal worlds @@ -1384,7 +1421,7 @@ mod test { #[test] fn with_existing_static_guard_does_not_invalidate_original() { let mut world = setup_world(|_, _| {}); - let world = WorldAccessGuard::new(&mut world); + let world = WorldAccessGuard::new_exclusive(&mut world); let mut sneaky_clone = None; WorldAccessGuard::with_existing_static_guard(world.clone(), |g| { @@ -1402,7 +1439,7 @@ mod test { #[test] fn test_with_access_scope_success() { let mut world = setup_world(|_, _| {}); - let guard = WorldAccessGuard::new(&mut world); + let guard = WorldAccessGuard::new_exclusive(&mut world); // within the access scope, no extra accesses are claimed let result = unsafe { guard.with_access_scope(|| 100) }; diff --git a/crates/bevy_mod_scripting_core/src/error.rs b/crates/bevy_mod_scripting_core/src/error.rs index f37d6f8fb..1c63c64bd 100644 --- a/crates/bevy_mod_scripting_core/src/error.rs +++ b/crates/bevy_mod_scripting_core/src/error.rs @@ -314,8 +314,9 @@ impl Display for MissingResourceError { impl std::error::Error for MissingResourceError {} #[derive(Debug, Clone, PartialEq, Reflect)] +#[reflect(opaque)] /// An error thrown when interoperating with scripting languages. -pub struct InteropError(#[reflect(ignore)] Arc); +pub struct InteropError(Arc); impl std::error::Error for InteropError {} @@ -1612,7 +1613,7 @@ mod test { let script_function_registry = AppScriptFunctionRegistry::default(); world.insert_resource(script_function_registry); - let world_guard = WorldGuard::new(&mut world); + let world_guard = WorldGuard::new_exclusive(&mut world); assert_eq!( error.display_with_world(world_guard), format!( diff --git a/crates/bevy_mod_scripting_core/src/extractors.rs b/crates/bevy_mod_scripting_core/src/extractors.rs index b00a6794d..d105cca7b 100644 --- a/crates/bevy_mod_scripting_core/src/extractors.rs +++ b/crates/bevy_mod_scripting_core/src/extractors.rs @@ -314,8 +314,8 @@ unsafe impl SystemParam for WithWorldGuard<'_, '_, T> { world: bevy::ecs::world::unsafe_world_cell::UnsafeWorldCell<'world>, change_tick: bevy::ecs::component::Tick, ) -> Self::Item<'world, 'state> { - // read components being read in the state, and lock those accesses in the new world access guard - let guard = WorldAccessGuard::new(world.world_mut()); + // create a guard which can only access the resources/components specified by the system. + let guard = WorldAccessGuard::new_exclusive(world.world_mut()); #[allow( clippy::panic, @@ -378,7 +378,7 @@ fn individual_conflicts(conflicts: AccessConflicts) -> FixedBitSet { } } -fn get_all_access_ids(access: &Access) -> Vec<(ReflectAccessId, bool)> { +pub(crate) fn get_all_access_ids(access: &Access) -> Vec<(ReflectAccessId, bool)> { let mut access_all_read = Access::::default(); access_all_read.read_all(); diff --git a/crates/bevy_mod_scripting_functions/Cargo.toml b/crates/bevy_mod_scripting_functions/Cargo.toml index ecd504daa..d275b0064 100644 --- a/crates/bevy_mod_scripting_functions/Cargo.toml +++ b/crates/bevy_mod_scripting_functions/Cargo.toml @@ -37,6 +37,7 @@ bevy_mod_scripting_core = { workspace = true } bevy_mod_scripting_derive = { workspace = true } bevy_mod_scripting_lua = { path = "../languages/bevy_mod_scripting_lua", optional = true, version = "0.9.11" } bevy_mod_scripting_rhai = { path = "../languages/bevy_mod_scripting_rhai", optional = true, version = "0.9.11" } +bevy_system_reflection = { path = "../bevy_system_reflection", version = "0.1.0" } [lints] workspace = true diff --git a/crates/bevy_mod_scripting_functions/src/core.rs b/crates/bevy_mod_scripting_functions/src/core.rs index f2814f73b..cd3bbffbb 100644 --- a/crates/bevy_mod_scripting_functions/src/core.rs +++ b/crates/bevy_mod_scripting_functions/src/core.rs @@ -8,12 +8,13 @@ use bevy_mod_scripting_core::{ function::{ from::Union, namespace::GlobalNamespace, script_function::DynamicScriptFunctionMut, }, - schedule::{ReflectSchedule, ReflectSystem, ScriptSystemBuilder}, + script_system::ScriptSystemBuilder, }, docgen::info::FunctionInfo, *, }; use bevy_mod_scripting_derive::script_bindings; +use bevy_system_reflection::{ReflectSchedule, ReflectSystem}; use bindings::{ function::{ from::{Ref, Val}, @@ -29,7 +30,6 @@ use bindings::{ }; use error::InteropError; use reflection_extensions::{PartialReflectExt, TypeIdExtensions}; - pub fn register_bevy_bindings(app: &mut App) { #[cfg(feature = "bevy_bindings")] app.add_plugins(crate::bevy_bindings::LuaBevyScriptingPlugin); @@ -762,6 +762,33 @@ impl ReflectSchedule { .into_iter() .find_map(|s| (s.identifier() == name || s.path() == name).then_some(s.into()))) } + + /// Renders the schedule as a dot graph string. + /// + /// Useful for debugging scheduling. + /// + /// Arguments: + /// * `ctxt`: The function call context + /// * `self_`: The schedule to render. + /// Returns: + /// * `dot`: The dot graph string. + fn render_dot( + ctxt: FunctionCallContext, + self_: Ref, + ) -> Result { + profiling::function_scope!("render_dot"); + let world = ctxt.world()?; + world.with_resource(|schedules: &Schedules| { + let schedule = schedules + .get(*self_.label()) + .ok_or_else(|| InteropError::missing_schedule(self_.identifier()))?; + let mut graph = bevy_system_reflection::schedule_to_reflect_graph(schedule); + graph.absorb_type_system_sets(); + graph.sort(); + let graph = bevy_system_reflection::reflect_graph_to_dot(graph); + Ok(graph) + })? + } } #[script_bindings( @@ -797,6 +824,45 @@ impl ReflectSystem { name = "script_system_builder_functions" )] impl ScriptSystemBuilder { + fn query( + self_: Val, + query: Val, + ) -> Result, InteropError> { + profiling::function_scope!("query"); + let mut builder = self_.into_inner(); + builder.query(query.into_inner()); + Ok(builder.into()) + } + + /// Requests the system have access to the given resource. The resource will be added to the + /// list of arguments of the callback in the order they're provided. + /// Arguments: + /// * `self_`: The system builder to add the resource to. + /// * `resource`: The resource to add. + /// Returns: + /// * `builder`: The system builder with the resource added. + fn resource( + self_: Val, + resource: Val, + ) -> Val { + profiling::function_scope!("resource"); + let mut builder = self_.into_inner(); + builder.resource(resource.into_inner()); + builder.into() + } + + /// Specifies the system is to run exclusively, meaning it can access anything, but will not run in parallel with other systems. + /// Arguments: + /// * `self_`: The system builder to make exclusive. + /// Returns: + /// * `builder`: The system builder that is now exclusive. + fn exclusive(self_: Val) -> Val { + profiling::function_scope!("exclusive"); + let mut builder = self_.into_inner(); + builder.exclusive(true); + builder.into() + } + /// Specifies the system is to run *after* the given system /// /// Note: this is an experimental feature, and the ordering might not work correctly for script initialized systems @@ -812,7 +878,7 @@ impl ScriptSystemBuilder { ) -> Val { profiling::function_scope!("after"); let mut builder = self_.into_inner(); - builder.after(system.into_inner()); + builder.after_system(system.into_inner()); Val(builder) } @@ -831,7 +897,7 @@ impl ScriptSystemBuilder { ) -> Val { profiling::function_scope!("before"); let mut builder = self_.into_inner(); - builder.before(system.into_inner()); + builder.before_system(system.into_inner()); Val(builder) } } diff --git a/crates/bevy_system_reflection/Cargo.toml b/crates/bevy_system_reflection/Cargo.toml new file mode 100644 index 000000000..510d3b986 --- /dev/null +++ b/crates/bevy_system_reflection/Cargo.toml @@ -0,0 +1,16 @@ +[package] +name = "bevy_system_reflection" +version = "0.1.0" +edition = "2024" + +[dependencies] +bevy = { workspace = true, default-features = false } +dot-writer = "0.1.4" +petgraph = "*" + + +[dev-dependencies] +pretty_assertions = "1.4" +manifest-dir-macros = "0.1.18" +[lints] +workspace = true diff --git a/crates/bevy_system_reflection/src/lib.rs b/crates/bevy_system_reflection/src/lib.rs new file mode 100644 index 000000000..bf7cf158d --- /dev/null +++ b/crates/bevy_system_reflection/src/lib.rs @@ -0,0 +1,544 @@ +//! A visualiser for bevy system schedules, as well as utilities for querying them via reflection +use std::ops::Deref; +use std::{any::TypeId, borrow::Cow}; + +use bevy::ecs::schedule::{ + InternedScheduleLabel, InternedSystemSet, NodeId, Schedule, ScheduleLabel, SystemSet, +}; +use bevy::ecs::system::{System, SystemInput}; +use bevy::reflect::Reflect; +use bevy::utils::hashbrown::{HashMap, HashSet}; +use dot_writer::{Attributes, DotWriter}; + +#[derive(Reflect, Debug, Clone)] +#[reflect(opaque)] +/// A reflectable system. +pub struct ReflectSystem { + pub(crate) name: Cow<'static, str>, + pub(crate) type_id: TypeId, + pub(crate) node_id: ReflectNodeId, + pub(crate) default_system_sets: Vec, +} + +impl ReflectSystem { + /// Retrieves the name of the system. + pub fn name(&self) -> &str { + self.name.as_ref() + } + + /// Retrieves the type id of the system. + pub fn type_id(&self) -> TypeId { + self.type_id + } + + /// Retrieves the node id of the system. + pub fn node_id(&self) -> NodeId { + self.node_id.0 + } + + /// Retrieves the default system sets of the system. + pub fn default_system_sets(&self) -> &[InternedSystemSet] { + &self.default_system_sets + } + /// Creates a reflect system from a system specification + pub fn from_system( + system: &dyn System, + node_id: NodeId, + ) -> Self { + ReflectSystem { + name: system.name().clone(), + type_id: system.type_id(), + node_id: ReflectNodeId(node_id), + default_system_sets: system.default_system_sets(), + } + } + + /// gets the short identifier of the system, i.e. just the function name + pub fn identifier(&self) -> &str { + // if it contains generics it might contain more than + if self.name.contains("<") { + self.name + .split("<") + .next() + .unwrap_or_default() + .split("::") + .last() + .unwrap_or_default() + } else { + self.name.split("::").last().unwrap_or_default() + } + } + + /// gets the path of the system, i.e. the fully qualified function name + pub fn path(&self) -> &str { + self.name.as_ref() + } +} + +#[derive(Reflect, Clone, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)] +#[reflect(opaque)] +pub(crate) struct ReflectNodeId(pub NodeId); + +/// A reflectable schedule. +#[derive(Reflect, Clone, Debug)] +pub struct ReflectSchedule { + /// The name of the schedule. + type_path: &'static str, + label: ReflectableScheduleLabel, +} + +#[derive(Reflect, Clone, Debug)] +#[reflect(opaque)] +struct ReflectableScheduleLabel(InternedScheduleLabel); + +impl Deref for ReflectableScheduleLabel { + type Target = InternedScheduleLabel; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl From for ReflectableScheduleLabel { + fn from(label: InternedScheduleLabel) -> Self { + Self(label) + } +} + +impl ReflectSchedule { + /// Retrieves the name of the schedule. + pub fn type_path(&self) -> &'static str { + self.type_path + } + + /// Retrieves the short identifier of the schedule + pub fn identifier(&self) -> &'static str { + self.type_path.split("::").last().unwrap_or_default() + } + + /// Retrieves the label of the schedule + pub fn label(&self) -> &InternedScheduleLabel { + &self.label + } + + /// Creates a new reflect schedule from a schedule label + pub fn from_label(label: T) -> Self { + ReflectSchedule { + type_path: std::any::type_name::(), + label: label.intern().into(), + } + } +} + +#[derive(Reflect)] +/// A reflectable system set. +pub struct ReflectSystemSet { + /// The node id of the system set. + node_id: ReflectNodeId, + + /// The debug print of the system set + debug: String, + + /// If this is a typed system set, the type id + type_id: Option, +} + +impl ReflectSystemSet { + /// Creates a reflect system set from a system set + pub fn from_set(set: &dyn SystemSet, node_id: NodeId) -> Self { + ReflectSystemSet { + node_id: ReflectNodeId(node_id), + debug: format!("{:?}", set), + type_id: set.system_type(), + } + } +} + +/// Renders a schedule to a dot graph using the optimized schedule. +pub fn schedule_to_dot_graph(schedule: &Schedule) -> String { + let graph = schedule_to_reflect_graph(schedule); + reflect_graph_to_dot(graph) +} + +/// Renders a reflectable system graph to a dot graph +pub fn reflect_graph_to_dot(graph: ReflectSystemGraph) -> String { + // create a dot graph with: + // - hierarchy represented by red composition arrows + // - dependencies represented by blue arrows + let mut output_bytes = Vec::new(); + let mut writer = DotWriter::from(&mut output_bytes); + { + let mut writer = writer.digraph(); + + let mut node_id_map = HashMap::new(); + for node in graph.nodes { + match node { + ReflectSystemGraphNode::System(reflect_system) => { + let mut node = writer.node_auto(); + + node.set_label(&reflect_system.name); + node_id_map.insert(reflect_system.node_id, node.id()); + } + ReflectSystemGraphNode::SystemSet(reflect_system_set) => { + let name = if reflect_system_set.type_id.is_some() { + // special sets, that pollute the graph, summize them as "system type set", each system gets one + "SystemTypeSet".to_owned() + } else { + format!("SystemSet {}", reflect_system_set.debug) + }; + + let mut node = writer.node_auto(); + node.set_label(&name); + node_id_map.insert(reflect_system_set.node_id, node.id()); + } + } + } + + // go through hierarchy edges + for edge in graph.hierarchy { + let from = node_id_map.get(&edge.from).cloned().unwrap_or_else(|| { + let mut unknown = writer.node_auto(); + unknown.set_label(&format!("unknown_parent {:?}", edge.from.0)); + let id = unknown.id(); + node_id_map.insert(edge.from, id.clone()); + id + }); + let to = node_id_map.get(&edge.to).cloned().unwrap_or_else(|| { + let mut unknown = writer.node_auto(); + unknown.set_label(&format!("unknown_child {:?}", edge.to.0)); + let id = unknown.id(); + node_id_map.insert(edge.to, id.clone()); + id + }); + writer + .edge(to, from) + .attributes() + .set_color(dot_writer::Color::Red) + .set_label("child of") + .set_arrow_head(dot_writer::ArrowType::Diamond); + } + // go through dependency edges + for edge in graph.dependencies { + let from = node_id_map.get(&edge.from).cloned().unwrap_or_else(|| { + let mut unknown = writer.node_auto(); + unknown.set_label(&format!("unknown_dependant {:?}", edge.from.0)); + let id = unknown.id(); + node_id_map.insert(edge.from, id.clone()); + id + }); + let to = node_id_map.get(&edge.to).cloned().unwrap_or_else(|| { + let mut unknown = writer.node_auto(); + unknown.set_label(&format!("unknown_dependency {:?}", edge.to.0)); + let id = unknown.id(); + node_id_map.insert(edge.to, id.clone()); + id + }); + writer + .edge(from, to) + .attributes() + .set_color(dot_writer::Color::Blue) + .set_label("runs before") + .set_arrow_head(dot_writer::ArrowType::Normal); + } + } + + String::from_utf8(output_bytes).unwrap_or_default() +} + +/// Converts a schedule to a reflectable system graph +pub fn schedule_to_reflect_graph(schedule: &Schedule) -> ReflectSystemGraph { + let graph = schedule.graph(); + let hierarchy = graph.hierarchy().graph(); + let dependency = graph.dependency().graph(); + + let mut nodes = Vec::new(); + let mut covered_nodes = HashSet::new(); + for (node_id, system_set, _) in graph.system_sets() { + covered_nodes.insert(node_id); + nodes.push(ReflectSystemGraphNode::SystemSet( + ReflectSystemSet::from_set(system_set, node_id), + )); + } + + // for some reason the graph doesn't contain these + if let Ok(systems) = schedule.systems() { + for (node_id, system) in systems { + covered_nodes.insert(node_id); + nodes.push(ReflectSystemGraphNode::System(ReflectSystem::from_system( + system.as_ref(), + node_id, + ))); + } + } + + // try find all uncovered nodes, and warn about them, or do something about them + // for now we just warn + for node_id in hierarchy.nodes() { + if covered_nodes.contains(&node_id) { + continue; + } + + bevy::log::warn!("Found uncovered node {node_id:?}"); + } + + let dependencies = dependency + .all_edges() + .map(|(from, to, _)| Edge { + from: ReflectNodeId(from), + to: ReflectNodeId(to), + }) + .collect(); + + let hierarchy = hierarchy + .all_edges() + .map(|(from, to, _)| Edge { + from: ReflectNodeId(from), + to: ReflectNodeId(to), + }) + .collect(); + + ReflectSystemGraph { + schedule: ReflectSchedule::from_label(schedule.label()), + nodes, + dependencies, + hierarchy, + } +} + +/// A graph of systems and system sets for a single schedule +#[derive(Reflect)] +pub struct ReflectSystemGraph { + /// The schedule that this graph represents + schedule: ReflectSchedule, + /// All of the included nodes + nodes: Vec, + + /// The edges signifying the dependency relationship between each node. + /// + /// I.e. if there is an edge from A -> B, then A depends on B + dependencies: Vec, + + /// The edges signifying the hierarchy relationship between each node. + /// I.e. if there is an edge from A -> B, then A is a child of B + hierarchy: Vec, +} + +impl ReflectSystemGraph { + /// Sorts the graph nodes and edges. + /// + /// Useful if you want a stable order and deterministic graphs. + pub fn sort(&mut self) { + // sort everything in this graph + self.nodes.sort_by_key(|node| match node { + ReflectSystemGraphNode::System(system) => system.node_id.0, + ReflectSystemGraphNode::SystemSet(system_set) => system_set.node_id.0, + }); + + self.dependencies.sort(); + + self.hierarchy.sort(); + } + + /// removes the set and bridges the edges connecting to it + fn absorb_set(&mut self, node_id: NodeId) { + // collect hierarchy parents and children in one pass + let mut hierarchy_parents = Vec::new(); + let mut hierarchy_children = Vec::new(); + // the relation ship expressed by edges is "is child of" + for edge in &self.hierarchy { + // these are children + if edge.to.0 == node_id { + hierarchy_children.push(edge.from.clone()); + } + // these are parents + if edge.from.0 == node_id { + hierarchy_parents.push(edge.to.clone()); + } + } + + // + let mut dependencies = Vec::new(); + let mut dependents = Vec::new(); + // the relationship expressed is "runs before" i.e. "is depended on by" + for edge in &self.dependencies { + if edge.to.0 == node_id { + dependencies.push(edge.from.clone()); + } + if edge.from.0 == node_id { + dependents.push(edge.to.clone()); + } + } + + let mut new_hierarchy_edges = + HashSet::with_capacity(hierarchy_parents.len() * hierarchy_children.len()); + let mut new_dependency_edges = + HashSet::with_capacity(dependencies.len() * dependents.len()); + + // each parent, becomes a parent to the sets children + for parent in hierarchy_parents.iter() { + for child in hierarchy_children.iter() { + new_hierarchy_edges.insert(Edge { + from: child.clone(), + to: parent.clone(), + }); + } + } + + for child in hierarchy_parents.iter() { + // bridge dependencies + for dependency in dependencies.iter() { + new_dependency_edges.insert(Edge { + from: dependency.clone(), + to: child.clone(), + }); + } + + for dependent in dependents.iter() { + new_dependency_edges.insert(Edge { + from: child.clone(), + to: dependent.clone(), + }); + } + } + + // remove any edges involving the set to absorb + self.hierarchy + .retain(|edge| edge.from.0 != node_id && edge.to.0 != node_id); + self.dependencies + .retain(|edge| edge.from.0 != node_id && edge.to.0 != node_id); + + // remove the set from nodes + self.nodes.retain(|node| match node { + ReflectSystemGraphNode::SystemSet(system_set) => system_set.node_id.0 != node_id, + _ => true, + }); + + // add new bridging edges + self.hierarchy.extend(new_hierarchy_edges); + self.dependencies.extend(new_dependency_edges); + } + + /// type system sets, are not really important to us, for all intents and purposes + /// they are one and the same as the underlying systems + /// Adapter and pipe systems might have multiple default system sets attached, but we want all them gone from the graph. + /// + /// Inlines every type system set into its children, replacing anything pointing to those sets by edges to every system contained in the set + pub fn absorb_type_system_sets(&mut self) { + let type_sets = self + .nodes + .iter() + .filter_map(|node| match node { + ReflectSystemGraphNode::SystemSet(system_set) => { + if system_set.type_id.is_some() { + Some(system_set.node_id.0) + } else { + None + } + } + _ => None, + }) + .collect::>(); + + // yes this is very inefficient, no this isn't a big hot path, these graphs mostly serve a debugging role. + for node_id in type_sets { + self.absorb_set(node_id); + } + } +} + +/// A node in the reflectable system graph +#[derive(Reflect)] +pub enum ReflectSystemGraphNode { + /// A system node + System(ReflectSystem), + /// A system set node + SystemSet(ReflectSystemSet), +} + +/// An edge in the graph +#[derive(Reflect, PartialEq, Eq, PartialOrd, Ord, Hash)] +pub struct Edge { + /// The id of the node this edge is coming from + from: ReflectNodeId, + /// The id of the node this edge is going to + to: ReflectNodeId, +} + +#[cfg(test)] +mod test { + use bevy::{ + app::Update, + ecs::{ + schedule::{IntoSystemConfigs, IntoSystemSetConfigs}, + world::World, + }, + }; + + use super::*; + + fn system_a() {} + + fn system_b() {} + + fn system_c() {} + + fn system_d() {} + + fn system_e() {} + + const BLESS_MODE: bool = false; + + #[test] + fn test_graph_is_as_expected() { + // create empty schedule and graph it + + let mut schedule = Schedule::new(Update); + + #[derive(SystemSet, Hash, PartialEq, Eq, PartialOrd, Ord, Debug, Clone, Copy)] + enum SystemSet { + SystemSetG, + SystemSetH, + } + + // add a few systems that depend on each other, some via system sets + + schedule + .add_systems(system_a) + .add_systems(system_b.before(system_a)) + .add_systems(system_c.after(system_b).before(SystemSet::SystemSetH)) + .add_systems(system_d.in_set(SystemSet::SystemSetG)) + .add_systems(system_e.in_set(SystemSet::SystemSetH)) + .configure_sets(SystemSet::SystemSetG.after(SystemSet::SystemSetH)); + let mut world = World::new(); + schedule.initialize(&mut world).unwrap(); + + let mut graph = schedule_to_reflect_graph(&schedule); + graph.absorb_type_system_sets(); + graph.sort(); + let dot = reflect_graph_to_dot(graph); + + let normalize = |s: &str| { + // trim each line individually from the start, and replace " = " with "=" to deal with formatters + let lines: Vec<&str> = s.lines().map(|line| line.trim_start()).collect(); + lines + .join("\n") + .replace(" = ", "") + .replace(";", "") + .replace(",", "") + .trim() + .to_string() + }; + + // check that the dot graph is as expected + // the expected file is found next to the src/lib.rs folder + let expected = include_str!("../test_graph.dot"); + let expected_path = manifest_dir_macros::file_path!("test_graph.dot"); + + if BLESS_MODE { + std::fs::write(expected_path, normalize(&dot)).unwrap(); + panic!("Bless mode is active"); + } else { + pretty_assertions::assert_eq!(normalize(&dot), normalize(expected)); + } + } +} diff --git a/crates/bevy_system_reflection/test_graph.dot b/crates/bevy_system_reflection/test_graph.dot new file mode 100644 index 000000000..8f1657aa4 --- /dev/null +++ b/crates/bevy_system_reflection/test_graph.dot @@ -0,0 +1,15 @@ +digraph { +node_0 [label="bevy_system_reflection::test::system_a"] +node_1 [label="bevy_system_reflection::test::system_b"] +node_2 [label="bevy_system_reflection::test::system_c"] +node_3 [label="bevy_system_reflection::test::system_d"] +node_4 [label="bevy_system_reflection::test::system_e"] +node_5 [label="SystemSet SystemSetH"] +node_6 [label="SystemSet SystemSetG"] +node_4 -> node_5 [color=red label="child of" arrowhead=diamond] +node_3 -> node_6 [color=red label="child of" arrowhead=diamond] +node_1 -> node_0 [color=blue label="runs before" arrowhead=normal] +node_1 -> node_2 [color=blue label="runs before" arrowhead=normal] +node_2 -> node_5 [color=blue label="runs before" arrowhead=normal] +node_5 -> node_6 [color=blue label="runs before" arrowhead=normal] +} \ No newline at end of file diff --git a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs index 6ae0f69d5..fee3189de 100644 --- a/crates/languages/bevy_mod_scripting_rhai/src/lib.rs +++ b/crates/languages/bevy_mod_scripting_rhai/src/lib.rs @@ -72,7 +72,7 @@ impl Default for RhaiScriptingPlugin { runtime_settings: RuntimeSettings { initializers: vec![|runtime: &RhaiRuntime| { let mut engine = runtime.write(); - + engine.set_max_expr_depths(999, 999); engine.build_type::(); engine.build_type::(); engine.register_iterator_result::(); diff --git a/crates/testing_crates/script_integration_test_harness/Cargo.toml b/crates/testing_crates/script_integration_test_harness/Cargo.toml index d335d8ee6..850c77679 100644 --- a/crates/testing_crates/script_integration_test_harness/Cargo.toml +++ b/crates/testing_crates/script_integration_test_harness/Cargo.toml @@ -15,3 +15,4 @@ bevy_mod_scripting_functions = { workspace = true, features = [ "lua_bindings", ] } regex = { version = "1.11" } +pretty_assertions = "1.*" diff --git a/crates/testing_crates/script_integration_test_harness/src/lib.rs b/crates/testing_crates/script_integration_test_harness/src/lib.rs index 1a171e309..d17836bbc 100644 --- a/crates/testing_crates/script_integration_test_harness/src/lib.rs +++ b/crates/testing_crates/script_integration_test_harness/src/lib.rs @@ -35,6 +35,8 @@ use test_utils::test_data::setup_integration_test; fn dummy_update_system() {} fn dummy_startup_system() {} +fn dummy_before_post_update_system() {} +fn dummy_post_update_system() {} #[derive(Event)] struct TestEventFinished; @@ -124,6 +126,12 @@ pub fn execute_integration_test< app.add_systems(Update, dummy_update_system); app.add_systems(Startup, dummy_startup_system::); + app.add_systems( + PostUpdate, + dummy_before_post_update_system.before(dummy_post_update_system), + ); + app.add_systems(PostUpdate, dummy_post_update_system); + app.cleanup(); app.finish(); @@ -145,7 +153,7 @@ pub fn execute_integration_test< if let Some(event) = error_events.into_iter().next() { return Err(event .error - .display_with_world(WorldGuard::new(app.world_mut()))); + .display_with_world(WorldGuard::new_exclusive(app.world_mut()))); } let events_completed = app.world_mut().resource_ref::>(); diff --git a/crates/testing_crates/script_integration_test_harness/src/test_functions.rs b/crates/testing_crates/script_integration_test_harness/src/test_functions.rs index d57ccd810..71a526881 100644 --- a/crates/testing_crates/script_integration_test_harness/src/test_functions.rs +++ b/crates/testing_crates/script_integration_test_harness/src/test_functions.rs @@ -53,6 +53,10 @@ pub fn register_test_functions(world: &mut App) { let mut allocator = allocator.write(); ReflectReference::new_allocated(comp, &mut allocator) }) + .register("_sleep", |time: f64| { + std::thread::sleep(std::time::Duration::from_secs_f64(time)); + Ok(()) + }) .register( "_get_entity_with_test_component", |s: FunctionCallContext, name: String| { @@ -104,5 +108,16 @@ pub fn register_test_functions(world: &mut App) { NamespaceBuilder::::new_unregistered(world) .register("global_hello_world", || Ok("hi!")) - .register("make_hashmap", |map: HashMap| map); + .register("make_hashmap", |map: HashMap| map) + .register( + "assert_str_eq", + |s1: String, s2: String, reason: Option| { + pretty_assertions::assert_eq!( + s1.trim(), + s2.trim(), + "Reason Provided: {}", + reason.unwrap_or_default() + ) + }, + ); } diff --git a/crates/testing_crates/test_utils/src/test_plugin.rs b/crates/testing_crates/test_utils/src/test_plugin.rs index f5d63c00a..7a091f6c8 100644 --- a/crates/testing_crates/test_utils/src/test_plugin.rs +++ b/crates/testing_crates/test_utils/src/test_plugin.rs @@ -33,7 +33,8 @@ macro_rules! make_test_plugin { #[derive(Default)] struct TestRuntime { - pub invocations: parking_lot::Mutex>, + pub invocations: + parking_lot::Mutex>, } #[derive(Default)] diff --git a/other-release-plz.toml b/other-release-plz.toml new file mode 100644 index 000000000..86f0b8d00 --- /dev/null +++ b/other-release-plz.toml @@ -0,0 +1,96 @@ +[workspace] +dependencies_update = false +publish_timeout = "30m" +git_release_enable = false +git_tag_enable = false +git_release_body = """ +{{ changelog }} +{% if remote.contributors %} +### Contributors +{% for contributor in remote.contributors | unique(attribute="username") %} +* @{{ contributor.username }} +{% endfor %} +{% endif %} +""" + +[changelog] +commit_parsers = [ + # dont include chore changes in changelog + { message = "^chore.*", skip = true }, + { message = "^test.*", skip = true }, + { message = "^docs.*", skip = true }, + { message = "^feat", group = "added" }, + { message = "^changed", group = "changed" }, + { message = "^deprecated", group = "deprecated" }, + { message = "^fix", group = "fixed" }, + { message = "^security", group = "security" }, + { message = "^.*", group = "other" }, +] + +[[package]] +name = "bevy_mod_scripting" +publish_features = ["lua54"] +version_group = "main" +git_release_latest = true +git_release_enable = true +git_tag_enable = true +git_tag_name = "v{{ version }}" +git_release_name = "v{{ version }}" + +changelog_include = [ + "bevy_mod_scripting_lua", + "bevy_mod_scripting_core", + "bevy_mod_scripting_rhai", + # "bevy_mod_scripting_rune", + "bevy_mod_scripting_functions", +] + +[[package]] +name = "bevy_mod_scripting_lua" +publish_features = ["lua54"] +version_group = "main" + +[[package]] +name = "bevy_mod_scripting_core" +version_group = "main" + +[[package]] +name = "bevy_mod_scripting_derive" +version_group = "main" + +[[package]] +name = "bevy_mod_scripting_rhai" +version_group = "main" + +# [[package]] +# name = "bevy_mod_scripting_rune" +# version_group = "main" + +[[package]] +name = "bevy_mod_scripting_functions" +version_group = "main" + +[[package]] +name = "ladfile" +git_release_enable = true +git_release_latest = false +git_tag_enable = true +git_tag_name = "v{{ version }}-ladfile" +git_release_name = "v{{ version }}-ladfile" + +[[package]] +name = "ladfile_builder" +git_release_enable = true +git_release_latest = false +git_tag_enable = true +git_tag_name = "v{{ version }}-ladfile_builder" +git_release_name = "v{{ version }}-ladfile_builder" + +[[package]] +changelog_update = true +name = "mdbook_lad_preprocessor" +git_release_enable = true +git_release_latest = false +git_tag_enable = true +git_tag_name = "v{{ version }}-mdbook_lad_preprocessor" +git_release_name = "v{{ version }}-mdbook_lad_preprocessor" diff --git a/release-plz.toml b/release-plz.toml index cf84be79a..83b5bc6e0 100644 --- a/release-plz.toml +++ b/release-plz.toml @@ -100,3 +100,12 @@ git_release_latest = false git_tag_enable = true git_tag_name = "v{{ version }}-mdbook_lad_preprocessor" git_release_name = "v{{ version }}-mdbook_lad_preprocessor" + +[[package]] +changelog_update = true +name = "bevy_system_reflection" +git_release_enable = true +git_release_latest = false +git_tag_enable = true +git_tag_name = "v{{ version }}-bevy_system_reflection" +git_release_name = "v{{ version }}-bevy_system_reflection"