Skip to content

Commit 117d9e5

Browse files
committed
refactor(gdnative-core): get_api panics with error message
Instead of abort the process directly, which might be confusing to users.
1 parent b8e8799 commit 117d9e5

File tree

2 files changed

+42
-6
lines changed

2 files changed

+42
-6
lines changed

gdnative-core/src/macros.rs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -245,7 +245,12 @@ macro_rules! godot_test_impl {
245245
).is_ok();
246246

247247
if !ok {
248-
$crate::godot_error!(" !! Test {} failed", str_name);
248+
if ::std::panic::catch_unwind(|| {
249+
$crate::godot_error!(" !! Test {} failed", str_name);
250+
}).is_err() {
251+
eprintln!(" !! Test {} failed", str_name);
252+
eprintln!(" !! And failed to call Godot API to log error message");
253+
}
249254
}
250255

251256
ok

gdnative-core/src/private.rs

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,13 +80,44 @@ unsafe fn check_api_compatibility(
8080
/// Returns a reference to the current API struct.
8181
///
8282
/// This function is intended to be part of the internal API. It should only be called after
83-
/// `gdnative_init` and before `gdnative_terminate`. **Calling this function when the API is
84-
/// not bound will lead to an abort**, since in most cases there is simply no point to continue
85-
/// if `get_api` failed. This allows it to be used in FFI contexts without a `catch_unwind`.
83+
/// `gdnative_init` and before `gdnative_terminate`.
84+
///
85+
/// # Panics
86+
///
87+
/// **Calling this function when the API is not bound will panic**. Note that it will abort
88+
/// directly during tests (i.e. in unit testing or enable `feature = gd-test`). Since in
89+
/// most cases there is simply no point to continue if `get_api` failed. This allows it to
90+
/// be used in FFI contexts without a `catch_unwind`. (Unwinding across FFI boundary is an
91+
/// undefined behavior.)
92+
///
93+
/// In testing environment, this function use `Option::expect` because unwinding in this
94+
/// scenario should be safe.
95+
///
96+
/// See more: https://github.com/godot-rust/godot-rust/pull/929
8697
#[inline]
87-
#[allow(clippy::redundant_closure)] // clippy false positive: https://github.com/rust-lang/rust-clippy/issues/7812
8898
pub fn get_api() -> &'static sys::GodotApi {
89-
unsafe { GODOT_API.as_ref().unwrap_or_else(|| std::process::abort()) }
99+
const ERR_MSG: &str = "
100+
This code requires the Godot engine to be running and the GDNative initialization \
101+
to have completed. It cannot execute as a standalone Rust program.
102+
103+
Hint: If you encounter this issue during unit testing, you might \
104+
need to use the godot_test! macro, and invoke the test functions in test/src/lib.rs.
105+
";
106+
107+
// Unwinding during tests should be safe and provide more ergonomic UI.
108+
#[cfg(any(test, feature = "gd-test"))]
109+
unsafe {
110+
return GODOT_API.as_ref().expect(ERR_MSG);
111+
}
112+
113+
// Abort directly to avoid undefined behaviors.
114+
#[cfg(not(any(test, feature = "gd-test")))]
115+
unsafe {
116+
return GODOT_API.as_ref().unwrap_or_else(|| {
117+
eprintln!("{}", ERR_MSG);
118+
std::process::abort();
119+
});
120+
}
90121
}
91122

92123
/// Returns a reference to the current API struct if it is bounds, or `None` otherwise.

0 commit comments

Comments
 (0)