godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.99k stars 21.16k forks source link

Inner classes functions should appear in functions list panel #19386

Closed jahd2602 closed 6 years ago

jahd2602 commented 6 years ago

Godot version: 3.0.2 stable

OS/device including version: MacBook Air (13-inch, Early 2014) (1,4 GHz Intel Core i5) MacOS High Sierra 10.13.3

Issue description: Gdscripts can contain classes, that can contain functions. The functions panel of the code editor is not showing the classes nor the classes' functions.

How it should appear

Steps to reproduce:

  1. Create a gdscript with a class and a function inside that class (an inner function).
  2. The class and the inner function don't appear in the functions panel. (the class may or may not appear, but the inner function should definitely appear)
Zylann commented 6 years ago

Ignore this message, see later messages


Keep in mind scripts are classes already. We call them scripts because they fit in a file and their name is the name of the file. When you write class in a GDScript, you actually define a class within a class. As such, they have no magical access to their enclosing instance. It's like they are in a namespace. You could have defined the inner class as another script, it would have been the same.

Here, the editor is not showing functions of the inner class because it makes no sense. When you add a script to a node, that node will get an instance of that script. Not the inner class. So you can't call functions on that inner class because the script on the node is not an instance of that inner class.

jahd2602 commented 6 years ago

I understand that those are inner classes. But still, they are supported in gdscript, the code is there, so that's why its functions should appear in the functions panel/navigator.

It can look like this:

InnerClass:
    innerFunction1
    innerFunction2
function1
function2

or like this:

InnerClass.innerFunction1
InnerClass.innerFunction2
function1
function2
OvermindDL1 commented 6 years ago

The issue is that 'how' would it call it? You need to call those functions (unless static) on an 'instance' of the class, so how would you define that instance if it is, say, stored somewhere else?

jahd2602 commented 6 years ago

Yes, you can instance that inner class and use the inner functions without problems. I am actually using them without problems. They just don't appear in the panel/navigator.

Here an usage example: https://github.com/jahd2602/godot-inventory/blob/master/inventory.gd

Zylann commented 6 years ago

Oooh hang on I completely misunderstood the use case... 🤦‍♂️ You don't want to call them. You want them to show up in the list of functions in the script editor, and that's indeed a sensible request. Maybe that panel could be promoted as an "outlining" of the current script and show functions, and classes as folders so we can see functions in them, like you said.

jahd2602 commented 6 years ago

What are some of the files that should be modified to accomplish that?

guilhermefelipecgs commented 6 years ago

What are some of the files that should be modified to accomplish that?

Here's what you're looking for: https://github.com/godotengine/godot/blob/6f328420c3f62726c2264d5d3725a48d57f218e1/editor/plugins/script_editor_plugin.cpp#L1429-L1451

jahd2602 commented 6 years ago

I have been diving into the code, and it looks like a new for loop is needed here https://github.com/godotengine/godot/blob/6f328420c3f62726c2264d5d3725a48d57f218e1/modules/gdscript/gdscript_editor.cpp#L110-L119 But that would only change gdscript, there should be similar changes for all supported languages right? nativescript.cpp visual_script.cpp etc...

vnen commented 6 years ago

But that would only change gdscript, there should be similar changes for all supported languages right?

Not really. Only GDScript has the concept of inner classes. Unless you want to change this to show the whole outline as well.

Zylann commented 6 years ago

C# also has inner classes. I think that depends on whatever parses the edited script, and if that doesn't return nested symbols, then the panel just won't show them.

jahd2602 commented 6 years ago

I could get this to work fine: https://github.com/jahd2602/godot/commit/9df6e7e20ffa20b00371a84d687622e5680cacae

It is using this notation:

InnerClass.innerFunction1
InnerClass.innerFunction2
function1
function2

and it looks like this:

I am working on top of 3.0.2 because after that point, master is very buggy in macOS. Should I pull request branch 3.0.2?

guilhermefelipecgs commented 6 years ago

Should I pull request branch 3.0.2?

I don't think so. It's better to send to master.

guilhermefelipecgs commented 6 years ago

Maybe you can open an pr from your 3.0.2 to master. CC @akien-mga

akien-mga commented 6 years ago

No, pull requests should be made from a branch synced with master and to the master branch. You can do something like:

git checkout master -b inner-classes-list
git pull --rebase upstream master
git cherry-pick 9df6e7e

to apply your commit 9df6e7e on top of the master branch in a new feature branch.