Zylann/godot_heightmap_plugin

Terrain importer broken in version 1.6

McSpider opened this issue · 7 comments

Describe the bug
Terrain importer not working.

To Reproduce
Attempting to import terrain using the Terrain>Import maps... menu raises several Invalid call. Nonexistent function errors.

Cause
Errors are somewhat nonsensical. Fix for me was to remove : HT_Logger static typing from the following lines:


static func _load_image_size(path: String, logger: HT_Logger) -> Dictionary:

I'm not sure why this is an issue and if removing it is the correct course of action, but doing so allows me to use the importer.

Environment
Windows 10
Godot 3.3.3 to 3.4.4
Plugin version 1.6

Other Notes
Not tested in Godot 3.3.2 or lower due to the add-on using .gdshader files, which weren't introduced until 3.3.3.

Fix for me was to remove : HT_Logger static typing from the following lines:

This is indeed complete nonsense... I can't fix that this way, and you should not need to. This sort of typing is used everywhere in the plugin, even in other functions of that same file. Godot is buggy here.

I tried testing import with 3.4.4 and had no such problem. So if you still face this problem, you may be in a particular case that you need to describe (like how you get there from scratch).

Did you do this after installing the plugin for the first time without restarting the editor?
If you rather updated it from an old version, I recommend turning off the plugin, closing the editor, deleting the plugin, putting in the new version, starting again the editor and then turning the plugin back on. Installing "over" while the editor runs does not cleanup old files and Godot might fail to reload things properly.

So I just did a clean download of Godot 3.4.4, new project, installed add-on through AssetLib, restarted Godot, activated the addon and setup a test scene. Same problem. Restarted Godot, still broken. Then I repeated the steps above but got the addon directory by cloning it from GitHub. Same errors:

 res://addons/zylann.hterrain/tools/importer/importer_dialog.gd:260 - Invalid call. Nonexistent function 'debug' in base 'Nil'.
 res://addons/zylann.hterrain/tools/importer/importer_dialog.gd:189 - Invalid call. Nonexistent function 'has' in base 'Nil'.
 res://addons/zylann.hterrain/tools/importer/importer_dialog.gd:87 - Invalid get index 'errors' (on base: 'Nil').

Also tested this in two different machines a Mac and a Windows PC, both behaved the same.

I did check around some more, the only cases where you're static typing the HT_logger is in importer_dialog.gd

And then I noticed that in logger.gd two classes are defined; HT_LoggerBase & HT_LoggerVerbose so I tried changing the : HT_Logger to : HT_Logger.HT_LoggerBase and that makes everything work fine. I believe this is because in Godot, scenes and scripts are inherently classes. So doing HT_Logger.get_for(self) actually returns HT_LoggerBase or HT_LoggerVerbose which can be accessed using HT_Logger.HT_LoggerBase in the importer script.

I feel like this somewhat makes sense aside from the fact that it works for you without modification.

And then I noticed that in logger.gd two classes are defined; HT_LoggerBase & HT_LoggerVerbose

Oh, that would be that then. So indeed the static typing in this specific case must be changed or removed because it's not the right class.
Yet... I think Godot is still buggy for not raising that all along when I tested this xD But I guess that's the downsides of Godot 3 vs 4.
I believe I didnt get the error because """""""""static"""""""" typing didn't check anything when the wrong logger type was passed to the function. Worse: it apparently turned it into null. So errors only trigger when a function is called on it. Which... did not happen for me, because I tried importing a PNG without errors, so the logger wasn't called. You must have been importing something else like a RAW (which does print debug stuff), or had errors/warnings.

Yeah Godot 3 could definitely do better with the error messages. I was indeed importing RAW files, didn't even occur to me to try a PNG... Would you like me to open up a pull request for changing HT_Logger to HT_Logger.HT_LoggerBase?

I'm currently doing the changes, preparing for a 1.6.1 release

It should be fixed in 6f269ad. I didnt static-type it for consistency with the rest of the codebase where the logger is used.
I submitted an edit to the AssetLibrary so 1.6.1 should be available there later.

Cool, thanks for the help in getting this figured out.