search

Kubulambula / Godot-GDShell

Light-weight, powerful, customizable and modular in-game console for Godot 4.
https://godotengine.org/asset-library/asset/1526
MIT License
213 stars 10 forks source link

`ls` (done) and `scn` commands. #6

Open tryoxiss opened 1 year ago

tryoxiss commented 1 year ago

scn is used to edit the current scene (e.g., instancing nodes, freeing nodes). ls is used to view the current scene tree. I have some docs articles here to give an idea of what i'm thinking but feel free to suggest changes!

Kubulambula commented 1 year ago

The ls command is great especially with the #15 proposal.

With the proposal however, I would split the scn command into separate specialized commands like instance and free instead of parameters. I'm not sure if we should do the kill command then or just to have a flag for free to use free() instead of queue_free() - discussion for the future.

tryoxiss commented 1 year ago

I defintely agree with the seccond point, there could be like a --now or -N flag.

For the first one I defintely see what your saying, and thinking about it now I do think I agree with it. Maybe call it instance or ins for short maybe? Maybe its just a me thing but I don't like typing out commonly used commands a lot when they are long, so shorter aliases are worth thinking about I think.

Kubulambula commented 1 year ago

  1. The --now or -N flags are not really that descriptive about what it does. Maybe the free command would do free() by default and with --queue or -Q flag it would do queue_free()?

  2. Well we could call it instance and have it register with an ins alias. That's not really an issue.

tryoxiss commented 1 year ago

I think --now is fairly descriptive, and we defintely don't want free() to be the default as queue_free() will be used a lot more and is what should be used. Maybe --ignore-active-actions or something for free() instead of queue_free()? Not sure.

I think I am going to try and make these, not sure how far I will get though. Nevermind, literally nothing works. Not even the default UI scene is valid apparently (according to both godot 3.5 and 4.0).

Kubulambula commented 1 year ago

I think --now is fairly descriptive, and we defintely don't want free() to be the default as queue_free() will be used a lot more and is what should be used. Maybe --ignore-active-actions or something for free() instead of queue_free()? Not sure.

I definitely agree that queue_free() should be the default behavior. Also note that Object does not have the queue_free() method as it is on the inherited Node class. This might cause some issues in the future.

But for the flag: --now is definitely better than having free() as the default. Maybe we could use --force, --unsafe, or just simply --free with --queue_free and --queue_free will be implicit default?

I don't know honestly. Maybe we should stick with --now

Nevermind, literally nothing works. Not even the default UI scene is valid apparently (according to both godot 3.5 and 4.0).

What do you mean? I just downloaded the main branch and it works fine for me in 4.0.stable

Can you share the error?

tryoxiss commented 1 year ago

Yeah I think --force and --unsafe are defiontely better than what I got! and yeah queue_free being the implcit default would be good.

Well is there anything stopping us from having multiple flags mean the same thing? I think now, force, and unsafe are all good and descriptive enough and its hard to chose any one.

Ahh, I added it to an existing project so I could more easily test the scenes and assumed it wouldn't have issues, though I don't think the errors are related to that. (Looking at the format, I don't even think it would work with just that as theres no project.godot), I also created a new project with just that and got the same errors.

Errors:

  1. Unknwon class: "GDShellCommand"
  2. plugin.gd: Parse error: Unexpected "@"
  3. plugin.gd (when I remove that): the identifier GDSHellMain isn't declared in the current scope
  4. default_ui.tscn: Error while parsing default_ui.tscn
Kubulambula commented 1 year ago

Yeah I think --force and --unsafe are defiontely better than what I got! and yeah queue_free being the implcit default would be good.

Well is there anything stopping us from having multiple flags mean the same thing? I think now, force, and unsafe are all good and descriptive enough and its hard to chose any one.

Good idea! I think we could have --safe and --unsafe with --now or --force as an alias for --unsafe as it is not a default.

Ahh, I added it to an existing project so I could more easily test the scenes and assumed it wouldn't have issues, though I don't think the errors are related to that. (Looking at the format, I don't even think it would work with just that as theres no project.godot), I also created a new project with just that and got the same errors.

Yeah the project.godot is in .gitignore so if you want to run GDShell project itself, you have to create it manually (I should probably change that)

But the errors are very strange. Maybe try to delete the .godot/ directory inside your project and let Godot to reimport all the classes. Also what Godot version are you using?

CollCaz commented 1 year ago

So i tried implementing the ls command and, couldn't find a way to see if a node is an instanced scene or just a "stray" node. Any idea how i could do that?

tryoxiss commented 1 year ago

So i tried implementing the ls command and, couldn't find a way to see if a node is an instanced scene or just a "stray" node. Any idea how i could do that?

I believe there is a is_instance() method, but that may be my imagination. A ask.godotengine.org thread suggests trying the get_filename() method, if its a scene it returns a file, if its a stray node it returns none if I am understanding correctly.

Best luck on implementing!