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

Support private and protected Custom Properties #150

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

elexis1
Copy link

@elexis1 elexis1 commented Aug 15, 2020

It is common and good practice to make properties and fields private or protected, so that the code flow becomes more obvious (fewer ways to change the state of a class instance) and less error prone.

For that reason codebases that protected most properties from other classes and utilize SuperTiled2Unity cannot use Custom Properties unless applying this patch.

On the patch correctness:
The reasoning to only write to public fields might have been to be consistent with the access modifier.
The counterargument to that would be that Unity well supports serialization and deserialization of private and protected fields, by using the [SerializeField] attribute. And the tiled 2 unity import process resembles deserialization.

If it is deemed important to distinguish public and non-public property Custom Properties, then one might add another checkbox in the Import user interface (similar to the Tiles As Objects checkbox).

@elexis1
Copy link
Author

elexis1 commented May 26, 2021

An argument to prohibit writing to private and protected fields is that the code becomes structured so that one can assume that private and protected fields are not written to except by the class that owns them.

An argument to allow writing to private and protected fields is that we want to be able to set initial property values of instances of prefab replacements and still have these fields being protected from being written to by other code. (Imagine having hundreds more of public fields in a codebase; it becomes much harder to find out which variable is written to by other code and which one is not.)

In case it's ambiguous whether it's a better choice to allow or prohibit assigning private and protected fields from tiled specified properties, we could also make this an optional behaviour using a compile-time constant to let the enduser decide individually and change their mind whenever they want.

It can be set here:
Project Settings -> Player -> Scripting Define Symbols
https://docs.unity3d.com/Manual/class-PlayerSettingsStandalone.html#Configuration
We could call it ENABLE_ST2U_PRIVATE_PROPERTIES.

Would that be preferable?

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.

1 participant