Skip to content

Commit dd9a6ab

Browse files
committed
Remove NotUniqueError, keep Gd::try_to_unique() simple
Implements IoError::source() for all paths.
1 parent 30a6dc5 commit dd9a6ab

File tree

5 files changed

+56
-85
lines changed

5 files changed

+56
-85
lines changed

godot-core/src/meta/error/mod.rs

-2
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99
1010
mod call_error;
1111
mod convert_error;
12-
mod not_unique_error;
1312

1413
pub use call_error::*;
1514
pub use convert_error::*;
16-
pub use not_unique_error::*;

godot-core/src/meta/error/not_unique_error.rs

-33
This file was deleted.

godot-core/src/obj/gd.rs

+17-16
Original file line numberDiff line numberDiff line change
@@ -561,31 +561,32 @@ where
561561
{
562562
/// Makes sure that `self` does not share references with other `Gd` instances.
563563
///
564-
/// If `self` is unique, i.e. its reference count is 1, then `Ok(self)` is returned.
565-
///
566-
/// If `self` is shared or not reference-counted (`T=Object` pointing to a dynamic type that is manually managed),
567-
/// then `Err((self, NotUniqueError))` is returned. You can thus reuse the original object (first element in the tuple).
564+
/// Succeeds if the reference count is 1.
565+
/// Otherwise, returns the shared object and its reference count.
568566
///
569567
/// ## Example
570568
///
571569
/// ```no_run
572570
/// use godot::prelude::*;
573-
/// use godot::meta::error::NotUniqueError;
574-
///
575-
/// let unique = RefCounted::new_gd();
576-
/// assert!(unique.try_to_unique().is_ok());
577571
///
578-
/// let shared = RefCounted::new_gd();
579-
/// let shared2 = shared.clone();
580-
/// assert!(shared.try_to_unique().is_err());
572+
/// let obj = RefCounted::new_gd();
573+
/// match obj.try_to_unique() {
574+
/// Ok(unique_obj) => {
575+
/// // No other Gd<T> shares a reference with `unique_obj`.
576+
/// },
577+
/// Err((shared_obj, ref_count)) => {
578+
/// // `shared_obj` is the original object `obj`.
579+
/// // `ref_count` is the total number of references (including one held by `shared_obj`).
580+
/// }
581+
/// }
581582
/// ```
582-
pub fn try_to_unique(mut self) -> Result<Self, (Self, NotUniqueError)> {
583+
pub fn try_to_unique(self) -> Result<Self, (Self, usize)> {
583584
use crate::obj::bounds::DynMemory as _;
584585

585586
match <T as Bounds>::DynMemory::get_ref_count(&self.raw) {
586587
Some(1) => Ok(self),
587-
Some(ref_count) => Err((self, NotUniqueError::Shared { ref_count })),
588-
None => Err((self, NotUniqueError::NotRefCounted)),
588+
Some(ref_count) => Err((self, ref_count)),
589+
None => unreachable!(),
589590
}
590591
}
591592
}
@@ -774,5 +775,5 @@ impl<T: GodotClass> std::hash::Hash for Gd<T> {
774775
impl<T: GodotClass> std::panic::UnwindSafe for Gd<T> {}
775776
impl<T: GodotClass> std::panic::RefUnwindSafe for Gd<T> {}
776777

777-
#[deprecated = "Moved to `godot::meta::error`"]
778-
pub use crate::meta::error::NotUniqueError;
778+
#[deprecated = "Removed; see `Gd::try_to_unique()`"]
779+
pub type NotUniqueError = ();

godot-core/src/tools/io_error.rs

+37-21
Original file line numberDiff line numberDiff line change
@@ -6,19 +6,20 @@
66
*/
77

88
use std::error::Error;
9+
use std::fmt;
910

1011
use crate::gen::classes::FileAccess;
1112
use crate::global::Error as GodotError;
12-
use crate::obj::{Gd, NotUniqueError};
13+
use crate::obj::Gd;
1314

1415
/// Error that can occur while using `gdext` IO utilities.
1516
#[derive(Debug)]
1617
pub struct IoError {
1718
data: ErrorData,
1819
}
1920

20-
impl std::fmt::Display for IoError {
21-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
21+
impl fmt::Display for IoError {
22+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
2223
match &self.data {
2324
ErrorData::Load(err) => err.fmt(f),
2425
ErrorData::Save(err) => err.fmt(f),
@@ -29,14 +30,12 @@ impl std::fmt::Display for IoError {
2930

3031
impl Error for IoError {
3132
fn source(&self) -> Option<&(dyn Error + 'static)> {
32-
if let ErrorData::GFile(GFileError {
33-
kind: GFileErrorKind::NotUniqueRef(err),
34-
..
35-
}) = &self.data
36-
{
37-
return Some(err);
33+
// Note: inner types are not public, but the dyn trait can be used.
34+
match &self.data {
35+
ErrorData::Load(err) => Some(err),
36+
ErrorData::Save(err) => Some(err),
37+
ErrorData::GFile(err) => Some(err),
3838
}
39-
None
4039
}
4140
}
4241

@@ -87,23 +86,27 @@ impl IoError {
8786

8887
match file_access.try_to_unique() {
8988
Ok(gd) => Ok(gd),
90-
Err((_, err)) => Err(Self {
89+
Err((_drop, ref_count)) => Err(Self {
9190
data: ErrorData::GFile(GFileError {
92-
kind: GFileErrorKind::NotUniqueRef(err),
91+
kind: GFileErrorKind::NotUniqueRef { ref_count },
9392
path: path.to_string(),
9493
}),
9594
}),
9695
}
9796
}
9897
}
9998

99+
// ----------------------------------------------------------------------------------------------------------------------------------------------
100+
100101
#[derive(Debug)]
101102
enum ErrorData {
102103
Load(LoaderError),
103104
Save(SaverError),
104105
GFile(GFileError),
105106
}
106107

108+
// ----------------------------------------------------------------------------------------------------------------------------------------------
109+
107110
#[derive(Debug)]
108111
struct LoaderError {
109112
kind: LoaderErrorKind,
@@ -117,8 +120,10 @@ enum LoaderErrorKind {
117120
Cast,
118121
}
119122

120-
impl std::fmt::Display for LoaderError {
121-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
123+
impl Error for LoaderError {}
124+
125+
impl fmt::Display for LoaderError {
126+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
122127
let class = &self.class;
123128
let path = &self.path;
124129

@@ -135,15 +140,19 @@ impl std::fmt::Display for LoaderError {
135140
}
136141
}
137142

143+
// ----------------------------------------------------------------------------------------------------------------------------------------------
144+
138145
#[derive(Debug)]
139146
struct SaverError {
140147
class: String,
141148
path: String,
142149
godot_error: GodotError,
143150
}
144151

145-
impl std::fmt::Display for SaverError {
146-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
152+
impl Error for SaverError {}
153+
154+
impl fmt::Display for SaverError {
155+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
147156
let class = &self.class;
148157
let path = &self.path;
149158
let godot_error = &self.godot_error;
@@ -152,6 +161,8 @@ impl std::fmt::Display for SaverError {
152161
}
153162
}
154163

164+
// ----------------------------------------------------------------------------------------------------------------------------------------------
165+
155166
#[derive(Debug)]
156167
struct GFileError {
157168
kind: GFileErrorKind,
@@ -160,17 +171,22 @@ struct GFileError {
160171

161172
#[derive(Debug)]
162173
enum GFileErrorKind {
163-
NotUniqueRef(NotUniqueError),
174+
NotUniqueRef { ref_count: usize },
164175
NotOpen,
165176
}
166177

167-
impl std::fmt::Display for GFileError {
168-
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
178+
impl Error for GFileError {}
179+
180+
impl fmt::Display for GFileError {
181+
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
169182
let path = &self.path;
170183

171184
match &self.kind {
172-
GFileErrorKind::NotUniqueRef(err) => {
173-
write!(f, "access to file '{path}' is not unique: '{err}'")
185+
GFileErrorKind::NotUniqueRef { ref_count } => {
186+
write!(
187+
f,
188+
"Gd<FileAccess> for '{path}' is not unique (ref-count {ref_count})"
189+
)
174190
}
175191
GFileErrorKind::NotOpen => write!(f, "access to file '{path}' is not open"),
176192
}

itest/rust/src/object_tests/object_test.rs

+2-13
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ use godot::classes::{
1414
RefCounted,
1515
};
1616
use godot::global::instance_from_id;
17-
use godot::meta::error::NotUniqueError;
1817
use godot::meta::{FromGodot, GodotType, ToGodot};
1918
use godot::obj::{Base, Gd, Inherits, InstanceId, NewAlloc, NewGd, RawGd};
2019
use godot::register::{godot_api, GodotClass};
@@ -849,19 +848,9 @@ fn object_try_to_unique() {
849848
assert_eq!(a.instance_id(), id);
850849

851850
let b = a.clone();
852-
let (b, err) = b.try_to_unique().expect_err("b.try_to_unique()");
851+
let (b, ref_count) = b.try_to_unique().expect_err("b.try_to_unique()");
853852
assert_eq!(b.instance_id(), id);
854-
assert!(matches!(err, NotUniqueError::Shared { ref_count: 2 }));
855-
856-
/* Re-enable if DynMemory is fixed.
857-
858-
let c = Object::new_alloc(); // manually managed
859-
let id = c.instance_id();
860-
let (c, err) = c.try_to_unique().expect_err("c.try_to_unique()");
861-
assert_eq!(c.instance_id(), id);
862-
assert!(matches!(err, NotUniqueError::NotRefCounted));
863-
c.free();
864-
*/
853+
assert_eq!(ref_count, 2);
865854
}
866855

867856
// ----------------------------------------------------------------------------------------------------------------------------------------------

0 commit comments

Comments
 (0)