Allow Commands to register systems
Objective
- Allow registering of systems from Commands with
Commands::register_one_shot_system - Make registering of one shot systems more easy
Solution
- Add the Command
RegisterSystemfor Commands use. - Creation of SystemId based on lazy insertion of the System
- Changed the privacy of the fields in SystemId so Commands can return the SystemId
Changelog
Added
- Added command
RegisterSystem - Added function
Commands::register_one_shot_system - Added function
App::register_one_shot_system
Changed
- Changed the privacy and the type of struct tuple to regular struct of SystemId
Migration Guide
Before, if you wanted to register a system with
Commands, you would need to do:
commands.add(|world: &mut World| {
let id = world.register_system(your_system);
// You would need to insert the SystemId inside an entity or similar
})
Now, you can:
let id = commands.register_one_shot_system(your_system);
// Do what you want with the Id
While at it, is it a good Idea to add tests for testing Commands::run_system and Commands::register_system in system_registry.rs?
I will never say no to more tests :D I think in this case we should probably split them out into a seperate PR if we can: I suspect they'll be much faster to merge that way.
Regarding the matter about verifying if the System was already registred before, found out something
fn do_something() {}
fn do_another_something() {}
// If we cast the function into an pointer, we can found out the actual address of the function, which can be very useful
assert_ne!(do_something as usize, do_another_something as usize);
// That same thing is possible with closure, with a little problem though
let closure_one = || {};
let closure_two = || {};
assert_ne!(closure_one as fn() as usize, closure_two as fn() as usize);
// Closures must be casted first as a function, then into an address for this to work
// Last, with the same problem, closures created from a function can't be casted into an Address
// But if you pass the function into a var first, it can later be casted into an address from inside the function
// This works
fn get_closure_address() -> usize {
let closure = || {};
closure as fn() as usize
}
// This doesn't
fn get_closure_address() -> impl Fn() {
|| {}
}
let closure = get_closure_address();
closure as fn() as usize // Can't be casted
Maybe that can be useful
It would be cool if you added Commands::register_one_shot_system, but maybe that's better as an App method.
Made an doc-test, but not sure if that is the best doc-test In this situation. Must I assert that the function is indeed an System or maybe assert that the run will make the Counter value change?
Made an doc-test, but not sure if that is the best doc-test In this situation. Must I assert that the function is indeed an System or maybe assert that the run will make the Counter value change?
I think the latter is more robust.
I think that latter is more robust
You mean that's too much for this doc Test or more robust like "better"?
Better :) Less likely to pass while the functionality is broken.
Done it, now the doc verify both: If the function is a system and if the run affect the counter, asserting that the system is registered
I'll solve conflicts today :)