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

Make GDScriptDepSort transitive #13414

Closed

Conversation

ghost
Copy link

@ghost ghost commented Nov 30, 2017

fixes #13353 (hopefully)

GDScriptDepSort now sorts using the number of steps up to the base of the inheritance tree. This will sort the base classes before the things that inherit them which is what the original sort was wanting to do, but is also is transitive.

Testing:
This can be tested by using #13354 with the following test project: SortTest.zip. This bug being triggered does depend on the order of the scripts in the script list. I assume this order depends on the order they are loaded in so you may or may not see the bug with this test project. Incase it is difficult to reproduce with the test project I have also included a partial log of two test runs: log.txt. The bug in the log is that node_a_51.gd is getting sorted (and reloaded) before node_a_41.gd, which is its base class.

GDScriptDepSort now sorts using the number of steps up to the base of the inheritence tree.
@akien-mga akien-mga added this to the 3.0 milestone Nov 30, 2017
@akien-mga akien-mga requested a review from reduz November 30, 2017 19:51
@SaracenOne
Copy link
Member

Just tested this and while this in theory should solve the problem, I'm still getting issues and I'm actually getting null pointer errors now.

@akien-mga
Copy link
Member

Since it was tested as not fully working, and we're going into release freeze for Godot 3.0, I'll reject this PR for now. The issue is still open though and this PR can be reused as a base for a better fix for 3.1 and 3.0.x.

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

Successfully merging this pull request may close these issues.

GDScriptDepSort broken.
2 participants