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

removed ondisable method, to fix #349 #369

Merged
merged 2 commits into from
Oct 23, 2022

Conversation

soraphis
Copy link
Collaborator

@soraphis soraphis commented Oct 18, 2022

this addresses #349 and fixes an issue introduced with #313

when starting the runtime without changed event, it would set the "_changedInstantiatedAtRuntime" to true.

OnDisable is only called if you hit play again and are about to enter the playmode again, which then RESETS all your changes (e.g. assigning an event in edit time) to null.

the alternative would be to address this in the custom editor drawer, to set _changedInstantiatedAtRuntime to false again when _changed was set.

but I found that the OnDisable was not needed, as the values where already set back when leaving playmode and in the build (which i have not tested!! it wouldn't matter anyways since OnDisable is only called if the memory is freed altogether)

@soraphis soraphis added this to the v4.4.6 milestone Oct 18, 2022
@AdamRamberg
Copy link
Collaborator

Looks good to me ✅ Pushed a small commit removing _changed and _changedInstantiatedAtRuntime since they are not used anymore after removing OnDisable.

@soraphis
Copy link
Collaborator Author

soraphis commented Nov 2, 2022

@AdamRamberg @soraphis
not sure anymore if this will work.

Should do a check with disables domain reload and stuff.

@AdamRamberg
Copy link
Collaborator

@soraphis Not sure I follow. What exactly do you think will break in this change? Do you think we should revert this before doing the next release?

@soraphis
Copy link
Collaborator Author

soraphis commented Nov 6, 2022

When i find time again, i'll need to recheck again if everything is correctly resetted.

I think if domain reload is off, neither onEnable nor onDisable are called.

So this should not break anything, but i wanna double check state is correctly cleared between playmode switches, since i'm not 100% sure what my settings in the editor where.

@AdamRamberg
Copy link
Collaborator

Cool! Lets wait with the release until that then

@soraphis
Copy link
Collaborator Author

soraphis commented Nov 8, 2022

ok, as far as i can tell it still works correctly even if domain+scene reload is disabled in the editor settings.
so release is good to go imho. 👍

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

Successfully merging this pull request may close these issues.

None yet

2 participants