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

Metadata doesn't save when adding it in editor #66421

Closed
fkeyzuwu opened this issue Sep 26, 2022 · 7 comments
Closed

Metadata doesn't save when adding it in editor #66421

fkeyzuwu opened this issue Sep 26, 2022 · 7 comments

Comments

@fkeyzuwu
Copy link
Contributor

fkeyzuwu commented Sep 26, 2022

Godot version

4.0 dev, ef26618, compiled in debug mode

System information

Windows 10

Issue description

Script metadata is being added in the editor, but isn't actually updated behind the scenes.
When doing a check with has_meta(), the metadata is not found.
When closing and re-opening the project, the metadata is not found in the editor since it isn't actually saved.

Steps to reproduce

Make a project, add metadata to a script via the inspector, try accessing it in code.
Also close and re-open the project to confirm the metadata is not saved.

Minimal reproduction project

Metadata Deleted.zip

@timothyqiu
Copy link
Member

I can't reproduce this on current master.

Is the metadata added to Script or to the Node? Metadata set on the script won't be saved.

@fkeyzuwu
Copy link
Contributor Author

fkeyzuwu commented Sep 26, 2022

Yes, the metadata is added to the script. But that is the problem.

I tried finding info about this but didn't couldn't find anything. But the metadata DOES let me add it on a script without a node, which was fixed by you in one of latest merges.
#65868

Its good that it doesn't crash, however if this is not the correct way to use metadata and won't give you the actual results that you entered in the UI, why not add an error message to the screen saying you cant do that and stop you from doing it?

I'd say there's a couple things that can be done, in order of easiest to hardest:

  1. Just leave it like this as the intended behavior, and at least somewhere on the documentation add a line which says metadata is not saved on scripts which are not attached to nodes, and you should not do it.
  2. Add an error popup that stops you from adding metadata on a naked script
  3. Add support for metadata related methods for naked scripts and saving them

Obviously I don't expect all this to happen anytime soon, but I assume 1 and 2 should not be THAT hard to do and would stop most of the confusion.

@timothyqiu
Copy link
Member

timothyqiu commented Sep 26, 2022

This has been a long existing issue.

Script is a special type of resource that does not have other properties saved (which is what could be improved). You can reproduce this on 3.x too. Checking Local To Scene won't be saved either.

But the metadata DOES let me add it on a script without a node, which was fixed by you in one of latest merges.

The crash is not related to script properties not saved. It crashed because the dialog did not handle Resource correctly.


Since your MRP uses has_meta() directly, you should add your metadata to the Node instead of the script. That'll be checked correctly.

@fkeyzuwu
Copy link
Contributor Author

fkeyzuwu commented Sep 26, 2022

Script is a special type of resource that does not have other properties saved (which is what could be improved). You can reproduce this on 3.x too. Checking Local To Scene won't be saved either.

Thanks for confirming this, I was suspecting Script is a special type of resource and that it doesn't save properties(not just meta). I'll be aware of this in the future.

Since your MRP uses has_meta() directly, you should add your metadata to the Node instead of the script. That'll be checked correctly.

This works if you the scripts inherits from node, but if the script is just RefCounted you still need to dynamically add meta in code, which kinda defeats the purpose for my use case of it being easy to add const values that are not hard-coded inside the script itself.

Thanks for the reply!

@kmadridr
Copy link

Shouldn't this issue be closed?

@fkeyzuwu
Copy link
Contributor Author

fkeyzuwu commented Oct 4, 2022

Shouldn't this issue be closed?

Dunno. The original issue deviated to a UI focused problem. But is still a problem and just because it is the "intended" way it should work, doesn't mean it isn't an issue and can't be misleading. Especially since in the UI/console nothing says something is wrong.

@adamscott
Copy link
Member

Closing as duplicate of #84653. That issue may be more recent than this one, it is better detailed than this one, as it is more circumspect.

@adamscott adamscott closed this as not planned Won't fix, can't repro, duplicate, stale Feb 1, 2024
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