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

1.4.0 - RC1 #78

Merged
merged 1 commit into from Jun 5, 2014
Merged

1.4.0 - RC1 #78

merged 1 commit into from Jun 5, 2014

Conversation

wms
Copy link
Contributor

@wms wms commented Jun 2, 2014

I've assembled the PRs tagged for 1.4.0 on my develop branch, and would like to merge them upstream. Change summary is as follows:

If I've missed anything from this list that we said we would include, or if you have any other feedback, please let me know.

EDIT - This also disables the per-frame caching that I believe leads to the dreaded vessel duplication bug. This is only a partial application of madadam's original fix, but I found that this change was enough to prevent the map view glitching out, which I believe is the precursor to the main bug. See my comments at #59 for further information.

@pjf
Copy link
Contributor

pjf commented Jun 2, 2014

For us watchers without a build environment yet, is there a .dll built from this? Because I'd sure be happy to help test. :)

@wms
Copy link
Contributor Author

wms commented Jun 2, 2014

@pjf - We're working on automating pre-release builds for exactly this reason as we speak. Stay tuned!

@pjf
Copy link
Contributor

pjf commented Jun 2, 2014

@warrenseymour : You totally rock! Thank you! :D

@Pezmc
Copy link
Contributor

Pezmc commented Jun 2, 2014

Not ideal, but here's a copy as of a7e0fe8 from warrenseymour/RemoteTech/develop

rt2-warren-develop-@a7e0fe8.zip Removed, see Warren's comment below.

@Starstrider42
Copy link
Contributor

Missing PR's:

@pjf
Copy link
Contributor

pjf commented Jun 2, 2014

