-
Notifications
You must be signed in to change notification settings - Fork 102
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
1.4.0 - RC1 #78
Conversation
For us watchers without a build environment yet, is there a .dll built from this? Because I'd sure be happy to help test. :) |
@pjf - We're working on automating pre-release builds for exactly this reason as we speak. Stay tuned! |
@warrenseymour : You totally rock! Thank you! :D |
Not ideal, but here's a copy as of a7e0fe8 from warrenseymour/RemoteTech/develop
|
Missing PR's:
|
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. :( |
@pjf That'll be the bug. Can you tell us what actions you took prior to this occurring? |
@pjf Would you mind testing with this build? https://www.dropbox.com/s/dgspd5rdz909v0u/RemoteTech2-51bab7e6702d1c1362a7f8e3a2677afc64000e2d.zip |
@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; | ||
} |
There was a problem hiding this comment.
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);
There was a problem hiding this comment.
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?
@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:
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. ;) |
KSP_Data/output_log.txt has stacktraces available. I think it's in the same place under Linux. Maybe in your home folder? |
And another launch, this time with different errors when switching to map view, but corruption also seen:
However with these errors I could return to the space centre, and then back to my probe, and everything would be fine. :) |
@Cilph : Found 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):
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. |
@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. |
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. |
Test summary: To force this exception i tried two test cases: First Version: v1.3.3 (~10 tests) Second Version: The linked from warren Third Version: (self compiled: wms/RemoteTech@master...develop ) @warrenseymour what version did you compiled for pjf ? |
@Peppie23 : Aye aye! (Allow 12-16 hours for me to finish work and into space!) |
@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. |
@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. |
@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 |
@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! |
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. 😉 |
@warrenseymour madadams docking/undocking test case always throws exceptions with your latest dll
combined with some:
|
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. |
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? |
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. |
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. |
Agreed, I will close some PRs but with issues you are correct. |
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:
|
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. |
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! |
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 |
So I kind of ran out of steam on fixing #94, but I still think I can finish it this week. |
@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? |
"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.) |
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.