Jump to content


Feedback on 2016.1.135.7649-Release


  • Please log in to reply
1 reply to this topic

#1 ava

ava

    Member

  • Members
  • PipPip
  • 23 posts
  • LocationBruges

Posted 14 March 2016 - 11:57 AM

Hello,

I've been asked in a response to my review for feedback on the latest update of the P4Connect plugin on the asset store (https://www.assetsto...!/content/25523)

As a reference, we're currently using 2015.2.124.5673-Release, so I compare everything with that version.

1. The output in the console log is still the same, every time I run the game or open the project I get this output:

Posting New Topic - Perforce Forums - Mozilla Firefox.jpg

However, the setting says only to show "fatal" messages:

PrtScr capture_11.jpg

2. We use several third party plugins in our projects. If we put every plugin into his own folder in the Assets folder we would have a large and cluttered mess. So instead we have a folder "3rdParty" where we put all third party libraries. The same goes for the p4Connect plugin. If I use the latest version I get these error messages:

Unable to open archive file: C:/Develop/Kweetet-Dev/Launcher/Assets/P4Connect/Editor/P4Connect.icons
UnityEngine.AssetBundle:LoadFromFile(String)
P4Connect.Icons:Initialize()
P4Connect.Icons:CheckInit() (at c:/tmp/62217225/depot/main/p4-connect/src/P4Connect/P4Connect/P4Connect.PasswordPrompt.cs:71)
P4Connect.Icons:GetIcon(String)
P4Connect.Config:OnGuiStatusBar() (at c:/tmp/62217225/depot/main/p4-connect/src/P4Connect/P4Connect/P4Connect.Menus.cs:247)
P4Connect.Config:OnGUI()
UnityEditor.DockArea:OnGUI()

Failed to load C:\Develop\Kweetet-Dev\Launcher\Assets\P4Connect\Editor\P4Connect.icons bundle
UnityEngine.Debug:Log(Object)
P4Connect.Icons:Initialize()
P4Connect.Icons:CheckInit() (at c:/tmp/62217225/depot/main/p4-connect/src/P4Connect/P4Connect/P4Connect.PasswordPrompt.cs:71)
P4Connect.Icons:GetIcon(String)
P4Connect.Config:OnGuiStatusBar() (at c:/tmp/62217225/depot/main/p4-connect/src/P4Connect/P4Connect/P4Connect.Menus.cs:247)
P4Connect.Config:OnGUI()
UnityEditor.DockArea:OnGUI()

NullReferenceException: Object reference not set to an instance of an object
P4Connect.Icons.GetTexture (System.String name)
P4Connect.Icons.GetIcon (System.String name)
P4Connect.Config.OnGuiStatusBar () (at c:/tmp/62217225/depot/main/p4-connect/src/P4Connect/P4Connect/P4Connect.Menus.cs:247)
P4Connect.Config.OnGUI ()
System.Reflection.MonoMethod.Invoke (System.Object obj, BindingFlags invokeAttr, System.Reflection.Binder binder, System.Object[] parameters, System.Globalization.CultureInfo culture) (at /Users/builduser/buildslave/mono-runtime-and-classlibs/build/mcs/class/corlib/System.Reflection/MonoMethod.cs:222)

From what I gather this means that the path to the icons file is hardcoded. I get thousands of these messages, probably meaning that the code tries to load the file every OnGUI call, and not in a initialization function that stops when either the file has been loaded or an error occured.
I would suggest that the plugin can run from any folder a user would want to put it in. If the icons are the only items with a hardcoded path, I would just get rid of them, see also point 6. And don't do any loading in OnGUI functions.

3. The project root issue seems to be fixed

4. The message "Bad settings, please fix" popping up 4 times is gone, thank you :).

5. When I enter a username that does not exist I get an error in the log:

P4Exception: User test doesn't exist.

Which is good, but I also get this warning:

P4Connect - Perforce integration is enabled but inactive: Password invalid

Which is weird, since the user doesn't exist, why bother checking a password?

6. This may be a more personal issue and just a matter of taste, but the overall layout of the settings window looks quite clumsy and confusing to me, with the many big and colored buttons. And what's up with that bee? I'd suggest that you try to create a window layout that fits more in the professional look and feel of Unity.


So overall, I won't be upgrading since it breaks our projects and I see no reason (yet) to alter my review.

Regards,
Alex

#2 P4Norman

P4Norman

    Advanced Member

  • Staff
  • 81 posts
  • LocationOakland CA, USA

Posted 15 April 2016 - 07:21 PM

This is a duplicate topic,  please see the other one to see the follow up posts.




0 user(s) are reading this topic

0 members, 0 guests, 0 anonymous users