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

Add project settings and move launcher path to it #852

Merged
merged 7 commits into from
Jun 29, 2020

Conversation

Barsonax
Copy link
Member

No description provided.

@Barsonax Barsonax marked this pull request as ready for review June 25, 2020 16:22
@Barsonax Barsonax requested review from ilexp and SirePi June 25, 2020 16:22
Copy link
Member

@SirePi SirePi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok for me, just a little thing bugging my brain: we have UserData and AppData, so why not ProjectData? 😄

@ilexp
Copy link
Member

ilexp commented Jun 27, 2020

Ok for me, just a little thing bugging my brain: we have UserData and AppData, so why not ProjectData? 😄

To me, ProjectSettings just triggers the right associations as to what one should expect to find inside, and it's a term that's well known from other software and Unity. It's also too different from UserData and AppData, which are both read or (in case of user data) even written by the game while playing, when ProjectSettings is really an editor / development thing.

@Barsonax
Copy link
Member Author

@ilexp when publishing the game editor files wouldn't be included right?

We have a number of different settings files:

For the game:

  • AppData.dat
  • UserData.dat

For the editor:

  • EditorUserData.xml (uses some custom code flow to write to the file)
  • ProjectSettings.dat (newly introduced in this PR)
  • DesignTimeData.dat (not sure whats in here, its not in a readable format)

And some default variants of these files. We might wanna look into cleaning this up so we treat all these settings in the same way but thats for a different issue #855.

I agree with sirepi that the naming for ProjectSettings.dat might not be optimal. Personally I think since we already have AppData.dat for the game simply calling it EditorAppData.dat makes sense as it has the same purpose but for the editor.

@ilexp
Copy link
Member

ilexp commented Jun 29, 2020

@ilexp when publishing the game editor files wouldn't be included right?

It depends - it's perfectly valid to deploy the editor with the game to give players a map editor or something. In that case, the project settings / (non-user) editor settings would be included as well. Perhaps cleaned up in some form, but that's a separate issue for later I think.

I agree with sirepi that the naming for ProjectSettings.dat might not be optimal. Personally I think since we already have AppData.dat for the game simply calling it EditorAppData.dat makes sense as it has the same purpose but for the editor.

I still think EditorAppData is rather opaque as a term, not giving you a good idea on its contents, whereas ProjectSettings already gives you a rough direction. I'd rather clean up the naming of AppData and UserData into something more descriptive tbh. UserData should, judging from its contents, rather be a name with something like game options, game settings, or similar. And AppData might be better off with something like system config, engine config, app info.

Edit: Maybe let's move this discussion over to #855? I've commented there with some more thoughts on this. For this PR I'm fine with either choice as long as you guys come to an agreement 👍

DesignTimeData.dat

It's non-global / per-object data that is only relevant in the editor, see here. Right now it only stores whether an object is locked or hidden. Could probably be refactored to be merged into any of the other files, depending on how we define the scope of this info, or stay separate for a while until we happen across a good idea on what else to do with it.

@Barsonax
Copy link
Member Author

For now I will keep the name as is in this PR. Lets change the name to the final name after discussing it in the issue.

@Barsonax Barsonax merged commit 5e48414 into master Jun 29, 2020
@Barsonax Barsonax deleted the feature/addProjectSettings branch July 15, 2020 18:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Auto-adjust LauncherPath in Duality solution template to match executable name
3 participants