Refine Auto-save notification #877
Conversation
There was a problem hiding this comment.
Looks ok, but is a Unity console message better than a Rider notification?
Perhaps the Rider notification could have a hyperlink that causes the plugin to display the settings?
Or we just automatically set the do-not-compile-during-play-mode flag and show a Rider notification saying so with a link to show settings?
And one thing that would be really nice is a "learn more" link to take us to the wiki with more details.
| @@ -156,6 +156,9 @@ private static Version TryGetUnityVersionFromDll(XmlElement documentElement) | |||
| return null; | |||
|
|
|||
| var filePath = FileSystemPath.Parse(referencePathElement.InnerText); | |||
| if (!filePath.IsAbsolute) | |||
There was a problem hiding this comment.
If it's not absolute, does that mean it's relative to the project file? Can we use that to try to resolve it and continue?
There was a problem hiding this comment.
Unfortunately I don't have repro. Just Exception https://youtrack.jetbrains.com/issue/RIDER-21237
rider/src/main/kotlin/com/jetbrains/rider/plugins/unity/UnityHost.kt
Outdated
Show resolved
Hide resolved
|
I am rewriting it back to Notification in Rider. In progress. |
van800
force-pushed
the
183-auto-save-notification
branch
from
9594465 to
6aeb7a3
Compare
|
Rewrote it from scratch, could you please review again @citizenmatt ? |
van800
changed the title
There was a problem hiding this comment.
Looks good. Suggested some changes to the text.
It would be really cool if the text would include links that changed the setting:
Unity play mode script compilation
Unity is currently configured to compile scripts during play mode, and Rider's auto save is currently enabled. This can cause unwanted loss of state in the running game.
Change Unity to:
You can also change this manually in the $tabName tab of Unity's Preferences.
I would actually like to take this further, and have Rider to automatically set it to "Recompile After Finished Playing" and then show a notification that says "we've changed this, because of auto save. Here's how to change it back". This would be a good default for people that don't read notifications.
What do you think?
|
|
||
| val message = "Auto save is enabled in Rider. This can cause unwanted recompilation and the loss of current state during play mode." + | ||
| "<br/>* Consider changing the <i>Script Changes While Playing</i> in Unity Preferences $tabName tab." + | ||
| "<br/>* <a href=\"doNotShow\">Do not show</a> this notification for this solution." |
There was a problem hiding this comment.
This wording worked better when it was shown inside Unity's console tab. If we're displaying a notification in Rider, we should say:
Unity play mode script compilation
Unity is currently configured to compile scripts during play mode, and Rider's auto save is currently enabled. This can cause unwanted loss of state in the running game.
- Consider changing the Script Changes While Playing in the $tabName tab in Unity's Preferences.
- Do not show this notification again for this solution.
See if you can use <ul><li>…</li></ul> for the bullet points instead of asterisks.
There was a problem hiding this comment.
If it is so good default for everybody, why then Unity hasn't used it by default.
I think that if you write a proper Unity-way code, then even in PlayMode you can reload scripts without problems.
|
|
||
| val generalSettings = GeneralSettings.getInstance() | ||
| if (generalSettings.isAutoSaveIfInactive || generalSettings.isSaveOnFrameDeactivation){ | ||
| val autoSaveNotification = Notification(notificationGroupId.displayId, "Unity: scripts reload while Playing", message, NotificationType.WARNING) |
There was a problem hiding this comment.
Set the title to "Unity play mode script compilation"
|
|
|
Nice! Do we want to show the notification for all solutions? The setting is application wide, isn't it? |
|
This setting is frontend one. I am not changing the way it is implemented. It changes something in .idea folder. |
|
Notification may be shown when protocol gets connected. |
No description provided.