godot_heightmap_plugin icon indicating copy to clipboard operation
godot_heightmap_plugin copied to clipboard

Godot 3.6 breaks the plugin

Open RockyMadio opened this issue 1 year ago • 4 comments

Describe the bug Several scripts will break inside Godot 3.6 stable , such as load() in hterrain_loader script , it seems like more things are affected too. But well, the entire engine feels slugish for some reasons, maybe we should not use it

RockyMadio avatar Oct 09 '24 14:10 RockyMadio

As said in several past issues at this point, it has been years that I'm no longer maintaining the Godot 3 version of that plugin. Godot 4 has been around for a long time now. If you really want to use Godot 3 with an old version of this plugin, you will have to workaround that yourself, maybe also posting an issue on Godot's repo if it broke compatibility with something.

Also, I'm lacking details from your post, but if you tried to use the version of the plugin from the main repo, it will obviously not work in Godot 3. As for the one labelled "3.3" in the asset library, this might be the last version of Godot it was maintained with. I don't recall when exactly the switch to 4 happened; it might work in 3.4 or so, but if Godot broke compatibility with anything later, it's not handled. I noticed the asset library text has become misleading so I changed it to indicate that (hasn't gone through yet)

Zylann avatar Oct 09 '24 17:10 Zylann

I am using 3.5.3 stable right now. I am trying to fix only this part

func load(path, original_path): var res = HTerrainData.new() res.load_data(path.get_base_dir()) return res

which is iniside hterrain_resource_loader, as Godot 3.5 complains about the function named as load(), which might conflict. Can you please let me know which other part of the plugiin uses this function, so i can find them and change accordingly

RockyMadio avatar Oct 09 '24 18:10 RockyMadio

hterrain_resource_loader.gd is actually called by Godot's resource loader, it is not called by the plugin directly. load is a virtual function from the API: https://docs.godotengine.org/en/3.5/classes/class_resourceformatloader.html, so if it were to be renamed in the script, it would not even work anymore. If Godot was changed to complain when load is used as a function name, this is a dysfunctional change (for Godot 3, at least; Godot 4 uses _ prefixes), because that would mean it conflicts with that existing API and breaks its usage.

Zylann avatar Oct 09 '24 18:10 Zylann

The problem is not GDScript compatibility and conflict with a global function, but an incompatible change of Godot API, in 3.6 new required argument no_subresource_cache was added.

  • See godotengine/godot#62408.
  • 3.5: Variant load ( String path, String original_path ) virtual
  • 3.6: Variant load ( String path, String original_path, bool no_subresource_cache ) virtual

Actually, for virtual methods optional arguments do not make sense, it would require conditional passing of arguments by caller. But this really breaks compatibility and we haven't figured out how to handle it even in 4.x (a possible alternative is to use suffixes _2, _3, etc.).

The following change seems to solve the problem:

-func load(path, original_path):
+func load(path, original_path, _no_subresource_cache):

dalexeev avatar Oct 09 '24 21:10 dalexeev