godot-rust / gdext

Rust bindings for Godot 4
https://mastodon.gamedev.place/@GodotRust
Mozilla Public License 2.0
2.76k stars 166 forks source link

Can't set #[repr(i32)] Enum value to another enum, limitation to #[derrive(Property)] #466

Open fteppe opened 8 months ago

fteppe commented 8 months ago

I try to do this, to create Skill as a subset of Attribute.

#[repr(i32)]
#[derive(Property, Hash, PartialEq, Eq, Clone, Copy)]
pub enum Attribute {
    MaxHealth = 0,
    Strength = 1,
    Dexterity = 2,
    Consitution = 3,
    Intelligence = 4,
    Wisdom = 5,
    Charisma = 6,
    Athletics = 7,
}

#[repr(i32)]
#[derive(Property, Hash, PartialEq, Eq, Clone, Copy)] <----- ERROR HERE
pub enum Skill {
    Atheltics = Attribute::Athletics as i32,
}

The error is as follow:

error: Complex values for enum variants are not supported unless they are between parentheses.
  --> src\player\character_sheet.rs:15:10
   |
15 | #[derive(Property, Hash, PartialEq, Eq, Clone, Copy)]

If we look at the expanded macro where we replace Attribute as i32 => 1:

// Recursive expansion of Property macro // ======================================

#[allow(unused_parens)]
impl godot::bind::property::Property for Skill {
    type Intermediate = (i32);
    fn get_property(&self) -> (i32) {
        match &self {
            Self::Atheltics => 1,
        }
    }
    fn set_property(&mut self, value: (i32)) {
        *self = match value {
            1 => Self::Atheltics,
            _ => panic!(
                "Incorrect conversion from {} to {}",
                stringify!((i32)),
                "Skill"
            ),
        }
    }
}

set_property is where we get issues with Attribute::Athletics as i32

I proposed one way to do it: in set_property generate a const ATHELTICS_I32_VAL : i32 = Attribute::Athletics as i32; and match against that.

Another proposed solution was to use if guards instead :

        *self = match value {
            val if val == (Attribute::Athletics as i32) => Self::Atheltics
patataofcourse commented 8 months ago

Have you tried wrapping Attribute::Athletics as i32 between parenthesis? That's what the error message says you should be doing

fteppe commented 8 months ago

I did try that, it generates even more errors.

error: expected identifier, found keyword `as`
  --> src\player\character_sheet.rs:17:39
   |
17 |     Atheltics = (Attribute::Athletics as i32),
   |                                       ^^ expected identifier, found keyword

error: expected one of `!`, `(`, `)`, `,`, `...`, `..=`, `..`, `::`, `{`, or `|`, found keyword `as`
  --> src\player\character_sheet.rs:17:39
   |
17 |     Atheltics = (Attribute::Athletics as i32),
   |                                      -^^ expected one of 10 possible tokens
   |                                      |
   |                                      help: missing `,`

error: expected one of `)`, `,`, `@`, or `|`, found `i32`
  --> src\player\character_sheet.rs:17:42
   |
17 |     Atheltics = (Attribute::Athletics as i32),
   |                                         -^^^ expected one of `)`, `,`, `@`, or `|`
   |                                         |
   |                                         help: missing `,`

error: proc-macro derive produced unparsable tokens
  --> src\player\character_sheet.rs:15:10
   |
15 | #[derive(Property, Hash, PartialEq, Eq, Clone, Copy)]
   |          ^^^^^^^^

warning: unused imports: `AttributeBlock`, `Attribute`
 --> src\player\creature_stats.rs:3:25
  |
3 | use super::attributes::{AttributeBlock, Attribute};
  |                         ^^^^^^^^^^^^^^  ^^^^^^^^^
  |
  = note: `#[warn(unused_imports)]` on by default

error[E0308]: mismatched types
  --> src\player\character_sheet.rs:17:17
   |
15 | #[derive(Property, Hash, PartialEq, Eq, Clone, Copy)]
   |          -------- this expression has type `i32`
16 | pub enum Skill {
17 |     Atheltics = (Attribute::Athletics as i32),
   |                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ expected `i32`, found `(_, _, _)`
   |
   = note: expected type `i32`
             found tuple `(_, _, _)

It makes sense once you see the generated code, you'll have a

match
{
       (Skill::Athletics as i32) => ...

And the compiler doesn't like that

patataofcourse commented 8 months ago

Ah, that does make sense. The constant may be the best way to go about it then, yeah.