Skip to content

ConvertError cannot easily be constructed by user? #643

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
bluenote10 opened this issue Mar 11, 2024 · 3 comments · Fixed by #645
Closed

ConvertError cannot easily be constructed by user? #643

bluenote10 opened this issue Mar 11, 2024 · 3 comments · Fixed by #645
Labels
bug c: core Core components

Comments

@bluenote10
Copy link
Contributor

I'm trying to migrate the following code to the latest version:

impl FromGodot for DragEventKind {
    fn try_from_godot(d: i32) -> Result<Self, ConvertError> {
        match d {
            0 => Ok(DragEventKind::DragStart),
            1 => Ok(DragEventKind::Drag),
            2 => Ok(DragEventKind::DragEnd),
            _ => Err(ConvertError::new()),
        }
    }
}

ConvertError::new() seems to have disappeared, and I cannot find a simple way to construct a ConvertError. The constructors that would be appropriate seem to be private. There is ConvertError::with_cause but I don't have a Box<dyn Error + Send + Sync>.

Searching the repo I've seen this pattern:

impl FromGodot for Vector2Axis {
    fn try_from_godot(via: Self::Via) -> Result<Self, ConvertError> {
        Self::try_from_ord(via).ok_or_else(|| FromGodotError::InvalidEnum.into_error(via))
    }
}

But FromGodotError also seems to be private.

Am I missing something or is this really no longer constructible on user side?

@lilizoey
Copy link
Member

lilizoey commented Mar 12, 2024

This was changed in #634, im not entirely sure why though? Or well apparently there is this comment:

// Constructors are private (or hidden) as only the library or its proc-macros should construct this type.

Which is a weird change to make imo, when i made the ConvertError type the intent was that people could use it to provide some custom errors in their manual FromGodot impls. That's why the FromGodot impl is hardcoded to return this error type instead of something more generic, because it's publicly constructible. Otherwise what are people supposed to do if they wanna fallibly convert a godot-type into their own custom type? Should they implement TryFrom and do like foo.try_into() instead? @Bromeon idk what your intent was here

@Bromeon
Copy link
Member

Bromeon commented Mar 12, 2024

No intention, just an oversight. I don't think there's a need for 3 constructors new, custom and default doing the same though.

I did it slightly differently in #645.

@Bromeon
Copy link
Member

Bromeon commented Mar 12, 2024

@bluenote10 Since you are an active user of the ConvertError API, #644 might also interest you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug c: core Core components
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants