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

Mod compatibility checker - null issue #386

Closed
originalfoo opened this issue Jun 20, 2019 · 7 comments · Fixed by #333
Closed

Mod compatibility checker - null issue #386

originalfoo opened this issue Jun 20, 2019 · 7 comments · Fixed by #333
Assignees
Labels
BUG Defect detected COMPATIBILITY Mod (in)compatibility / checker irreproducible Unable to reproduce problem technical Tasks that need to be performed in order to improve quality and maintainability
Milestone

Comments

@originalfoo
Copy link
Member

Subaru had this error on the mods incompatibility checker (LABS prior to PR #333) - only happened one time, I'm unable to reproduce it.

However, I'll add some try/catch wrappers to the updates in #333 so future errors won't impact end-users.

Note: Mac platform - Mac has known issues with Linq, we should avoid Linq wherever possible. I'll try and rip out the Linq code from #333 - it's only used in one place IIRC, a .last() somewhere.

System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.ArgumentNullException: Argument cannot be null.
Parameter name: source
  at System.Linq.Check.Source (System.Object source) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable.ToArray[PublishedFileId] (IEnumerable`1 source) [0x00000] in <filename unknown>:0 
  at TrafficManager.Util.ModsCompatibilityChecker.GetUserModsList () [0x00000] in <filename unknown>:0 
  at TrafficManager.Util.ModsCompatibilityChecker..ctor () [0x00000] in <filename unknown>:0 
  at TrafficManager.TrafficManagerMod.OnGameIntroLoaded () [0x00000] in <filename unknown>:0 
  at TrafficManager.TrafficManagerMod.OnEnabled () [0x00000] in <filename unknown>:0 
  at (wrapper managed-to-native) System.Reflection.MonoMethod:InternalInvoke (object,object[],System.Exception
@originalfoo originalfoo added BUG Defect detected technical Tasks that need to be performed in order to improve quality and maintainability irreproducible Unable to reproduce problem labels Jun 20, 2019
@originalfoo originalfoo self-assigned this Jun 20, 2019
@originalfoo
Copy link
Member Author

Log file containing the error:
Mac - Player.log

@dymanoid
Copy link
Contributor

dymanoid commented Jun 20, 2019

It's not an issue with Linq, it's an issue with the code in ModsCompatibilityChecker.

Just ContentManagerPanel.subscribedItemsTable was null, and there is no null-check when accessing that field, see the GetUserModsList method.

@dymanoid
Copy link
Contributor

dymanoid commented Jun 20, 2019

@aubergine10, just curios: "Mac has known issues with Linq". Can you provide some links proving that? As far as I know, there are no issues with Linq implementation in Mono, neither on Mac nor on any other supported platform.

@krzychu124
Copy link
Member

krzychu124 commented Jun 20, 2019

Just ContentManagerPanel.subscribedItemsTable was null

@dymanoid, Is it possible? Just curious. I can see that ContentManagerPanel implements MenuPanel which calls Initialize() (init subscribedItemsTable array and other stuff) in Start() method.

I will check that anyway 😉

BTW I would like to see more info about issues with Linq too.
In case of checking for incompatible mods I didn't care much about performance because it is one time check and with 500k items, that operation should take max ~1sec (BTW Workshop has only ~186k items 😄 ).

@originalfoo
Copy link
Member Author

@dymanoid It was something @VictorPhilipp mentioned ages ago, specifically that it caused major performance issues on Macs (from very vague memory it was used in the savegame serialisation code).

In any case, I'll add the null checking on that subscribedItemsTable (I think I already ditched that in #333 so probably not an issue any more, but will check when time permits).

@originalfoo originalfoo changed the title Mod compatibility checker - Issue with Linq on Mac platform Mod compatibility checker - null issue Jun 20, 2019
@dymanoid
Copy link
Contributor

I'm not sure the "massive performance issues" on Mac were caused by Linq. Rather by an inappropriate Linq usage 😄

@VictorPhilipp, can you give a particular example where Linq causes performance drop on Mac only? This is a very interesting topic.

@krzychu124, that field indeed can be null. The call into your code comes from Awake of the OptionsMainPanel. The ContentManagerPanel is a different object, thus it will be initialized independently from the options panel. The initialization order is not guaranteed.

@originalfoo
Copy link
Member Author

originalfoo commented Jun 20, 2019

@dymanoid Check the updated code in #333 - that scenario should no longer be an issue as it no longer pulls mod list from or otherwise interacts with the ContentManagerPanel.

@originalfoo originalfoo added the COMPATIBILITY Mod (in)compatibility / checker label Aug 13, 2019
@originalfoo originalfoo added this to the 10.21 milestone Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG Defect detected COMPATIBILITY Mod (in)compatibility / checker irreproducible Unable to reproduce problem technical Tasks that need to be performed in order to improve quality and maintainability
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants