Project

General

Profile

Bug #19510

ArgumentOutOfRangeException breaks saving, loading, exiting, and vessel switching under certain circumstances with Making History Expansion installed

Added by marr75 over 5 years ago.

Status:
New
Severity:
Low
Assignee:
-
Category:
Application
Target version:
-
Start date:
08/12/2018
% Done:

0%

Version:
Platform:
Windows
Expansion:
Making History
Language:
English (US)
Mod Related:
No
Votes:
Arrow u r green
Arrow d r red

Description

Under certain combinations of vessels and launch sites, the method/property `Vessel.get_LandedInStockLaunchSite` will throw an uncaught ArgumentOutOfRangeException and cause attempts to switch scenes, save, load, or exit to fail. The offending code will only execute when the expansion is installed.

The issue comes from a for loop that iterates through indices of `PSystemSetup.Instance.StockLaunchSites` but then accesses `PSystemSetup.Instance.LaunchSites[i].name`. Note the difference between the two collections being accessed, which can result in the `ArgumentOutOfRangeException`. With this discrepency resolved, the bug no longer occurs.

I know that for performance reasons, you've switched `foreach` loops to `for` loops. In more recent versions of Unity (and when not in a deeply nested loop) the difference between these loops is minor but the possibility of a bug of this nature is much higher with the for loop than the foreach loop. As far as I know, using foreach on an array now generates identical IL to a for loop even. Another downside of this switch is that while a foreach loop creates a reusable reference to whatever collection you're iterating through, for loops often repeat all of the accessors on the way to the collection being iterated when accessing members of the collection by their index. This is true in this case, obviously. The performance gains of not creating a single iterator for the entire loop which will pretty certainly be collected in a cheap, gen0 garbage collection vs repeatedly calling the same unnecessary accessors is dubious (especially if any of the properties are expensive, i.e. if anything in the dotted path is even a somewhat expensive to compute property).

Also available in: Atom PDF