Version 1.1.0 closeout fixes #2010
Conversation
Fixes KSP-KOS#1783 UpdateHandler.cs * Add a new method ClearAllObservers which will clear all observers and Dispose of any observer that implements IDisposable SharedObjects.cs * Call the new ClearAllObservers method in SharedObjects.DestroyObjects HighlightStructure.cs * Add event subscription to monitor ship status * Unsubscribe from events inside of Dispose
Fixes KSP-KOS#1911 CommNetConnectivityManager.cs * Update HasConnection to return true no matter what if checking connection to the same cpu vessel * Also fix a comment typo
Fixes KSP-KOS#1977 SharedObjects.cs * Check to see if there is a Set method before setting a value to null
| // control path. | ||
|
|
||
| // WARNING: In stock this will only work for vessels with a relay antenna installed. | ||
| // This is a limitation put in place to improve performance in the stock game, and | ||
| // there isn't a very good way around it. | ||
| var net = CommNetNetwork.Instance.CommNet; | ||
| tempPath = new CommPath(); | ||
| return net.FindPath(vessel1.Connection.Comm, tempPath, vessel2.Connection.Comm) || net.FindPath(vessel2.Connection.Comm, tempPath, vessel1.Connection.Comm); | ||
| return vessel1 == vessel2 || net.FindPath(vessel1.Connection.Comm, tempPath, vessel2.Connection.Comm) || net.FindPath(vessel2.Connection.Comm, tempPath, vessel1.Connection.Comm); |
There was a problem hiding this comment.
Should we be using a raw reference-equals check here? We seem to be using vessel.id for this sort of check in a lot of places elsewhere in the code.
Granted, it would be really weird for there to be two instances of Vessel that are really the same vessel, so it probably doesn't matter, but I wouldn't put it past KSP to have something that weird happening.
There was a problem hiding this comment.
That's a fair point. The reason we use the vessel id comparison is that the default equality comparison walks the fields on the object to check equality, which compares ID, and Name, and all the underlying unity fields. Which means that comparing a single GUID should be faster than comparing all of those fields.
There was a problem hiding this comment.
I thought that obj1 == obj2 just gives a reference equals, regardless of the equality overrides of the object type, and you have to use obj1.Equals(obj2) to get it to actually execute the equality comparison method. (Otherwise how would you tell the compiler that you are trying to do a reference comparison, in cases where == had been re-mapped to not do it.)
There was a problem hiding this comment.
Ah okay, another place where C#'s features are ones I do NOT consider a good idea. The caller has to dig down into the implementation of a class to discover if == is a reference equals or not. It is, like 98% of the time, but to be sure about that other 2%, you have to explicitly call a method (ReferenceEquals) to do the more low-level thing of reference comparison. That's massively un-intuitive (calling a method by a name is a more high-level construct than using a keyword or operator like ==, but in this case the method by name is actually the more low-level operation, weirdly. I understand the need to over-ride an operator, but there shouldn't be a case where an operator is higher level than a method like this. The reference-equals should have itself been an operator or keyword too, not something implemented by the more high level framework library .NET.)).
CommNetConnectivityManager.cs * Compare Vessel.id instead of Vessel itself to save a little execution time
|
To be honest, I didn't spend as long as I usually do on testing this one. All the code changes were simple and straightforward. I know I won't be around 'til late tonight and I didn't want to hold up a potential pre-release waiting on me. I just did a very quick one-liner check on each of the issues the PR mentioned and verified they didn't happen anymore. I didn't really try to think of any strange edge-cases or anything like that. |
UpdateHandler.cs
Dispose of any observer that implements IDisposable
SharedObjects.cs
HighlightStructure.cs
CommNetConnectivityManager.cs
connection to the same cpu vessel
Fixes #1977
Fixes #1911
Fixes #1783