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

Don't allow removing scripts in nodes of inherited scenes #2670

Closed
Shadowblitz16 opened this issue Apr 29, 2021 · 18 comments
Closed

Don't allow removing scripts in nodes of inherited scenes #2670

Shadowblitz16 opened this issue Apr 29, 2021 · 18 comments

Comments

@Shadowblitz16
Copy link

Describe the project you are working on

Nothing now

Describe the problem or limitation you are having in your project

Every time I use Godot I have issues because I can't bind a script to a node in a scene and provide a logical interface for it.

Right now any script can be removed even if another scene relies on it.

I have made many issue on this topic but I always have a hard time explaining why it should be done.

Describe the feature / enhancement and how it helps to overcome the problem

Can or will there ever be a possibility to lock a script onto a node and force inherited scenes to either leave it or extend it?

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

extends Node

func some_needed_function():
    pass
extends Node
func some_caller_function():
  get_node("MyNeededNode").some_needed_function()

Then you would mark a script as locked on a node through the gui.

If this enhancement will not be used often, can it be worked around with a few lines of script?

it would be used to provide a interface to scene nodes,
also not as I can see I have browsed plugins and I can't find one that could do this will all script types.

Is there a reason why this should be core and not an add-on in the asset library?

It allows people to add logic to nodes or scene that is needed for something to execute.
right now you can do this but people can remove the script and attach a new one which can break code that depends that the scene or node has that logic.

@YuriSizov
Copy link
Contributor

YuriSizov commented Apr 29, 2021

I have made many issue on this topic but I always have a hard time explaining why it should be done.

If you want to ask a question to the devs, there is Godot Contributors Chat. Alternatively, monthly Q&As, like the one we host at this very moment, is a place to ask a question. Though, Q&A requires you to ask your question in a short and clear form, so chat is likely preferable if you have problems with formulating your ideas.

This is not a place to ask a question, so I have to close this.

@Shadowblitz16
Copy link
Author

@pycbouh I was told by @Calinou to post it here

@KoBeWi
Copy link
Member

KoBeWi commented Apr 29, 2021

Can or will there ever be a possibility to lock a script onto a node and force inherited scenes to either leave it or extend it?

Why though? What does it solve? If it's for your code, then it's a non-problem, because you should know that the script should be inherited. If it's for plugin, just state it in readme and if someone breaks their scene, it's their fault.

You keep describing the solution for your problem, but it's hard to tell what problem does it actually solve.

@Shadowblitz16
Copy link
Author

Shadowblitz16 commented Apr 29, 2021

@KoBeWi
Basically its the same reason we don't allow nodes to be removed from inherited scenes in Godot because they provide a interface we can use.

It encapsulates the interface and guarantees it's there for other things to use it.

How do I guarantee that my BulletSpawner Node or Scene can spawn bullets if the script can be removed?
This isn't possible in Godot right now because node's don't hold logic they hold scripts that contain logic

Something like this would fix that issue

What I am confused about is why I have to explain this to professional c++ devs. you guys obviously know its important for a interface to be there to use.

I may not be as experienced as you in programming but I know when there is a usability flaw

@KoBeWi
Copy link
Member

KoBeWi commented Apr 29, 2021

Basically its the same reason we don't allow nodes to be removed from inherited scenes in Godot

It's not the same reason. It's just easier to not allow removing nodes from inherited scenes. Changing this would require changing tscn format to support "removed nodes".

How do I guarantee that my BulletSpawner Node or Scene can spawn bullets if the script can be removed?

Don't remove the script then? Without script it's no longer BulletSpawner.

In any case, I think I know what this proposal is about, so I'll just reopen it and edit the title.

@KoBeWi KoBeWi reopened this Apr 29, 2021
@KoBeWi KoBeWi changed the title What are the plans on providing a interface Don't allow removing scripts in nodes of inherited scenes Apr 29, 2021
@Shadowblitz16
Copy link
Author

@KoBeWi thankyou, but do you really think allowing people to modify inherited scene nodes is a good idea?

I think it much easier for users if you can guarantee that the logic and nodes they want there are going to be there.
it enforces type safety and good programming as well as helps out alot when people want to use static typing

@KoBeWi
Copy link
Member

KoBeWi commented Apr 29, 2021

do you really think allowing people to modify inherited scene nodes is a good idea?

