Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

class_name issue when using (compressed) binary tokens in pck #88796

Closed
wpbirney opened this issue Feb 25, 2024 · 11 comments · Fixed by #89005
Closed

class_name issue when using (compressed) binary tokens in pck #88796

wpbirney opened this issue Feb 25, 2024 · 11 comments · Fixed by #89005

Comments

@wpbirney
Copy link

Tested versions

  • Reproducible in Godot v4.3.dev (2e7fc81)

System information

Godot v4.3.dev (2e7fc81) - Void #1 SMP PREEMPT_DYNAMIC Wed Feb 7 19:24:35 UTC 2024 - Vulkan (Forward+) - dedicated AMD Radeon RX 7900 XTX (RADV NAVI31) () - AMD Ryzen 9 5950X 16-Core Processor (32 Threads)

Issue description

I have a scene with a script like so

class_name EnemyCharacter

extends CharacterBody3D

When running from the editor OR when the pck is exported with gdscript in text format not binary tokesn my Bullet class with the following works fine

func _on_body_entered(body):
    var b = body as EnemyCharacter
    if b:
        b.damage(xxx)

b is not null and it executes correctly.

However when i export using binary tokens this fails and returns null even though body.get_script() returns EnemyCharacter.

I cannot seem to narrow the issue down as I am using this same style of cast and it is working in other situations. I have item pickups working the same way without issue with either binary tokens or plain text. The only difference i see is the ones that are working have Node3D parents. While the one that is not working with binary tokens EnemyCharacter has a CharacterBody3D parent and Bullet has a RigidBody3D parent.

I believe it to be a bug simply because it works correctly with gdscript exported as Text or when run in the editor.

Steps to reproduce

Haven't been able to make a simple reproducible without including all of my games scripts.

  • Create EnemyCharacter class for a CharacterBody3D root scene
  • Create a RigidBody3D parented scene for a bullet
  • In Bullets func _on_body_entered(body): cast body to EnemyCharacter and check if null
  • will always return null if exported with Binary Tokens even though get_script() returns EnemyCharacter
  • Cast will work correctly with Text export or Editor run

Minimal reproduction project (MRP)

N/A

@dalexeev
Copy link
Member

See also #88353 (not a duplicate) and #88365. I'm guessing we forgot to fix path comparisons in GDScriptParser::DataType and a few other places.

@wpbirney
Copy link
Author

That seems to be the case because i am getting:

ERROR: Another resource is loaded from path 'res://Controllers/EnemyController.gd' (possible cyclic resource inclusion).

Which i did not notice at the time of filing the issue

@dalexeev
Copy link
Member

dalexeev commented Feb 26, 2024

I can't reproduce the bug described in the first post. Please provide MRP. Also could you please check if #88853 fixes your issue.

@wpbirney
Copy link
Author

I can't reproduce the bug described in the first post. Please provide MRP. Also could you please check if #88853 fixes your issue.

#88853 does not seem to resolve the issue. I'm working on creating an MRP today. Having trouble reproducing it other then in my project. Still believe it to be a bug since it works correctly with my project with Text Export

@eswartz
Copy link
Contributor

eswartz commented Feb 27, 2024

I am seeing this issue too... but also with a large-ish project. Similar results and workarounds as the OP. [on master, a586e86]

In my case, I have two .gd files mentioned as possible cyclic resources. I'm pretty sure they do depend on each other, indirectly at least, since one is an autoload file with some globals and the other is a core game level script whose callees also use that autoload.

This MRP shows the "cyclic resource" error in the exported project BUT does not crash. Let me know if you want more.

ScriptCycle.zip

@eswartz
Copy link
Contributor

eswartz commented Feb 27, 2024

This is an MRP which actually shows the bug and the bad results. The last error (invalid type) is evidence of the issue affecting real code.

ScriptCyclesMRP.zip

It's got no functionality whatsoever other than the kind of dynamic scene and script loading that triggers the issue for me.

When run under the editor, you should see nothing but a grey window. Normal output is:

level path: res://levels/zero/level_0_0.tscn
Level

When running the exported project, you'll see:

SCRIPT ERROR: Parse Error: Could not preload resource file "res://game/level.tscn".
          at: GDScript::reload (res://game/game.gd:9)
SCRIPT ERROR: Compile Error: 
          at: GDScript::reload (res://game/level.gd:-1)
SCRIPT ERROR: Compile Error: 
          at: GDScript::reload (res://levels/level_descriptor.gd:-1)
SCRIPT ERROR: Parse Error: Could not preload resource file "res://game/level.tscn".
          at: GDScript::reload (res://game/game.gd:9)
SCRIPT ERROR: Compile Error: 
          at: GDScript::reload (res://game/level.gdc:-1)
ERROR: Failed to load script "res://game/level.gdc" with error "Compilation failed".
   at: load (modules/gdscript/gdscript.cpp:2824)
ERROR: Another resource is loaded from path 'res://game/level.gd' (possible cyclic resource inclusion).
   at: set_path (core/io/resource.cpp:75)
ERROR: Another resource is loaded from path 'res://game/game.gd' (possible cyclic resource inclusion).
   at: set_path (core/io/resource.cpp:75)
level path: res://levels/zero/level_0_0.tscn
Level
SCRIPT ERROR: Invalid type in function 'setup' in base 'Node (Level)'. The Object-derived class of argument 1 (Node (Game)) is not a subclass of the expected argument class.
          at: Game.set_level (res://game/game.gd:66)

@wpbirney
Copy link
Author

wpbirney commented Feb 27, 2024

The only error im getting is

ERROR: Another resource is loaded from path 'res://Controllers/EnemyController.gd' (possible cyclic resource inclusion).
   at: set_path (core/io/resource.cpp:75)

And then any cast to EnemyCharacter (defined in class_name of EnemyController) fails only when exported as binary tokens. Text is working perfect. I'm going to see if it has anything to do with the global script ive added recently. Because i do not recall that issue prior to that

Not sure of a good way to figure out the issue as its working great with text export. Any ideas on troubleshooting this?

@eswartz
Copy link
Contributor

eswartz commented Feb 27, 2024

Not sure of a good way to figure out the issue as its working great with text export. Any ideas on troubleshooting this?

AFAIK the binary script export is a new feature recently added (and made default) on master. It's not present (or exposed?) in 4.3dev3. (I see raw text in the functioning exported binaries built with that engine/template). When the error is reported, it means problems will occur later on, presumably when creating Nodes using the mentioned types.

I was able to successfully export projects as of a week or so ago and also only saw it break recently without any structural changes in the area of code that now reports the cyclic dependencies. So, given all that, IMHO, there's nothing else you need to do. My MRP demonstrates the bug, I think.

@dalexeev
Copy link
Member

@eswartz I fixed the issue with your MRP in #89005. Please test if this fixes the bug in your project.

@wpbirney Please try testing with #89005. There is a chance that there is another kind of regression going on, not related to script paths (though I'm guessing that's the reason).

If the bug is still reproducible, then we need a MRP. If you can't create it from scratch, then you can try another approach. Make a copy of your project and iteratively remove parts that you think are not related to the bug. Continue until the project is small enough to debug. If the bug disappears at some point, go back to the previous step and try to remove something else.

@eswartz
Copy link
Contributor

eswartz commented Feb 29, 2024

@dalexeev Yes, that PR (or master, since it was merged a minute before I saw this) works properly in my actual game. Thanks!

@wpbirney
Copy link
Author

@dalexeev #89005 on current master is working flawlessly with my exported game with compressed binary tokens now!

Thank you! Trying to familiarize myself with the godot source so i can contribute to some fixes as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants