Fixes for FFI, refactor FFI Error to be easier to implement

src/ffi/dynamic_data/models/battle.rs

@@ -3,7 +3,7 @@ use crate::dynamic_data::{
 };
 use crate::ffi::dynamic_data::models::native_event_hook::NativeEventHook;
 use crate::ffi::ffi_handle::{FFIHandle, FromFFIHandle};
-use crate::ffi::FFIResult;
+use crate::ffi::{FFIResult, OwnedPtrString};
 use anyhow::anyhow;
 use std::ffi::{c_char, CStr, CString};
 use std::ops::Deref;
@@ -204,13 +204,13 @@ extern "C" fn battle_set_weather(ptr: FFIHandle<Battle>, weather: *const c_char)
 
 /// Gets the current weather of the battle. If no weather is present, this returns nullptr.
 #[no_mangle]
-extern "C" fn battle_weather_name(ptr: FFIHandle<Battle>) -> FFIResult<*mut c_char> {
+extern "C" fn battle_weather_name(ptr: FFIHandle<Battle>) -> FFIResult<OwnedPtrString> {
     match ptr.from_ffi_handle().weather_name() {
         Ok(Some(w)) => match CString::new(w.str()) {
-            Ok(s) => s.into_raw().into(),
+            Ok(s) => OwnedPtrString(s.into_raw()).into(),
             Err(e) => FFIResult::err(anyhow!("Failed to convert weather name to CString: {}", e)),
         },
-        Ok(None) => std::ptr::null_mut::<c_char>().into(),
+        Ok(None) => OwnedPtrString(std::ptr::null_mut()).into(),
         Err(e) => FFIResult::err(e),
     }
 }

src/ffi/dynamic_data/models/pokemon.rs

@@ -1,7 +1,7 @@
 use crate::defines::LevelInt;
 use crate::dynamic_data::{Battle, DamageSource, DynamicLibrary, LearnedMove, MoveLearnMethod, Pokemon};
 use crate::ffi::ffi_handle::{FFIHandle, FromFFIHandle};
-use crate::ffi::FFIResult;
+use crate::ffi::{FFIResult, OwnedPtrString};
 use crate::static_data::{
     Ability, AbilityIndex, Form, Gender, Item, Nature, Species, Statistic, StatisticSet, TypeIdentifier,
 };
@@ -180,16 +180,16 @@ extern "C" fn pokemon_height(handle: FFIHandle<Pokemon>) -> f32 {
 
 /// An optional nickname of the Pokemon.
 #[no_mangle]
-extern "C" fn pokemon_nickname(handle: FFIHandle<Pokemon>) -> FFIResult<*mut c_char> {
+extern "C" fn pokemon_nickname(handle: FFIHandle<Pokemon>) -> FFIResult<OwnedPtrString> {
     let form = handle.from_ffi_handle();
     let name = form.nickname();
     if let Some(v) = name {
         match CString::new(v.as_str()) {
-            Ok(v) => FFIResult::ok(v.into_raw()),
+            Ok(v) => FFIResult::ok(OwnedPtrString(v.into_raw())),
             Err(err) => FFIResult::err(anyhow!("Could not convert nickname to CString: {}", err)),
         }
     } else {
-        FFIResult::ok(std::ptr::null_mut())
+        FFIResult::ok(OwnedPtrString(std::ptr::null_mut()))
     }
 }
 

src/ffi/ffi_handle.rs

@@ -45,6 +45,15 @@ impl<T> Clone for FFIHandle<T> {
 
 impl<T> Copy for FFIHandle<T> {}
 
+impl<T> Default for FFIHandle<T> {
+    fn default() -> Self {
+        Self {
+            handle: 0,
+            _marker: std::marker::PhantomData,
+        }
+    }
+}
+
 /// An FFIObject we can fetch from the FFIHandle. We store this in a hashmap to be able to fetch
 /// the object from the handle, and to be able to drop the object when the FFI is done with the handle
 #[derive(Clone)]
@@ -98,7 +107,7 @@ unsafe impl Send for FFIObject {}
 unsafe impl Sync for FFIObject {}
 
 /// The next handle to be used.
-static NEXT_HANDLE: AtomicUsize = AtomicUsize::new(0);
+static NEXT_HANDLE: AtomicUsize = AtomicUsize::new(1);
 /// A lookup to get an actual object from a handle.
 static FFI_OBJECTS: LazyLock<RwLock<HashMap<usize, FFIObject>>> = LazyLock::new(|| RwLock::new(HashMap::new()));
 /// A lookup to get a handle from an object.
@@ -137,7 +146,7 @@ impl<T> FFIHandle<T> {
         FFI_OBJECTS
             .read()
             .get(&self.handle)
-            .ok_or(anyhow!("Unable to get handle"))
+            .ok_or(anyhow!("Unable to get handle {} from FFI_OBJECTS", self.handle))
             .unwrap()
             .clone()
     }

src/ffi/mod.rs

@@ -8,10 +8,19 @@ mod static_data;
 pub(self) use ffi_handle::*;
 
 /// Helper type for clearer functions.
-type OwnedPtrString = *mut c_char;
+#[repr(C)]
+#[derive(Debug, Copy, Clone)]
+struct OwnedPtrString(*mut c_char);
+
 /// Helper type for clearer functions.
 type NonOwnedPtrString = *const c_char;
 
+impl Default for OwnedPtrString {
+    fn default() -> Self {
+        OwnedPtrString(std::ptr::null_mut())
+    }
+}
+
 /// Generates a basic getter foreign function interface.
 macro_rules! ffi_handle_arc_dyn_getter {
     (
@@ -71,7 +80,7 @@ macro_rules! ffi_handle_vec_stringkey_getters {
             }
             #[no_mangle]
             extern "C" fn [< $type:lower _ $func _get>](ptr: FFIHandle<Arc<dyn $type>>, index: usize) -> OwnedPtrString {
-                CString::new(ptr.from_ffi_handle().$func().get(index).unwrap().str()).unwrap().into_raw()
+                OwnedPtrString(CString::new(ptr.from_ffi_handle().$func().get(index).unwrap().str()).unwrap().into_raw())
             }
         }
     };
@@ -102,40 +111,30 @@ impl<T: ?Sized> ExternPointer<T> {
     }
 }
 
-/// Helper utility class to give either the data or an error to a FFI.
-#[repr(C)]
-union ResultUnion<T: Copy> {
-    /// If the result is ok, this contains the value.
-    ok: T,
-    /// If the result is an error, this contains the error message.
-    err: NonOwnedPtrString,
-}
-
 /// The result of a FFI call that can either be an error or a value.
 #[repr(C)]
-pub struct FFIResult<T: Copy> {
+#[repr(packed)]
-    /// If the result is ok, this is 1, otherwise 0.
+pub struct FFIResult<T: Copy + Default> {
-    ok: u8,
+    /// If the result is ok, this is null, otherwise this is a pointer to the error string.
+    err: NonOwnedPtrString,
     /// The value or error.
-    value: ResultUnion<T>,
+    value: T,
 }
 
-impl<T: Copy> FFIResult<T> {
+impl<T: Copy + Default> FFIResult<T> {
     /// Creates a new NativeResult with the given value.
     pub fn ok(value: T) -> Self {
         Self {
-            ok: 1,
+            err: std::ptr::null(),
-            value: ResultUnion { ok: value },
+            value,
         }
     }
     /// Creates a new NativeResult with the given error.
     #[allow(clippy::unwrap_used)] // We know for certain this is not empty.
     pub fn err(err: anyhow_ext::Error) -> Self {
         Self {
-            ok: 0,
-            value: ResultUnion {
             err: CString::new(err.to_string()).unwrap().into_raw(),
-            },
+            value: Default::default(),
         }
     }
 
@@ -143,15 +142,13 @@ impl<T: Copy> FFIResult<T> {
     #[allow(clippy::unwrap_used)] // We know for certain this is not empty.
     pub fn err_from_str(err: &str) -> Self {
         Self {
-            ok: 0,
-            value: ResultUnion {
             err: CString::new(err).unwrap().into_raw(),
-            },
+            value: Default::default(),
         }
     }
 }
 
-impl<T: Copy> From<anyhow::Result<T>> for FFIResult<T> {
+impl<T: Copy + Default> From<anyhow::Result<T>> for FFIResult<T> {
     fn from(value: anyhow::Result<T>) -> Self {
         match value {
             Ok(v) => Self::ok(v),
@@ -160,7 +157,7 @@ impl<T: Copy> From<anyhow::Result<T>> for FFIResult<T> {
     }
 }
 
-impl<T: Copy> From<T> for FFIResult<T> {
+impl<T: Copy + Default> From<T> for FFIResult<T> {
     fn from(value: T) -> Self {
         Self::ok(value)
     }

src/ffi/static_data/ability.rs

@@ -38,7 +38,7 @@ unsafe extern "C" fn ability_new(
 #[no_mangle]
 unsafe extern "C" fn ability_name(handle: FFIHandle<Arc<dyn Ability>>) -> FFIResult<OwnedPtrString> {
     match CString::new(handle.from_ffi_handle().name().str()) {
-        Ok(s) => FFIResult::ok(s.into_raw()),
+        Ok(s) => FFIResult::ok(OwnedPtrString(s.into_raw())),
         Err(_) => FFIResult::err(anyhow!("Failed to convert name to CString")),
     }
 }
@@ -47,7 +47,7 @@ unsafe extern "C" fn ability_name(handle: FFIHandle<Arc<dyn Ability>>) -> FFIRes
 #[no_mangle]
 unsafe extern "C" fn ability_effect(ptr: FFIHandle<Arc<dyn Ability>>) -> FFIResult<OwnedPtrString> {
     match CString::new(ptr.from_ffi_handle().effect().str()) {
-        Ok(s) => FFIResult::ok(s.into_raw()),
+        Ok(s) => FFIResult::ok(OwnedPtrString(s.into_raw())),
         Err(_) => FFIResult::err(anyhow!("Failed to convert effect to CString")),
     }
 }

src/ffi/static_data/form.rs

@@ -75,7 +75,7 @@ unsafe extern "C" fn form_name(ptr: FFIHandle<Arc<dyn Form>>) -> FFIResult<Owned
     let obj = ptr.from_ffi_handle();
     let name = obj.name();
     match CString::new(name.str()) {
-        Ok(name) => FFIResult::ok(name.into_raw()),
+        Ok(name) => FFIResult::ok(OwnedPtrString(name.into_raw())),
         Err(_) => FFIResult::err(anyhow!("Unable to convert name `{}` to CString", name.str())),
     }
 }

src/ffi/static_data/item.rs

@@ -41,7 +41,7 @@ unsafe extern "C" fn item_name(ptr: FFIHandle<Arc<dyn Item>>) -> FFIResult<Owned
     let item = ptr.from_ffi_handle();
     let name = item.name();
     match CString::new(name.str()) {
-        Ok(name) => FFIResult::ok(name.into_raw()),
+        Ok(name) => FFIResult::ok(OwnedPtrString(name.into_raw())),
         Err(_) => FFIResult::err(anyhow!("Unable to convert name `{}` to CString", name.str())),
     }
 }

src/ffi/static_data/libraries/mod.rs

@@ -53,9 +53,9 @@ macro_rules! library_interface {
                 let lib = ptr.from_ffi_handle();
                 let v = lib.get_key_by_index(index);
                 if let Some(value) = v {
-                    std::ffi::CString::new(value.str()).unwrap().into_raw()
+                    OwnedPtrString(std::ffi::CString::new(value.str()).unwrap().into_raw())
                 } else {
-                    std::ptr::null_mut()
+                    OwnedPtrString(std::ptr::null_mut())
                 }
             }
 

src/ffi/static_data/libraries/nature_library.rs

@@ -68,7 +68,7 @@ unsafe extern "C" fn nature_library_get_nature_name(
             let name = ptr.from_ffi_handle().get_nature_name(&nature);
             match name {
                 Ok(name) => match CString::new(name.str()) {
-                    Ok(cstr) => FFIResult::ok(cstr.into_raw()),
+                    Ok(cstr) => FFIResult::ok(OwnedPtrString(cstr.into_raw())),
                     Err(_) => FFIResult::err(anyhow!("Failed to convert nature name to C string")),
                 },
                 Err(e) => FFIResult::err(e),

src/ffi/static_data/libraries/type_library.rs

@@ -1,7 +1,7 @@
 use crate::ffi::ffi_handle::{FFIHandle, FromFFIHandle};
-use crate::ffi::{FFIResult, NonOwnedPtrString};
+use crate::ffi::{FFIResult, NonOwnedPtrString, OwnedPtrString};
 use crate::static_data::{TypeIdentifier, TypeLibrary, TypeLibraryImpl};
-use std::ffi::{c_char, CStr, CString};
+use std::ffi::{CStr, CString};
 use std::sync::Arc;
 
 /// Instantiates a new type library with a specific capacity.
@@ -33,17 +33,17 @@ unsafe extern "C" fn type_library_get_type_name(
     ptr: FFIHandle<Arc<dyn TypeLibrary>>,
     type_id: TypeIdentifier,
     found: *mut bool,
-) -> FFIResult<*mut c_char> {
+) -> FFIResult<OwnedPtrString> {
     if let Some(v) = ptr.from_ffi_handle().get_type_name(type_id) {
         *found = true;
 
         match CString::new(v.str()) {
-            Ok(v) => FFIResult::ok(v.into_raw()),
+            Ok(v) => FFIResult::ok(OwnedPtrString(v.into_raw())),
             Err(e) => FFIResult::err(e.into()),
         }
     } else {
         *found = false;
-        FFIResult::ok(std::ptr::null_mut())
+        FFIResult::ok(OwnedPtrString(std::ptr::null_mut()))
     }
 }
 

src/ffi/static_data/mod.rs

@@ -108,7 +108,7 @@ extern "C" fn effect_parameter_get_as_string(ptr: FFIHandle<Arc<EffectParameter>
     let p = ptr.from_ffi_handle();
     if let EffectParameter::String(b) = p.deref() {
         match CString::new(b.str().to_string()) {
-            Ok(cstr) => FFIResult::ok(cstr.into_raw()),
+            Ok(cstr) => FFIResult::ok(OwnedPtrString(cstr.into_raw())),
             Err(_) => FFIResult::err(PkmnError::InvalidCString.into()),
         }
     } else {

src/ffi/static_data/move_data.rs

@@ -1,5 +1,5 @@
 use crate::ffi::ffi_handle::{FFIHandle, FromFFIHandle};
-use crate::ffi::{ffi_handle_arc_dyn_getter, ExternPointer, FFIResult, NonOwnedPtrString, OwnedPtrString};
+use crate::ffi::{ffi_handle_arc_dyn_getter, FFIResult, NonOwnedPtrString, OwnedPtrString};
 use crate::static_data::{
     EffectParameter, MoveCategory, MoveData, MoveDataImpl, MoveTarget, SecondaryEffect, SecondaryEffectImpl,
     TypeIdentifier,
@@ -64,7 +64,7 @@ unsafe extern "C" fn move_data_name(ptr: FFIHandle<Arc<dyn MoveData>>) -> FFIRes
     let move_data = ptr.from_ffi_handle();
     let name = move_data.name();
     match CString::new(name.str()) {
-        Ok(name) => FFIResult::ok(name.into_raw()),
+        Ok(name) => FFIResult::ok(OwnedPtrString(name.into_raw())),
         Err(_) => FFIResult::err_from_str("Unable to convert name to string"),
     }
 }
@@ -90,9 +90,9 @@ unsafe extern "C" fn move_data_secondary_effect(
 
 /// Arbitrary flags that can be applied to the move.
 #[no_mangle]
-unsafe extern "C" fn move_data_has_flag(ptr: ExternPointer<Arc<dyn MoveData>>, flag: *const c_char) -> u8 {
+unsafe extern "C" fn move_data_has_flag(ptr: FFIHandle<Arc<dyn MoveData>>, flag: *const c_char) -> u8 {
     let flag = CStr::from_ptr(flag).into();
-    u8::from(ptr.as_ref().has_flag(&flag))
+    u8::from(ptr.from_ffi_handle().has_flag(&flag))
 }
 
 /// Instantiates a new Secondary Effect.
@@ -119,37 +119,37 @@ unsafe extern "C" fn secondary_effect_new(
 
 /// The chance the effect triggers.
 #[no_mangle]
-unsafe extern "C" fn secondary_effect_chance(ptr: ExternPointer<Box<dyn SecondaryEffect>>) -> f32 {
+unsafe extern "C" fn secondary_effect_chance(ptr: FFIHandle<Arc<dyn SecondaryEffect>>) -> f32 {
-    ptr.as_ref().chance()
+    ptr.from_ffi_handle().chance()
 }
 
 /// The name of the effect.
 #[no_mangle]
 unsafe extern "C" fn secondary_effect_effect_name(
-    ptr: ExternPointer<Box<dyn SecondaryEffect>>,
+    ptr: FFIHandle<Arc<dyn SecondaryEffect>>,
 ) -> FFIResult<OwnedPtrString> {
-    match CString::new(ptr.as_ref().effect_name().str()) {
+    match CString::new(ptr.from_ffi_handle().effect_name().str()) {
-        Ok(name) => FFIResult::ok(name.into_raw()),
+        Ok(name) => FFIResult::ok(OwnedPtrString(name.into_raw())),
         Err(_) => FFIResult::err(anyhow!(
             "Unable to convert effect name '{}' to CString",
-            ptr.as_ref().effect_name()
+            ptr.from_ffi_handle().effect_name()
         )),
     }
 }
 
 /// The length of parameters of the effect.
 #[no_mangle]
-unsafe extern "C" fn secondary_effect_parameter_length(ptr: ExternPointer<Box<dyn SecondaryEffect>>) -> usize {
+unsafe extern "C" fn secondary_effect_parameter_length(ptr: FFIHandle<Arc<dyn SecondaryEffect>>) -> usize {
-    ptr.as_ref().parameters().len()
+    ptr.from_ffi_handle().parameters().len()
 }
 
 /// Get a parameter of the effect.
 #[no_mangle]
 unsafe extern "C" fn secondary_effect_parameter_get(
-    ptr: ExternPointer<Box<dyn SecondaryEffect>>,
+    ptr: FFIHandle<Arc<dyn SecondaryEffect>>,
     index: usize,
 ) -> FFIHandle<Arc<EffectParameter>> {
-    if let Some(v) = ptr.as_ref().parameters().get(index) {
+    if let Some(v) = ptr.from_ffi_handle().parameters().get(index) {
         FFIHandle::get_handle(v.clone().into())
     } else {
         FFIHandle::none()