Well, I'm not really a fan of telling someone how they should work with their project. While removing scripts from inherited scenes isn't very useful, artificially blocking it (which has to be implemented; although shouldn't be difficult to do) doesn't have much use either. Unless it would come with benefits, like scene-aware autocompletion or type-checking, but that's rather godotengine/godot#21187. It's like forcing static typing because "it's good for you".

But then again, you mentioned that it should be optional:

possibility to lock a script onto a node

Well, that would be more complicated to implement.

@Shadowblitz16
Copy link
Author

Shadowblitz16 commented Apr 30, 2021

the original idea was that you would be able to lock in script on nodes, but I would think its even better to just have inherited scenes not allow you to add or remove a script and only extend it.

also its not really forcing anybody to do anything because its not that hard to recreate a scene if you don't want to use it.
also it shouldn't be up to the person deriving from the scene what script they want attached to it, it should be up to the person who made the scene to provide a interface they can extend and do what they want to with.

@dalexeev
Copy link
Member

dalexeev commented May 1, 2021

These kinds of guarantees take too much effort, far more than they can be useful. You cannot guarantee that important_object.free() will not be called at runtime. But as long as you have power over your code and your scenes, just don't do destructive actions (and the fact that you can do them shouldn't worry you so much).

@Shadowblitz16
Copy link
Author

how does freeing a object have anything to do with locking a scripts to a derived scene?

@dalexeev
Copy link
Member

dalexeev commented May 1, 2021

Right now any script can be removed even if another scene relies on it.

but people can remove the script and attach a new one

How do I guarantee that my BulletSpawner Node or Scene can spawn bullets if the script can be removed?

Just don't do it. You are too fixated on guarantees.

how does freeing a object have anything to do with locking a scripts to a derived scene?

Because this is an analogy. You can, for example, successfully delete a singleton using queue_free(), and most likely something will break. You just don't need to do this so that nothing breaks, although you can.

And in inherited scenes, the script can extend the script of the ancestor scene, so the script is not locked so that you can delete the old one and attach the new one inherited from the old one. You shouldn't just delete it, of course.

@Shadowblitz16
Copy link
Author

Shadowblitz16 commented May 1, 2021

You shouldn't just delete it, of course.

why should we allow someone to remove a script from someone else's scene?
again it should be up to the creator of that scene to define what it is and isn't used for.

Because this is an analogy. You can, for example, successfully delete a singleton using queue_free(), and most likely something will break. You just don't need to do this so that nothing breaks, although you can.

also this shouldn't be possible either.. if code relies on a singleton they should be able to require it before use.
this really isn't a problem with the scene system though as its a limitation of gdscript since c++ and C# have static classes.
also gdscript could get around this using a true singleton and making sure when its used at all it loads it and enables it.

scripts and scenes are a whole different issue.
your literally forcing people to seperate their logic (scripts) and the predefined engine nodes and not providing a way to guarantee they are there without a tool script

@Shadowblitz16
Copy link
Author

Shadowblitz16 commented May 4, 2021

here is a example of unity3d supporting something similar, I'm sure other game engines support it as well
image

Looks like unity3d allows removing components from a prefab but it does say it was removed.
This isn't a problem really because game objects are generic and contain no real code.
Godot on the other hand contains logic in the built in nodes and can't attach multiple scripts.
image

@Shadowblitz16
Copy link
Author

These would help resolve the issue a lot because you can guarantee a interface through variables.
#2699
#615

@dalexeev
Copy link
Member

dalexeev commented May 8, 2021

Even if we prohibit deleting the script through the UI in the inherited scene, then we must add the ability to extend this script (replace the script with an extending one) and return to the previous script. And besides, additional checks will be required that the user did not manually change the script to the wrong one or did not change the extends ... to the wrong script. That is, it is not that easy. Maybe you just don't need to shoot yourself in the foot? 😃

@Shadowblitz16
Copy link
Author

Shadowblitz16 commented May 8, 2021

@dalexeev so your saying we should allow people to shoot themselves in the foot?
Godot is suppose to be a easy to use engine, if you allow people to shoot themselves in the foot it makes it harder for new users to learn.

Right now godot doesn't have a good way to link scripts together, Its type system is very immature.
These are not good things.
Yes it will take effort and time but all good engines take effort and time.

There is no benefit of allowing users to remove the logic of other people's scenes, and there is no loss of disallowing people to do it, they are there for a reason.

Edit: Also we can already extend scripts. the only thing we would need to do is disable the remove and add script options in the editor and make the script property setter check to see if the scene is inherited and if it is return without setting the script

@Shadowblitz16
Copy link
Author

I am closing this for a new issue specifically about builtin scripts only.

@YuriSizov YuriSizov closed this as not planned Won't fix, can't repro, duplicate, stale Jul 16, 2022
@Shadowblitz16
Copy link
Author

here is the new proposal #4890

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

No branches or pull requests

4 participants