Serialized Feedback Editor#446
Conversation
…e. presentation counts); Refactor SerializedPreferenceModule to support access while EXR is not running; Fix issue where Feedback preferences can get overwritten with blank data
dunity
left a comment
dunity
left a comment
There was a problem hiding this comment.
It's great to have a means of viewing feedback request data/views, and clearing presentation/view counts without clearing the other pref data.
One addition I feel this PR needs, is a means of setting the global MaxPresentations value within the editor/tab. Ideally we should have a means of setting a MaxPresentations value for each request, but that goes beyond the scope of this PR. The k_DefaultMaxPresentations value should be surfaced from ProxyFeedbackRequest, and allow the int/value to be set within this editor.
This will be a nice compliment to an eventual enum in the EditorVR preference panel, that allows for the following options : {AlwaysShow, Hide, ResetViewcountsEachSession}. The EditorVR editor pref panel should also have a means of opening the ProxyFeedbackEditor panel.
| [Serializable] | ||
| class RequestData | ||
| { | ||
| readonly Action m_OnBecameVisible; |
There was a problem hiding this comment.
This is addressed. Not sure why it's not hidden.
| { | ||
| sealed class ProxyFeedbackEditor : EditorWindow | ||
| { | ||
| Dictionary<Type, SerializedProxyFeedback> m_SerializedFeedback = new Dictionary<Type, SerializedProxyFeedback>(); |
| if (preferences == null) | ||
| return; | ||
|
|
||
| foreach (var kvp in new Dictionary<Type, SerializedPreferencesModule.SerializedPreferenceItem>(preferences.items)) |
There was a problem hiding this comment.
is it necessary to create a new dict ? preferences.items is already a Dictionary<Type, SerializedPreferenceItem>
There was a problem hiding this comment.
We are removing elements
|
Oh yeah I forgot to ask/mention: is anyone else seeing errors all over ProxyFeedbackEditor within Visual Studio? It seems like it's ignoring the "InternalsVisibleTo" that we set up to access Engine stuff from an Editor class. If everyone else is getting it, I'll report a bug |
|
Manual merge successful. Performing Github merge |
3 participants
Purpose of this PR
There was a need to clear out the serialized data that stores the number of visible feedback presentations while testing. For example, if I want to show the system to a new user, I might want them to have a "fresh start," or I might want to just enable feedback for one or two features.
This gives us a way of viewing and editing the serialized data for feedback.
Testing status
Tested on Vive and Rift in various scenes. Tested starting EXR without activating the controllers, and with activating controllers but not engaging feedback. Found and fixed a bug where feedback data could be cleared, because we were always returning a SerializedFeedback, even if the nodes themselves were null.
Technical risk
Low: Only changes to actual EXR stuff were fixes/refactors. Adds an editor window.
Comments to reviewers
I had to expose a bunch of stuff as internal to get static access or access without EXR running. There is a little bit of code duplication as a result, and because Feedback Module is designed around being instantiated and working with EXR systems.