Zylann / godot_voxel

Voxel module for Godot Engine
MIT License
2.68k stars 251 forks source link

Custom VoxelGenerator in C++ #363

Closed MadMartian closed 2 years ago

MadMartian commented 2 years ago

If I want to implement my own custom VoxelGenerator in C++, does it need to be published and loaded as a Godot editor plugin? is GDNative insufficient? In my case I can get Godot to see and load my GDNative class, but I have no idea how to get it to show-up in this list, hence I wonder if my trouble is that my custom code must be published as a Godot editor plugin: image

Zylann commented 2 years ago

If you use Godot 3, the flow with GDNative is the same as with GDScript. You create a VoxelStreamScript and attach your script to it. I don't think it will directly appear in that list AFAIK, or maybe you should give it a class_name (not sure if that works correctly tho).

Also... not sure why the dropdown shows you VoxelStreams AND VoxelGenerators, why it's not showing VoxelGeneratorScript, and why it's showing VoxelStream and VoxelGenerator which are both abstract classes Oo

Which version of the module are you using actually? Because one possible reason is you are using a very old one...

MadMartian commented 2 years ago

Should be revision at 603357eb, matching Godot v3.2.3. We plan to migrate to Godot v4 at some point when it stabilizes.

Zylann commented 2 years ago

Yeah it's a very old version, not sure how you'd go for this one. If you only want to stay in Godot 3 you could upgrade to the godot3.x version which is the most up-to-date-but-not-godot4 version, in which making a generator script goes as I said earlier.

MadMartian commented 2 years ago

Well I've upgraded to the 3.x branch and now I'm getting a different error:

E 0:00:04.151   instance: Class 'VoxelGenerator' or its base class cannot be instantiated.
  <C++ Error>   Condition "!ti->creation_func" is true. Returned: nullptr
  <C++ Source>  core/class_db.cpp:512 @ instance()
  <Stack Trace> VoxelLodTerrain.gd:10 @ _ready()

This is what my GDNative C++ class looks like:

namespace godot {
    class GeoWorldVoxelGeneratorSource : public VoxelGenerator {
    private:
        GODOT_CLASS(GeoWorldVoxelGeneratorSource, VoxelGenerator);

    private:
        String                  inputFile;
        geoworld::FileSource *  fileSource;

    public:
        static void _register_methods();

        GeoWorldVoxelGeneratorSource();
        virtual ~GeoWorldVoxelGeneratorSource();

        void _init();

        void setInputFile(String inputFile);
        String getInputFile();

        virtual void generate_block(Ref<VoxelBuffer> out_buffer, Vector3 origin_in_voxels, int64_t lod);
    };    
}

I get this error after attempting to instantiate the above GDNative class in a GDScript.

And then I noticed something peculiar in the GODOT_CLASS macro, shouldn't it return the name of the subclass, not the base class? should I file an issue against the Godot engine do you think?:

        ...
    inline static const char *___get_godot_class_name() {                                    \
        return Base::___get_godot_class_name();                                              \
    }
        ...
Zylann commented 2 years ago

You need to extend VoxelGeneratorScript, not VoxelGenerator.

And then I noticed something peculiar in the GODOT_CLASS macro, shouldn't it return the name of the subclass, not the base class?

I think it's correct, ___get_godot_class_name is supposed to return the most derived Godot class (i.e from Godot) that your custom class extend from. There is another static function to return your class name instead.

MadMartian commented 2 years ago

Getting closer! my custom implementation is still not being called though.

image

My class is being recognized by Godot now. Since my class' voxel generator implementation is not being called yet I am suspicious that my code is not overriding the right method. I noticed that the VoxelGeneratorScript class has a different signature for the generator method than the VoxelGenerator class...

VoxelGenerator:

void generate_block(Ref<VoxelBuffer> out_buffer, const Vector3 origin_in_voxels, const int64_t lod)

VoxelGeneratorScript:

void _generate_block(Ref<VoxelBuffer> out_buffer, const Vector3 origin_in_voxels, const int64_t lod)

In my class GeoWorldVoxelGeneratorSource I am not sure which one to override so I have tried overriding either of them to test and see if I get any behavior changes. The way I am testing to see if my code is called is by using Godot::print(...), which I have seen working before elsewhere, but nothing is showing-up in the Godot console despite which method(s) I override.

