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

Godot4.4-dev5: ClassDB provides classes AnimationNodeStartState and AnimationNodeEndStatewhere which are not registered #99534

Open
MikeSchulze opened this issue Nov 22, 2024 · 4 comments · May be fixed by #102433

Comments

@MikeSchulze
Copy link

Tested versions

v4.4.dev5.mono.official [9e60984]

System information

all

Issue description

I iterate over all classes provided by ClassDB for testing. Using ClassDB:get_class_list()
With Godot4.4.dev5 there is now “AnimationNodeStartState”, “AnimationNodeEndState” there are not documented and cannot be instantiated.

Steps to reproduce

Can be verified by

extends Node

func _ready() -> void:
	prints("AnimationNodeStartState class_exists:", ClassDB.class_exists("AnimationNodeStartState"))
	prints("AnimationNodeStartState can_instantiate:", ClassDB.can_instantiate("AnimationNodeStartState"))

        # compile error
	AnimationNodeStartState.new()

output:

AnimationNodeStartState class_exists: true
AnimationNodeStartState can_instantiate: true

errors:

 res://test.gd:7 - Parse Error: Native class GDScriptNativeClass used in script doesn't exist or isn't exposed.
  res://test.gd:7 - Parse Error: Static function "new()" not found in base "GDScriptNativeClass".

Minimal reproduction project (MRP)

n/a

@akien-mga akien-mga changed the title Godot4.4-dev5: ClassDB provides classes where not exists/accessible Godot4.4-dev5: ClassDB provides classes AnimationNodeStartState and AnimationNodeEndStatewhere which are not registered Nov 22, 2024
@akien-mga akien-mga added this to the 4.4 milestone Nov 22, 2024
@akien-mga
Copy link
Member

CC @TokageItLab @RandomShaper, see also #98963 which registered those classes as internal.

@akien-mga akien-mga moved this from Unassessed to Release Blocker in 4.x Release Blockers Nov 22, 2024
@TokageItLab
Copy link
Member

They are "currently" only used to make decisions to prevent editing, so I believe they can actually be replaced by the AnimationNode. However, they will need to be exposed again in the future when we allow additional ports (see also #88878 (comment)), but until then they are not necessary.

@dsnopek
Copy link
Contributor

dsnopek commented Dec 10, 2024

output:

AnimationNodeStartState class_exists: true
AnimationNodeStartState can_instantiate: true

I feel like this indicates a bug somewhere. These classes are registered via GDREGISTER_INTERNAL_CLASS(), which I would have expected to make can_instantiate() return false

UPDATE: Did a little more poking around!

Internally, we use ClassDB::_can_instantiate() to check if we really can instantiate a class - this should return true for an internal class, because we need to be able to instantiate one for internal classes to work from GDExtension.

But, for the public ClassDB::can_instantiate() it would make logical sense to return false - but I'm not sure if that would maybe break something else somewhere? If it would, then maybe we need to add a ClassDB::is_exposed() function so its possible to check if a registered class is internal?

@dsnopek
Copy link
Contributor

dsnopek commented Feb 4, 2025

I've spent a little more time digging into this, and I've gone in circles a little bit. :-)

The main question is: do we want ClassDB::instantiate() to work for classes that aren't exposed?

Because currently:

func _ready() -> void:
	# Error
	AnimationNodeStartState.new()

	# But this works!
	ClassDB.instantiate("AnimationNodeStartState")

So, these are the two options:

  1. We don't want to ClassDB::instantiate() to work for unexposed classes: then we add a check to ClassDB::_can_instantiate() to return false if the class isn't exposed. Then expectations match reality - ClassDB says we can't instantiate it, and we can't instantiate it.
  2. We want ClassDB::instantiate() to continue working for unexposed classes: then we expose ClassDB::class_is_exposed() to scripting, and folks need to know that GDScript can't create unexposed classes the normal way, and you would need to use ClassDB::instantiate(). So, this way there's an indication of this special case.

I think option nr 1 is the cleanest, so long as that change doesn't break anything.

It would have no impact on GDExtension (I was wrong about what I wrote above), because classes that aren't exposed already can't be used from GDExtension (aside from the same ClassDB::instantiate("...") workaround). Other than users directly using ClassDB::instantiate(), I think this would only potentially affect loading .tscn and .tres files - are there any unexposed resources we allow to be saved in those files?

Here's PR #102440 that implements option nr 1

If PR #102433 is enough to fix this for all classes that exist at the moment, we should go with that for Godot 4.4. And, if no one thinks of a case that my PR would break, we can try that early in Godot 4.5's dev.

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