godotengine / godot

Godot Engine ā€“ Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.23k stars 21.04k forks source link

GDextension C++ bug Dictionary PROPERTY, variable in editor #85854

Closed kone9 closed 10 months ago

kone9 commented 10 months ago

Tested versions

Godot engine 4.2 stable

Captura de pantalla 2023-12-06 132243

System information

windows 11, intel i5 10ma, rx 6600 xt,godot 4.2 stable

Issue description

When exposing a dictionary in the editor with C++ GDextension, the dictionary is reset every time you exit the node, you cannot add more keys either. That is, if you add more keys or add data, when you reload the property is deleted and it remains as if it had not been modified

Steps to reproduce

Captura de pantalla 2023-12-06 133551

https://www.youtube.com/watch?v=dEY7l8SMW18

creates a dictionary with its corresponding get and set and then adds the property to the editor.

.h

  public:
      Dictionary sounds;

        // Getter para la propiedad 'sounds'
        Dictionary get_sounds() const;

        // Setter para la propiedad 'sounds'
        void set_sounds(Dictionary value);

.CPP

//property editor binding method
ClassDB::bind_method(D_METHOD("get_sounds"), &Zombie::get_health);
ClassDB::bind_method(D_METHOD("set_sounds", "sounds"), &Zombie::set_health);
ClassDB::add_property("Zombie", PropertyInfo(Variant::DICTIONARY, "sounds"), "set_sounds", "get_sounds");
// Getter para la propiedad 'sound'
Dictionary Zombie::get_sounds() const
{
    return sounds;
}

// Setter para la propiedad 'health'
void Zombie::set_sounds(Dictionary value)
{
    sounds = sounds;
}

Minimal reproduction project (MRP)

open the zombie scene and see the scene root that was inherited from the gdextension Zombie class. There is the Dictionary property that gives an error when modifying its value from the editor.

The following project opens with visual studio 2022 and has the static libraries preconfigured, you just have to press F5 if you have installed correctly what the repository says.

https://gitlab.com/kone9/visual-studio-godot-engine-gdextension-cpp-template/-/tree/Godot_4_2_ZOMBIE?ref_type=heads

AThousandShips commented 10 months ago

Please enter the version as text, it's not accessible to provide it with a screenshot šŸ™‚

stesim commented 10 months ago

https://gitlab.com/kone9/visual-studio-godot-engine-gdextension-cpp-template/-/blob/Godot_4_2_ZOMBIE/Zombie.cpp?ref_type=heads#L245

That should probably say sounds = value; instead of sounds = sounds;. šŸ™‚

AThousandShips commented 10 months ago

@kone9 can you please confirm if this solves your code? If so then this isn't a bug šŸ™‚

kone9 commented 10 months ago

@kone9 can you please confirm if this solves your code? If so then this isn't a bug šŸ™‚

Sorry for the delay in responding, I made the correction, but the problem remains. Not only that I can't add, when I try to add all the values are deleted, all the editor properties are broken when modifying the dictionary.

https://gitlab.com/kone9/visual-studio-godot-engine-gdextension-cpp-template/-/blob/Godot_4_2_ZOMBIE/Zombie.cpp?ref_type=heads#L245

https://www.youtube.com/watch?v=GRDaNFp-NN8

AThousandShips commented 10 months ago

You're binding get/set_health to sounds, your code is full of errors, please fix your code and see if it still doesn't work, but please fix it before opening issue reports for it šŸ™‚

ClassDB::bind_method(D_METHOD("get_sounds"), &Zombie::get_health);
ClassDB::bind_method(D_METHOD("set_sounds", "sounds"), &Zombie::set_health);
kone9 commented 10 months ago

You're binding get/set_health to sounds, your code is full of errors, please fix your code and see if it still doesn't work, but please fix it before opening issue reports for it šŸ™‚

ClassDB::bind_method(D_METHOD("get_sounds"), &Zombie::get_health);
ClassDB::bind_method(D_METHOD("set_sounds", "sounds"), &Zombie::set_health);

ok Captura de pantalla 2023-12-07 200003

AThousandShips commented 10 months ago

No you need to fix it:

ClassDB::bind_method(D_METHOD("get_sounds"), &Zombie::get_sounds);  
ClassDB::bind_method(D_METHOD("set_sounds", "sounds"), &Zombie::set_sounds);

I was showing your code beng wrong

kone9 commented 10 months ago

No you need to fix it:

ClassDB::bind_method(D_METHOD("get_sounds"), &Zombie::get_sounds);    
ClassDB::bind_method(D_METHOD("set_sounds", "sounds"), &Zombie::set_sounds);

I was showing your code beng wrong

Ok, I'll try it and tell you.

AThousandShips commented 10 months ago

This is not a bug and I can confirm the correct code works, for support questions and help please first turn to the other community channels, this is for bug reporting, not a help desk. Please use other channels first to see if this is a bug or user error.

Please try and make sure your code is in basic working order before making a bug report, asking in other channels

kone9 commented 10 months ago

ClassDB::bind_method(D_METHOD("get_sounds"), &Zombie::get_sounds);
ClassDB::bind_method(D_METHOD("set_sounds", "sounds"), &Zombie::set_sounds);

The error continues, but now only when I choose another node

This is not a bug and I can confirm the correct code works, for support questions and help please first turn to the other community channels, this is for bug reporting, not a help desk. Please use other channels first to see if this is a bug or user error.

Please try and make sure your code is in basic working order before making a bug report, asking in other channels

Ok, yes you are right. Now it works. It's a bit confusing to register the property and I accidentally put in the wrong code. It is still rare that all properties are broken because of just one poorly written one. To take that into account. Thank you so much.

video showing the correct operation of dictionaries with GDextension. https://www.youtube.com/watch?v=PCXieycqWXc

AThousandShips commented 10 months ago

I can't tell from the video what the other problems are, if any issues still remain please describe them in text so it's easier to access šŸ™‚

kone9 commented 10 months ago

I can't tell from the video what the other problems are, if any issues still remain please describe them in text so it's easier to access šŸ™‚

I was doing a lot of tests. I found several problems when using properties and registering methods with.GDextension

If you register a property incorrectly, all the properties seen in the editor are broken, meaning not only does the newly registered one not work, but all the others stop working.

It is very important if you register a property incorrectly, nowhere does it notify you that the property is incorrectly registered, if it warns you that a name in the code is incorrect, but it does not notify you that it has to do with the property.

The most important thing of all. When you register a property you have to restart the editor.... This wastes a lot of time, meaning for each new property you register you have to restart the editor, otherwise it won't load. The same thing happens when you want to register a signal, when registering the method, until you restart the editor, the registration of the signal does not work and you cannot connect the signal to the object.

I don't know if I should create another issue here on Github saying that you have to restart the editor when generating the properties and signals with GDextension.

I also wanted to say that to register "packet scene" with GDextension c++ in variant there is no PACKETSCENE data type, you can only use OBJECT, and when loading from the editor it loads all the assets, but the property only works correctly when you choose the scene. I leave code for that Captura de pantalla 2023-12-08 104113 Captura de pantalla 2023-12-08 104229 Captura de pantalla 2023-12-08 104346 Captura de pantalla 2023-12-08 104411

I also want to say that in Vscode there is a plugin called Gmatis that allows you to easily generate getters and setters, but I would like a plugin within VScode that allows you to create godot properties easily, as a shortcut to create it, I may do that in the future. .. I mean, it would be good to develop an official GDextension C++ plugin for both VScode and Visual Studio to help in the development.

I don't know if I should create a new issue documenting all these topics in detail, with video and many images.

AThousandShips commented 10 months ago

These are all very different questions, I'd suggest reading the documentation and asking on the other channels, most of this sounds like support questions, don't see any bugs from what you're describing

For the packed scene you should do: PropertyInfo(Variant::OBJECT, "...", PROPERTY_HINT_RESOURCE_TYPE, "PackedScene"), see here

kone9 commented 10 months ago

These are all very different questions, I'd suggest reading the documentation and asking on the other channels, most of this sounds like support questions, don't see any bugs from what you're describing

For the packed scene you should do: PropertyInfo(Variant::OBJECT, "...", PROPERTY_HINT_RESOURCE_TYPE, "PackedScene"), see here

Yes, they are actually suggestions. Not bugs.

The only thing I do take as a bug is that every time you generate a property or want to reference a signal in GDextension C++ you have to restart the editor and that is a lot of waste of time.

I'll try what he told me I also did not find documentation on how to use packet scene with GDextension c++.

I also want to say that I used GDnative C++ before and now it is much more confusing to register a property.

Thank you very much for answering,

AThousandShips commented 10 months ago

If you have suggestions please open them here šŸ™‚

The documentation for GDExtension is lacking and work is being done on improving it

kone9 commented 10 months ago

If you have suggestions please open them here šŸ™‚

The documentation for GDExtension is lacking and work is being done on improving it

ok, I'm going to do them... at least the one where you don't have to restart the editor every time you add a property. Thank you so much.

AThousandShips commented 10 months ago

That might be a bug, please look for other bug reports, unsure what it might be here (depends on context)