My implementation is really very simple right now, just to push logs to the Godot console to prove that it's being called:

    void GeoWorldVoxelGeneratorSource::_generate_block(Ref<VoxelBuffer> out_buffer, Vector3 origin_in_voxels, int64_t lod) {
        // TODO: Synchronize
        Godot::print("(BEGIN) origin = <" + origin_in_voxels + ">, LOD = " + lod);

        const int64_t 
            width = out_buffer->get_size_x(), 
            depth = out_buffer->get_size_z(), 
            height = out_buffer->get_size_y();

        for (int64_t k = 0; k < depth; ++k)
            for (int64_t j = 0; j < height; ++j)
                for (int64_t i = 0; i < width; ++i)
                    out_buffer->set_voxel(
                        0, 
                        origin_in_voxels.x + i, 
                        origin_in_voxels.y + j, 
                        origin_in_voxels.z + k
                    );

        Godot::print("(END) origin = <" + origin_in_voxels + ">, LOD = " + lod);
    }

I also notice there's no virtual keyword in-front of these methods, which raises questions how the caller calls the correct sub-class implementation without a v-table.

Zylann commented 2 years ago

I noticed that the VoxelGeneratorScript class has a different signature for the generator method than the VoxelGenerator class... In my class GeoWorldVoxelGeneratorSource I am not sure which one to override

The one you have to implement is _generate_block, with a _, like many other methods in Godot. The names are different from the "method that is callable" on purpose. One calls the other internally. It's also shown as virtual in the doc to indicate that: https://voxel-tools.readthedocs.io/en/latest/api/VoxelGeneratorScript/

void _generate_block(Ref<VoxelBuffer> out_buffer, const Vector3 origin_in_voxels, const int64_t lod)

I don't see this signature anywhere. Maybe only in the doc, but not in C++.

The way I am testing to see if my code is called is by using Godot::print(...)

You could try to use a C++ debugger as well and put breakpoints. Keep in mind also _generate_block is going to be called by multiple threads at the same time. And I read people in Discord having problems with the print function.

I also notice there's no virtual keyword in-front of these methods

Where? In Godot 4 it definitely is. In Godot 3 I think it used to not be declared as such explicitely, but the call definitely exists and has been used notably in the blocky game demo project. See the C++ code calling it in the godot3.x branch, you can see the method bound as virtual here https://github.com/Zylann/godot_voxel/blob/6288eb64b7eabeba25d629707b040db34d30c73a/generators/voxel_generator_script.cpp#L39-L42 And called here (you can try putting a breakpoint here to see it called). generate_block internally calls _generate_block. https://github.com/Zylann/godot_voxel/blob/6288eb64b7eabeba25d629707b040db34d30c73a/generators/voxel_generator_script.cpp#L16 And here is your v-table: https://github.com/Zylann/godot_voxel/blob/6288eb64b7eabeba25d629707b040db34d30c73a/generators/voxel_generator_script.h#L13

MadMartian commented 2 years ago

I put some logging statements in the Voxel module source, dumping things like methods and properties, and I've found the reason it's failing is because it's looking for a _generate_block on my ScriptInstance object, but claims there are no methods on it. I think it has the correct object though because there is an InputFile property on it.

Do I need to explicitly register the _generate_block and _get_used_channels_mask methods in my sub-class?

    void GeoWorldVoxelGeneratorSource::_register_methods() {
        register_property<GeoWorldVoxelGeneratorSource, String>("InputFile", &GeoWorldVoxelGeneratorSource::setInputFile, &GeoWorldVoxelGeneratorSource::getInputFile, "default.pgw");

        // Do I need this?
        // register_method("_generate_block", &GeoWorldVoxelGeneratorSource::_generate_block);
    }

EDIT: It appears I do need to manually register the methods, because now it's finally working! but is that the correct way to do it?

Zylann commented 2 years ago

EDIT: It appears I do need to manually register the methods, because now it's finally working! but is that the correct way to do it?

I am not aware of C++ automagically registering methods to Godot. AFAIK this has always ever needed to be registered manually. It might be a bit different in Godot 4, but I haven't tinkered with GDExtension in a while.

MadMartian commented 2 years ago

Ok great, if this makes sense to you then I think we can close this issue. I hope that this issue / resolution will help someone else down the road if they run into similar confusion. Thanks for your help!