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

[Mono] Loading a scene with unbuilt/unexported variables automatically erases them. #15371

Closed
NathanWarden opened this issue Jan 5, 2018 · 8 comments · Fixed by #20583
Closed
Assignees
Milestone

Comments

@NathanWarden
Copy link
Contributor

Godot version:

34c988c

OS/device including version:

Any

Issue description:

Any variables in the main scene or any scene that gets opened before the Mono assembly is built will be automatically erased.

The most practical solution seems to be to build the mono assembly immediately upon opening the project prior to any scene opening. This way, any changes from other team mates that have been pulled in will also have their variables intact instead of erased

Steps to reproduce:

  1. Clone the following project
  2. Open the project
  3. Select the BehaviorTree node (notice no exported variables exist)
  4. Build the project with F5/F6 (and then hit F8 to stop)
  5. Quit the editor
  6. Open the project again
  7. Click on the BehaviorTree node
  8. Notice "Navigator Path" is empty
  9. Quit the project
  10. Do a "git reset --hard" on the repo to reset any changes
  11. Open the project
  12. Notice "Navigator Path" now has its correctly saved value

Minimal reproduction project:

The following project is the same one used in the video

Project:
https://github.com/NathanWarden/godot_ai_csharp

Video:
https://youtu.be/ymvryIPuZRU

@neikeq
Copy link
Contributor

neikeq commented Jan 5, 2018

Not only building it before opening the scene sounds like a hacky workaround, but if building fails then you still lose the variables.
It's weird though, because AFAIK you don't lose those variables if a GDScript fails to build, right? Maybe I'm doing something wrong.

@neikeq
Copy link
Contributor

neikeq commented Jan 5, 2018

It's weird though, because AFAIK you don't lose those variables if a GDScript fails to build, right? Maybe I'm doing something wrong.

Oh, wait. Actually the same happens with GDScript. Implementing automatic building should solve this issue then (supposing build doesn't fail), but IMO losing changes on exported variables when a build fails is really bad...

@Zireael07
Copy link
Contributor

Yeah, I am using export variables a lot with tool scripts, and if I make an error, they are gone until I fix it. I wish it didn't happen...

@NathanWarden
Copy link
Contributor Author

So, maybe Godot itself should just keep the variables until there's a successful build? This should cover all cases in all languages.. maybe?

@neikeq
Copy link
Contributor

neikeq commented Jan 5, 2018

I think yes, and instead of the reset-to-default button show a warning icon or something.

@neikeq
Copy link
Contributor

neikeq commented Feb 27, 2018

Look what I found: #17070

@neikeq
Copy link
Contributor

neikeq commented Jul 25, 2018

I cannot longer reproduce this on latest master. @NathanWarden can you still reproduce this?

@neikeq
Copy link
Contributor

neikeq commented Jul 26, 2018

Update: This issue is still valid. It was a special case which made me unable to reproduce it. Interestingly, thanks to this I started to understand the cause.

I think I came up with a good solution and I made a basic implementation for C# scripts. So far it seems to be working, but I haven't tested much.
The solution is to backup the exported properties when updating them. If a scripts fails to compile, instead of updating the exporting properties the normal way, it loads them from the backup file. Here is a wip: neikeq@c597db0
It's just a quick draft I made to confirm it works. Ideally, I would prefer this to be done by the editor, instead of being hard-coded in languages.
My implementation doesn't deal with tool scripts for now. Reason: they cannot be instanced if compilation failed, and they don't use placeholder script instances...

Another important fact to keep in mind is that this issue may not only affect properties, as it seems to be the case with #17070.

Anyway, I would really like to hear opinions. Specially @reduz

Reminder: this issue affects all script languages, not only C#.

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

Successfully merging a pull request may close this issue.

4 participants