Wrong domain key logging and writing config to disk #254
Conversation
The config written on disk is located in an Editor folder, which means it wont appear in builds.
Ah yeah, I like that, it seems that the pr isnt good so I'll fix the name too :) |
| #if UNITY_EDITOR | ||
| if (string.IsNullOrEmpty(settingsInstance.apiKey)) | ||
| { | ||
| string filePath = "Assets/LootLockerSDK/Resources/Config/Editor/PersistedLootLockerConfig.txt"; |
There was a problem hiding this comment.
If we're writing to disk, then why not use something like .ini that is a standard for configuration?
There was a problem hiding this comment.
Either that, or simply write LootLockerConfig as a serizialized json and then we can deserialize the object straight into an instance when reading?
| #if UNITY_EDITOR | ||
| if (string.IsNullOrEmpty(settingsInstance.apiKey)) | ||
| { | ||
| string filePath = "Assets/LootLockerSDK/Resources/Config/Editor/PersistedLootLockerConfig.txt"; |
There was a problem hiding this comment.
Also, here you hard code the file path but below you use Application.dataPath. These risk not being the same. Please settle for one.
| static void WriteConfigToDisk() | ||
| { | ||
| //Check for an already existing persistent config file | ||
| string fileDirectory = "Assets/LootLockerSDK/Resources/Config/Editor/"; |
There was a problem hiding this comment.
We really should create these once statically I feel like and then just reuse the paths instead of typing it out like this every time.
| if(!Directory.Exists(fileDirectory)) | ||
| { | ||
| Directory.CreateDirectory(fileDirectory); | ||
| File.Create(filePath).Close(); | ||
| } | ||
| else if(!File.Exists(filePath)) | ||
| { | ||
| File.Create(filePath).Close(); | ||
|
|
||
| } |
There was a problem hiding this comment.
| if(!Directory.Exists(fileDirectory)) | |
| { | |
| Directory.CreateDirectory(fileDirectory); | |
| File.Create(filePath).Close(); | |
| } | |
| else if(!File.Exists(filePath)) | |
| { | |
| File.Create(filePath).Close(); | |
| } | |
| if(!Directory.Exists(fileDirectory)) | |
| { | |
| Directory.CreateDirectory(fileDirectory); | |
| } | |
| if(!File.Exists(filePath)) | |
| { | |
| File.Create(filePath).Close(); | |
| } |
| @@ -1,37 +1,70 @@ | |||
| using System; | |||
| using System.IO; | |||
| using System.Linq; | |||
There was a problem hiding this comment.
Are you really using Linq?
| #if UNITY_EDITOR | ||
| private void OnEnable() | ||
| { | ||
| ProjectSettings.APIKeyEnteredEvent += WriteConfigToDisk; | ||
| ProjectSettings.DomainKeyEnteredEvent += WriteConfigToDisk; | ||
| } | ||
|
|
||
| private void OnDisable() | ||
| { | ||
| ProjectSettings.APIKeyEnteredEvent -= WriteConfigToDisk; | ||
| ProjectSettings.DomainKeyEnteredEvent -= WriteConfigToDisk; | ||
| } | ||
| #endif |
There was a problem hiding this comment.
I think I need to be taught how all this works. We have more settings than api key and domain key. Game version for example is a game breaking setting that needs to be persisted.
| @@ -91,6 +95,8 @@ private void DrawGameSettings() | |||
| if (EditorGUI.EndChangeCheck()) | |||
| { | |||
| gameSettings.domainKey = m_CustomSettings.FindProperty("domainKey").stringValue; | |||
| DomainKeyEnteredEvent?.Invoke(); | |||
| m_CustomSettings.FindProperty("domainKey").stringValue = gameSettings.domainKey; | |||
There was a problem hiding this comment.
From what I understand m_CustomSettings will be recreated from gameSettings next frame right? So why set this?
| if (match.Success) | ||
| { | ||
| string domainkey = match.Value; | ||
| Debug.LogWarning("You accidentally used the domain url instead of the domain key,\nWe took the domain key from the url.: " + domainkey); |
There was a problem hiding this comment.
I don't think we need to be explicit about this. They should figure it out when the input changes.
The config written on disk is located in an Editor folder, which means it wont appear in builds.