Repository: bevyengine/bevy
File: crates/bevy_ecs/src/system/combinator.rs (Rust)

Code before:
                    // SAFETY: We registered access to DefaultErrorHandler in `initialize`.
                    (unsafe { world.0.default_error_handler() })(
                        err,
                        ErrorContext::System {
                            name: system.name(),
                            last_run: system.get_last_run(),
                        },
                    );

                    // Since the error handler takes the error by value, create a new error:
                    // The original error has already been handled, including
                    // the reason for the failure here isn't important.
                    Err(format!("System `{}` failed", system.name()).into())
                }
                // `Skipped(_)` and `Ok(_)` are passed through:
                // system skipping is not an error, and isn't passed to the
                // world's error handler by the executors.
                result @ (Ok(_) | Err(RunSystemError::Skipped(_))) => result,
            }
        }

        Func::combine(
            input,
            &mut PrivateUnsafeWorldCell(world),
            // SAFETY: The world accesses for both underlying systems have been registered,
            // so the caller will guarantee that no other systems will conflict with (`a` or `b`) and DEH.
            // If either system has `is_exclusive()`, then the combined system also has `is_exclusive`.
            // Since we require a `combine` to pass in a mutable reference to `world` and that's a private type
            // passed to a function as an unbound non-'static generic argument, they can never be called in parallel
            // or re-entrantly because that would require forging another instance of `PrivateUnsafeWorldCell`.
            // This means that the world accesses in the two closures will not conflict with each other.
            // The closure's access to the DefaultErrorHandler does not
            // conflict with any potential access to the DefaultErrorHandler by
            // the systems since the closures are not run in parallel.
            |input, world| unsafe { run_system(&mut self.a, input, world) },
            // SAFETY: See the comment above.
            |input, world| unsafe { run_system(&mut self.b, input, world) },
        )
    }

    #[cfg(feature = "hotpatching")]
    #[inline]
    fn refresh_hotpatch(&mut self) {
        self.a.refresh_hotpatch();
        self.b.refresh_hotpatch();
    }

    #[inline]
    fn apply_deferred(&mut self, world: &mut World) {
        self.a.apply_deferred(world);
        self.b.apply_deferred(world);

Review comment (suggestion):
```suggestion
            // so the caller will guarantee that no other systems will conflict with (`a` or `b`) and the `DefaultErrorHandler` resource.
```

Code after:
                    // SAFETY: We registered access to DefaultErrorHandler in `initialize`.
                    (unsafe { world.0.default_error_handler() })(
                        err,
                        ErrorContext::System {
                            name: system.name(),
                            last_run: system.get_last_run(),
                        },
                    );

                    // Since the error handler takes the error by value, create a new error:
                    // The original error has already been handled, including
                    // the reason for the failure here isn't important.
                    Err(format!("System `{}` failed", system.name()).into())
                }
                // `Skipped(_)` and `Ok(_)` are passed through:
                // system skipping is not an error, and isn't passed to the
                // world's error handler by the executors.
                result @ (Ok(_) | Err(RunSystemError::Skipped(_))) => result,
            }
        }

        Func::combine(
            input,
            &mut PrivateUnsafeWorldCell(world),
            // SAFETY: The world accesses for both underlying systems have been registered,
            // so the caller will guarantee that no other systems will conflict with (`a` or `b`) and the `DefaultErrorHandler` resource.
            // If either system has `is_exclusive()`, then the combined system also has `is_exclusive`.
            // Since we require a `combine` to pass in a mutable reference to `world` and that's a private type
            // passed to a function as an unbound non-'static generic argument, they can never be called in parallel
            // or re-entrantly because that would require forging another instance of `PrivateUnsafeWorldCell`.
            // This means that the world accesses in the two closures will not conflict with each other.
            // The closure's access to the DefaultErrorHandler does not
            // conflict with any potential access to the DefaultErrorHandler by
            // the systems since the closures are not run in parallel.
            |input, world| unsafe { run_system(&mut self.a, input, world) },
            // SAFETY: See the comment above.
            |input, world| unsafe { run_system(&mut self.b, input, world) },
        )
    }

    #[cfg(feature = "hotpatching")]
    #[inline]
    fn refresh_hotpatch(&mut self) {
        self.a.refresh_hotpatch();
        self.b.refresh_hotpatch();
    }

    #[inline]
    fn apply_deferred(&mut self, world: &mut World) {
        self.a.apply_deferred(world);
        self.b.apply_deferred(world);