4.3.beta3 GDScript errors on load of Zylann hterrain plugin: Parser bug: Mismatched external parser.

yythlj commented 4 months ago

Tested versions

4.3.BETA3

System information

WINDOWS 10

Issue description

this plugin can use on godot4.3.beta2 with no error.and when I update my godot to 4.3.beta3.error occur https://github.com/Zylann/godot_heightmap_plugin/issues/456

Bugsquad edit: Copied from other issue.

ERROR: Parser bug: Mismatched external parser.
at: (modules/gdscript/gdscript_analyzer.cpp:909)
SCRIPT ERROR: Parse Error: Cannot infer the type of "i" variable because the value doesn't have a set type.
at: GDScript::reload (res://addons/zylann.hterrain/tools/brush/brush_editor.gd:158)
ERROR: Failed to load script "res://addons/zylann.hterrain/tools/brush/brush_editor.gd" with error "Parse error".
at: load (modules/gdscript/gdscript.cpp:2925)
ERROR: Parser bug: Mismatched external parser.
at: (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
at: (modules/gdscript/gdscript_analyzer.cpp:909)
SCRIPT ERROR: Invalid call. Nonexistent function 'set_terrain_painter' in base 'HBoxContainer (brush_editor.gd)'.
at: set_terrain_painter (res://addons/zylann.hterrain/tools/panel.gd:47)
SCRIPT ERROR: Invalid call. Nonexistent function 'setup_dialogs' in base 'HBoxContainer (brush_editor.gd)'.
at: setup_dialogs (res://addons/zylann.hterrain/tools/panel.gd:25)

Steps to reproduce

add plugin, and open project

Minimal reproduction project (MRP)

add plugin, and open project

Calinou commented 4 months ago

I wonder if this is a regression from https://github.com/godotengine/godot/pull/94025.

cc @dalexeev

akien-mga commented 4 months ago

I can reproduce the issue in 4.3-beta3 and 0918fd2a213021e7bba4765c6398359c729d3fb4.

Here's the full log output after installing the Heightmap Terrain plugin from the assetlib, and attempting to enable it in the Project Settings:

ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
SCRIPT ERROR: Parse Error: Cannot infer the type of "i" variable because the value doesn't have a set type.
          at: GDScript::reload (res://addons/zylann.hterrain/tools/brush/brush_editor.gd:156)
ERROR: Failed to load script "res://addons/zylann.hterrain/tools/brush/brush_editor.gd" with error "Parse error".
   at: load (modules/gdscript/gdscript.cpp:2925)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Parser bug: Mismatched external parser.
   at: resolve_class_member (modules/gdscript/gdscript_analyzer.cpp:909)
ERROR: Compiler bug: unresolved assign.
   at: write_assign_with_conversion (modules/gdscript/gdscript_byte_codegen.cpp:919)
SCRIPT ERROR: Invalid call. Nonexistent function 'set_terrain_painter' in base 'HBoxContainer (brush_editor.gd)'.
          at: set_terrain_painter (res://addons/zylann.hterrain/tools/panel.gd:47)
SCRIPT ERROR: Invalid call. Nonexistent function 'setup_dialogs' in base 'HBoxContainer (brush_editor.gd)'.
          at: setup_dialogs (res://addons/zylann.hterrain/tools/panel.gd:25)

I wonder if this is a regression from https://github.com/godotengine/godot/pull/94025.

I tested and it doesn't seem to be the case (tested with a local revert of that PR).

this plugin can use on godot4.3.beta2 with no error.and when I update my godot to 4.3.beta3.error occur

I can't fully reproduce the fact that 4.3.beta2 has no error though.

While I have the full error list pasted above in 4.3.beta3, in 4.3.beta2 and earlier snapshots I get this error:

ERROR: Compiler bug: unresolved assign.
   at: write_assign_with_conversion (modules/gdscript/gdscript_byte_codegen.cpp:919)

This one is reproducible all the way down to 4.3.dev6 included.

So there's one regression introduced in 4.3.dev6 ("ERROR: Compiler bug: unresolved assign.") and another one introduced in 4.3.beta3. Both would need to be bisected.

Reproduction project (empty project with Zylann hterrain installed): hterraintest.zip

akien-mga commented 4 months ago

Bisected the 4.3.beta3 regression (parser bug in GDScriptAnalyzer) to ffc0d0981b48aaa6372f9859da5d238c825efbad / #92616, CC @rune-scape.

akien-mga commented 4 months ago

Bisected the 4.3.dev6 regression (compiler bug in CodeGen) to 27d7760f41d0c819075db37d3234d2587092e682 / #89990, CC @dalexeev.

HolonProduction commented 4 months ago

For the #92616 regression it comes down to the following:

reduce preload loads a scene
the scene has an attached script
this script is loaded via GDScriptCache::get_full_script
this function calls GDScrip::reload which destructs the parser (and with it all depended parsers and class types that are cached)
When the script is loaded later, it isn't cached and loaded again, thus creating a new ClassNode. The analyzer still has the old class node while the parser ref has the new class node. They have the same path but are different objects thus the parser ref does not have the class from the analyzer which results in the mismatched parser error.

Not sure what the right solution would be though. Maybe this check shouldn't compare the objects but only the path.

rune-scape commented 4 months ago

Maybe this check shouldn't compare the objects but only the path.

it does need to have the exact same parser bc it needs to resolve a specific parser node in memory, hence why theres an error

i want to clarify what should happen in the last 2 bullet points: +GDScriptCache::get_full_script calls GDScript::reload which removes the parser from the cache (dependant parsers will still have a ref to the GDScriptParserRef) +GDScriptAnalyzer::resolve_external_member fetches the correct script from the parsers personal cache and it calls resolve_member on the correct analyzer,

i think the problem is that reduce preload does not add the parser to the cache, so if for some reason that parser does get reloaded, it wouldn't have it in the personal cache and would fetch it from GDScriptCache and see that its not the same parser and error

it uses GDScriptCache::get_shallow_script, then just returns the class node

working on the fix

HolonProduction commented 4 months ago

But isn't the problem that it still has the parser ref in its personal cache but the parser was freed due to the same script being loaded through a packed scene?

rune-scape commented 4 months ago

the parser isn't getting freed, otherwise this would be a segfault i think its just not getting added to the analyzer/parsers personal cache (edit: ok im thinking i might be wrong here, and u might be right the parser might actually be getting cleared when the script is removed)

rune-scape commented 4 months ago

it turned out to just be a delicate procedure of looking for the correct parser, as multiple dependant parsers are currently allowed to to be dependant on different parser trees of the same script

i have a working fix now

akien-mga commented 4 months ago

Bisected the 4.3.dev6 regression (compiler bug in CodeGen) to 27d7760 / #89990, CC @dalexeev.

I moved that bug report to #94654, since #94617 will fix the original report from this issue.

akien-mga commented 4 months ago

Reopening as #94617 was reverted.

godotengine / godot