Oh dear. Using @Pezmc 's build, and I just had the connection lines start failing, followed by Kerbin disappearing, and the space centre along with it; I'm pretty sure this counts as the map view glitching out. :(

@wms
Copy link
Contributor Author

wms commented Jun 2, 2014

@pjf That'll be the bug. Can you tell us what actions you took prior to this occurring?

@wms
Copy link
Contributor Author

wms commented Jun 2, 2014

@pjf
Copy link
Contributor

pjf commented Jun 2, 2014

@warrenseymour : I honestly don't know what actions I took, and I'm playing with a pretty heavy mod stack (everything in Realistic Progression Lite), which may make things difficult to reproduce. But in any case I'll be delighted to test with the new build.

Only gotcha is that it's just hit 3am in Australia and I'm about to crash (hopefully more gently than my last probe), so it may be at least a day or two before I'm playing again. But in any case, I'm happy to track dev builds and give feedback where I can.

flightInfo += EngineActivated ? RTUtil.FormatDuration(RemainingTime) : "-:-";

return flightInfo + Environment.NewLine + base.Description;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Once we get to this level of complexity on string concatenation, we should switch to string.format

return string.Format("Executing maneuver: {0}m/s{3}Remaining duration: {1}{3}{2}", 
    RemainingDelta.ToString("F2"), 
    EngineActivated ? RTUtil.FormatDuration(RemainingTime) : "-:-",
    base.Description,
    Environment.NewLine);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a number of the command classes use this approach, but I agree a formatted string would be better. Care to refactor?

@pjf
Copy link
Contributor

pjf commented Jun 3, 2014

@warrenseymour : Tested with the build you supplied. Launched my rocket, suffered a failure with a detached failing striking a fuel tank during escape. Managed to get my probe to deploy its chutes for a soft landing, but discovered upon doing so that "recover craft" had no effect. Looking at the log file reveals many:

[EXC 01:20:04.371] NullReferenceException
[EXC 01:20:04.376] KeyNotFoundException: The given key was not present in the dictionary.

although unhelpfully nothing saying if those errors are caused by RT2 or not. However I'm seeing map corruption (bogus connection lines, no craft shown anywhere), as well as not being able to switch to the space centre. I'm afraid that I can't say exactly when we switched from a bugged to non-bugged state.

Running KSP 0.23.5 64-bit under Linux, with more mods than you'd wish to count. ;)

@ghost
Copy link

ghost commented Jun 3, 2014

KSP_Data/output_log.txt has stacktraces available. I think it's in the same place under Linux. Maybe in your home folder?

@pjf
Copy link
Contributor

pjf commented Jun 3, 2014

And another launch, this time with different errors when switching to map view, but corruption also seen:

[LOG 01:35:05.913] [PlanetariumCamera]: Focus: Waterbear I
[LOG 01:35:05.920] Clouds: Volume Enabled=False
[LOG 01:35:05.921] Clouds: Volume Enabled=False
[LOG 01:35:05.922] Clouds: Volume Enabled=False
[LOG 01:35:05.928] [PlanetariumCamera]: Focus: Waterbear I
[EXC 01:35:05.955] ArgumentOutOfRangeException: Argument is out of range.
Parameter name: index
[LOG 01:35:05.958] RemoteTech: LoadImage(RemoteTech2/Textures/texBackground)
[LOG 01:35:05.958] RemoteTech: LoadImage(RemoteTech2/Textures/texPath)
[LOG 01:35:05.958] RemoteTech: LoadImage(RemoteTech2/Textures/texAll)
[LOG 01:35:05.958] RemoteTech: LoadImage(RemoteTech2/Textures/texDish)
[LOG 01:35:05.958] RemoteTech: LoadImage(RemoteTech2/Textures/texOmni)
[LOG 01:35:05.958] RemoteTech: LoadImage(RemoteTech2/Textures/texOmniDish)
[LOG 01:35:05.958] RemoteTech: LoadImage(RemoteTech2/Textures/texEmpty)
[LOG 01:35:05.958] RemoteTech: LoadImage(RemoteTech2/Textures/texPlanet)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButton)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButton)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButton)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButton)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonGray)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonGray)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonGray)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonGray)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonGreen)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonGreen)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonGreen)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonGreen)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonRed)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonRed)
[LOG 01:35:05.959] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonRed)
[LOG 01:35:05.960] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonRed)
[LOG 01:35:05.960] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonYellow)
[LOG 01:35:05.960] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonYellow)
[LOG 01:35:05.960] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonYellow)
[LOG 01:35:05.960] RemoteTech: LoadImage(RemoteTech2/Textures/texButtonYellow)
[LOG 01:35:05.960] RemoteTech: LoadImage(RemoteTech2/Textures/texSatellite)
[LOG 01:35:05.960] RemoteTech: LoadImage(RemoteTech2/Textures/texKnowledgeNormal)
[LOG 01:35:05.960] RemoteTech: LoadImage(RemoteTech2/Textures/texKnowledgeHover)
[LOG 01:35:05.960] RemoteTech: LoadImage(RemoteTech2/Textures/texKnowledgeActive)
[LOG 01:35:05.961] RemoteTech: LoadImage(RemoteTech2/Textures/texKnowledgeHover)
[EXC 01:35:06.011] ArgumentOutOfRangeException: Argument is out of range.
Parameter name: index
[EXC 01:35:06.052] ArgumentOutOfRangeException: Argument is out of range.
Parameter name: index
[EXC 01:35:06.094] ArgumentOutOfRangeException: Argument is out of range.
Parameter name: index
[EXC 01:35:06.135] ArgumentOutOfRangeException: Argument is out of range.
Parameter name: index

However with these errors I could return to the space centre, and then back to my probe, and everything would be fine. :)

@pjf
Copy link
Contributor

pjf commented Jun 3, 2014

@Cilph : Found it! ~/.config/unity3d/Squad/Kerbal Space Program/Player.log, which is a silly place for it.

For the second report, it looks like we have this (no timestamps in the Player.log file, but looks right from what's going on around it):

(Filename:  Line: 4294967295)

ArgumentOutOfRangeException: Argument is out of range.
Parameter name: index
  at System.Collections.Generic.List`1[RemoteTech.ISignalProcessor].get_Item (Int32 index) [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.<get_SignalProcessor>m__87 () [0x00000] in <filename unknown>:0 
  at RemoteTech.RTUtil.CachePerFrame[ISignalProcessor] (RemoteTech.CachedField`1& cachedField, System.Func`1 getter) [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.get_SignalProcessor () [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.get_Visible () [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.CheckVisibility (RemoteTech.BidirectionalEdge`1 edge) [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.<UpdateNetworkEdges>m__3C (RemoteTech.BidirectionalEdge`1 e) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable+<CreateWhereIterator>c__Iterator1D`1[RemoteTech.BidirectionalEdge`1[RemoteTech.ISatellite]].MoveNext () [0x00000] in <filename unknown>:0 
  at System.Collections.Generic.List`1[RemoteTech.BidirectionalEdge`1[RemoteTech.ISatellite]].AddEnumerable (IEnumerable`1 enumerable) [0x00000] in <filename unknown>:0 
  at System.Collections.Generic.List`1[RemoteTech.BidirectionalEdge`1[RemoteTech.ISatellite]]..ctor (IEnumerable`1 collection) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable.ToList[BidirectionalEdge`1] (IEnumerable`1 source) [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.UpdateNetworkEdges () [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.OnPreCull () [0x00000] in <filename unknown>:0 

(Filename:  Line: 4294967295)

My first report may just be the whole game corrupting itself, and doesn't appear to be directly related to RT2. The NullReferenceExceptions are from the clouds, and the KeyNotFound is from ScienceAlert. Apologies for the false alarm.

@wms
Copy link
Contributor Author

wms commented Jun 3, 2014

@pjf Thanks for the report; that stack trace confirms that the issue is still present. I've got an additional fix that I managed to wrangle from blizzy78, that should address this/ I'll get it pushed at some point this evening.

@ghost
Copy link

ghost commented Jun 3, 2014

Yeah, there was this weird issue where VesselSatellites seemingly exist without SignalProcessors, which should be impossible by design. I completely rewrote the design for this in my refactor, but I'm not sure if that fixed it. I just moved the responsibility to the Satellite/AntennaManager.

Do some checks to make sure Satellites are completely purged when no signal processors are left, and no queries are done on that Satellite while it is deregistering.

@Peppie84
Copy link
Member

Peppie84 commented Jun 3, 2014

Test summary:

To force this exception i tried two test cases:
Madadams docking test and the orbital staging test (original: duplicate test) with three versions of RT.

First Version: v1.3.3 (~10 tests)
I got this exception to 100% for both test cases

Second Version: The linked from warren
Madadams docking test: 100% with this Exception (~30 tests)
orbital staging: 100% bug free (~30 tests)

Third Version: (self compiled: wms/RemoteTech@master...develop )
Madadams docking test: 100% bug free (~20 tests)
orbital staging: 100% bug free (~30 tests)

@warrenseymour what version did you compiled for pjf ?
@pjf Can you please try your test with this dll: http://www.peppies-life.de/downloads/RemoteTech2_dll_only.zip

@pjf
Copy link
Contributor

pjf commented Jun 4, 2014

@Peppie23 : Aye aye! (Allow 12-16 hours for me to finish work and into space!)

@wms
Copy link
Contributor Author

wms commented Jun 4, 2014

@Peppie23 It would have been as of this commit - wms@51bab7e

I tried to keep per-frame caching and include jdmj's fix, but it didn't work. I've just updated my 'develop' branch to include jdmj's fix, but remove per-frame caching.

@wms
Copy link
Contributor Author

wms commented Jun 4, 2014

@pjf This build is slightly newer than the one peppie posted - https://www.dropbox.com/s/y06aa6tiha31os2/RemoteTech2-2a1cea2a67a7d3643ec9cb814d96582fde3b0a32.zip - please try that too. As far as I can see, this is the only issue blocking a release of 1.4.0 right now.

@pjf
Copy link
Contributor

pjf commented Jun 4, 2014

@warrenseymour: Managed to complete a high space mission in RPL using @Peppie23's DLL. No map glitches at all, which was fantastic. Ship had multiple controllers, staging, and all the other things which seem to trigger it, so that's very promising.

I still have a problem where trying to use the flight computer to maintain attitude results in it firing all the RCS thrusters and burning through all my propellant, but I've had that with every version of RemoteTech, and it appears completely unrelated to the map bug. It might be a duplicate of #76, and it might just be that I don't understand how to use the flight computer properly yet. In any case, I wouldn't block on a release because of that, as the 1.4.0 release candidate seems to have strictly fewer bugs than what's out there now. :)

I haven't tested 2a1cea2 yet, and may not get a chance to tonight.

Keep being awesome!

~ pjf

@wms
Copy link
Contributor Author

wms commented Jun 4, 2014

@pjf - Many thanks for the through investigation! The attitude problem is a known issue that's been around for some time, and for my money should be the first order of business (in terms of bugs) after we ship 1.4.0 - hopefully in the next few days!

@Starstrider42
Copy link
Contributor

I still have a problem where trying to use the flight computer to maintain attitude results in it firing all the RCS thrusters and burning through all my propellant, but I've had that with every version of RemoteTech, and it appears completely unrelated to the map bug. It might be a duplicate of #76, and it might just be that I don't understand how to use the flight computer properly yet.

It's not a duplicate of #76. Your problem is caused by the flight computer just being really, really aggressive, and making lots of large amplitude corrections it doesn't need. I know it sounds like a punchline, but the best workaround is to use reaction wheels instead of RCS. 😉

@Peppie84
Copy link
Member

Peppie84 commented Jun 4, 2014

@warrenseymour madadams docking/undocking test case always throws exceptions with your latest dll

KeyNotFoundException: The given key was not present in the dictionary.
  at System.Collections.Generic.Dictionary`2[System.Guid,System.Collections.Generic.List`1[RemoteTech.NetworkLink`1[RemoteTech.ISatellite]]].get_Item (Guid key) [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkManager.UpdateGraph (ISatellite a) [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkManager.OnPhysicsUpdate () [0x00000] in <filename unknown>:0 
  at (wrapper delegate-invoke) System.Action:invoke_void__this__ ()
  at RemoteTech.RTCore.FixedUpdate () [0x00000] in <filename unknown>:0 

combined with some:

ArgumentOutOfRangeException: Argument is out of range.
Parameter name: index
  at System.Collections.Generic.List`1[RemoteTech.ISignalProcessor].get_Item (Int32 index) [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.<get_SignalProcessor>m__86 () [0x00000] in <filename unknown>:0 
  at RemoteTech.RTUtil.CachePerFrame[ISignalProcessor] (RemoteTech.CachedField`1& cachedField, System.Func`1 getter) [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.get_SignalProcessor () [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.get_Visible () [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.CheckVisibility (RemoteTech.BidirectionalEdge`1 edge) [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.<UpdateNetworkEdges>m__3B (RemoteTech.BidirectionalEdge`1 e) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable+<CreateWhereIterator>c__Iterator1D`1[RemoteTech.BidirectionalEdge`1[RemoteTech.ISatellite]].MoveNext () [0x00000] in <filename unknown>:0 
  at System.Collections.Generic.List`1[RemoteTech.BidirectionalEdge`1[RemoteTech.ISatellite]].AddEnumerable (IEnumerable`1 enumerable) [0x00000] in <filename unknown>:0 
  at System.Collections.Generic.List`1[RemoteTech.BidirectionalEdge`1[RemoteTech.ISatellite]]..ctor (IEnumerable`1 collection) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable.ToList[BidirectionalEdge`1] (IEnumerable`1 source) [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.UpdateNetworkEdges () [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.OnPreCull () [0x00000] in <filename unknown>:0

@erendrake
Copy link
Member

I believe that this PR should be merged to Develop. It isnt perfect yet but at least it is a fairly known quantity. Also i would like to stop sending @warrenseymour pull requests.

@Pezmc
Copy link
Contributor

Pezmc commented Jun 5, 2014

Merging into develop makes sense to me! Especially if we can start generating pre-release develop builds for people to download and test. #78 doesn't actually have any changed in though?

@erendrake erendrake merged commit 2a1cea2 into RemoteTechnologiesGroup:develop Jun 5, 2014
@erendrake
Copy link
Member

Done, I imagine that leaves us with quite a few issues and PRs to close. If anyone is holding a PR you should check them to see if they are still needed.

@Starstrider42
Copy link
Contributor

Both of mine are still valid.

I think we should follow GitHub's default policy of not closing issues until they get merged with master, though. An issue isn't really confirmed fixed if we're still testing the fix.

@erendrake
Copy link
Member

Agreed, I will close some PRs but with issues you are correct.

@pjf
Copy link
Contributor

pjf commented Jun 6, 2014

Using 2a1cea2 I've just hit the bug whereby the link lines disappears, along with the space centre. First time playing with directional links, with an antenna each pointing at each of my mission controls (I have two in RSS).

This trace is repeated many, many times in flight, and once upon changing to the (now missing) space centre:

ArgumentOutOfRangeException: Argument is out of range.
Parameter name: index
  at System.Collections.Generic.List`1[RemoteTech.ISignalProcessor].get_Item (Int32 index) [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.<get_SignalProcessor>m__86 () [0x00000] in <filename unknown>:0 
  at RemoteTech.RTUtil.CachePerFrame[ISignalProcessor] (RemoteTech.CachedField`1& cachedField, System.Func`1 getter) [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.get_SignalProcessor () [0x00000] in <filename unknown>:0 
  at RemoteTech.VesselSatellite.get_Visible () [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.CheckVisibility (RemoteTech.BidirectionalEdge`1 edge) [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.<UpdateNetworkEdges>m__3B (RemoteTech.BidirectionalEdge`1 e) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable+<CreateWhereIterator>c__Iterator1D`1[RemoteTech.BidirectionalEdge`1[RemoteTech.ISatellite]].MoveNext () [0x00000] in <filename unknown>:0 
  at System.Collections.Generic.List`1[RemoteTech.BidirectionalEdge`1[RemoteTech.ISatellite]].AddEnumerable (IEnumerable`1 enumerable) [0x00000] in <filename unknown>:0 
  at System.Collections.Generic.List`1[RemoteTech.BidirectionalEdge`1[RemoteTech.ISatellite]]..ctor (IEnumerable`1 collection) [0x00000] in <filename unknown>:0 
  at System.Linq.Enumerable.ToList[BidirectionalEdge`1] (IEnumerable`1 source) [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.UpdateNetworkEdges () [0x00000] in <filename unknown>:0 
  at RemoteTech.NetworkRenderer.OnPreCull () [0x00000] in <filename unknown>:0 

(Filename:  Line: 4294967295)

@ghost
Copy link

ghost commented Jun 6, 2014

getSignalProcessor gets the 0th signal processor. That being unavailable means the satellite should be unregistered, and events listened to by the NetworkRenderer should delete it from any internal maps/sets.

So either the events are not firing, or the events are not being handled properly.

@Starstrider42
Copy link
Contributor

Hi guys,

Any word on duplication/corruption bugs? pjf's post above, #92 (if it's repeated), and the RCS part of #94 seem to be the only things blocking a 1.4.0 release.

@Pezmc
Copy link
Contributor

Pezmc commented Jun 12, 2014

Will do some gameplay with the latest pre-release and see if I can trigger any duplication, though ideally we need plenty of people to try!

@wms
Copy link
Contributor Author

wms commented Jun 12, 2014

Hey everyone, sorry I've been MIA the last week or so, I've been hammered with work. I've spent some time using build 13 in my modded career game and have only been able to reproduce the map bug once. That's an improvement over 1.3.3, where it would happen constantly, but I'm not sure how you guys feel about throwing this over the fence knowing that this defect is still present, even to a significantly lesser degree.

I was reluctant to take Madadam's fix and apply it wholesale, because it appeared to be a band-aid over deeper problems that Cilph pointed out in previous comments. However, I think that maybe we should just apply the whole thing and attempt to fix the real cause of the problem in a future release.

To address the RCS issue, shall we try to fix that for 1.4.0, or make it a high priority for the next release? I'm going to add my comments on this in #94

@Starstrider42
Copy link
Contributor

So I kind of ran out of steam on fixing #94, but I still think I can finish it this week. Now that we know the cause, it would be a shame to leave it unfixed.

@Starstrider42 Starstrider42 mentioned this pull request Jun 13, 2014
@Starstrider42
Copy link
Contributor

@warrenseymour, what's your take on the map/duplication bug? Given how infamous it's become among RemoteTech players, they might overreact if they see it in our "fixed" release. Would mentioning it as a known issue head things off, or would it give the impression that the problem is more serious than it is?

@pjf
Copy link
Contributor

pjf commented Jun 13, 2014

"Significant reduction in the incidence of the map/duplication bug."

If it's actually fixed, then that statement is still correct, and if it's still hiding in edge cases then we haven't told any fibs. (It's certainly gone from "most sessions" to "not in the observable universe" for me.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants