[1.10.1+] Contract Configurator [v1.30.5] [2020-10-05]

[KerbMav]

I don't see anything in the log about loading the part SatsWithParts001.... so I have to ask: is SatsWithParts001 a valid part? I'd expect to see something like this in the log:

[LOG 18:45:25.280] PartLoader: Compiling Part 'ContractPacks/AnomalySurveyor/Monolith/part/mega_monolith'

It should ... I never changed anything since the contract worked - only the weird orbits are giving me pause.

Noone else having this, am I the only one using OrbitGenerator???

You said you could not reproduce the error?

[nightingale]

It should ... I never changed anything since the contract worked - only the weird orbits are giving me pause.

Noone else having this, am I the only one using OrbitGenerator???

You said you could not reproduce the error?

This error has nothing to do with orbits. Are you still having issues with the "weird orbits" in 1.4.1?

I have not attempted to reproduce it - looks to be unrelated to Contract Configurator to me.

[KerbMav]

The screenshot shows one orbit to many with no AP/PE - but the AP/PE thing inside Kerbin is gone.

[nightingale]

The screenshot shows one orbit to many with no AP/PE - but the AP/PE thing inside Kerbin is gone.

Three dropbox links and one exception is not a good bug report - even more so when you're saying you have two issues that appear to be completely unrelated.

The problem with the orbits has to do with the performance changes for 1.3.0 that 'preloaded' contracts to reduce lag. I was able to get rid of the icons, but didn't realize that there was extra orbits. I'll take a look at that tonight.

[KerbMav]

Three dropbox links and one exception is not a good bug report - even more so when you're saying you have two issues that appear to be completely unrelated.

The problem with the orbits has to do with the performance changes for 1.3.0 that 'preloaded' contracts to reduce lag. I was able to get rid of the icons, but didn't realize that there was extra orbits. I'll take a look at that tonight.

Sorry, its just that I have always been talking about extra orbits and the (now gone) AP/PE inside Kerbin.

[Olsson]

This does not seem to be compatible with RSS/RO.

Installed, no contracts show up in mission control, no contract packs installed and installed have both been tested.

Once contract configurator is removed, contracts show up as usual.

[nightingale]

This does not seem to be compatible with RSS/RO.

Installed, no contracts show up in mission control, no contract packs installed and installed have both been tested.

Once contract configurator is removed, contracts show up as usual.

Need logs

[nightingale]

Lots and lots of bugfixes! Download now!

Contract Configurator 1.4.2

• Fixed asteroid sample showing up in KSC contracts for Field Research (thanks maculator).
  
• Fixed mirrored heading/roll/pitch in SpawnVessel (thanks Xephan).
  
• Fixed issue with RemoteTech vessel tracking (thanks johnpmayer).
  
• Fixed OrbitGenerator NRE in tracking station (thanks HarlyKin).
  
• Fixed extra orbits in tracking station (thanks KerbMav).
  

[Olsson]

Need logs

Log: https://www.dropbox.com/s/5iht89g3khmjw91/output_log.txt?dl=0

Gamedata: http://i.imgur.com/LBo65Sn.png

I'm running 64-bit, so it might be the cause of the issue. However I read that you're implementing 64-bit support so I'll try my luck anyways.

Anything I missed? Anything I can do to help or make it easier?

Edited June 19, 2015 by Olsson

[RickKermen]

This does not seem to be compatible with RSS/RO.

Installed, no contracts show up in mission control, no contract packs installed and installed have both been tested.

Once contract configurator is removed, contracts show up as usual.

I'm having the exact same issue.

http://i.imgur.com/AeYpo0d.jpg - No Contract Configurator

http://i.imgur.com/FWMGete.jpg - Contract Configurator and all contracts installation (for testing purposes)

http://i.imgur.com/s1Enw0T.jpg - No contracts after install

Running KSP 64-bit on linux with RSS+RO

Regards!

Edited June 19, 2015 by RickKermen

[nightingale]

I'm having the exact same issue... i only have stock contracts wiht RSS+RO installed, contracts for contract configurator won't show up.

Running KSP 64bit on linux

Same response. Need logs.

EDIT: Nevermind, I see that Olsson posted his this morning.

[RickKermen]

I believe it has something to do with the names of planets, was checking the logs and contract configurator is giving these errors, (RSS uses now Kopernicus with real planet names):

"[EXC 20:22:35.107] ArgumentException: 'Minmus' is not a valid CelestialBody. ContractConfigurator.ConfigNodeUtil.ParseCelestialBodyValue (System.String celestialName)

ContractConfigurator.ExpressionParser.CelestialBodyParser.ParseIdentifier (ContractConfigurator.ExpressionParser.Token token)

ContractConfigurator.ExpressionParser.ExpressionParser`1[T].ParseVarOrIdentifier (ContractConfigurator.ExpressionParser.Token token)

ContractConfigurator.ExpressionParser.ExpressionParser`1[CelestialBody].ParseSimpleStatement[CelestialBody] ()

Rethrow as Exception: Error parsing statement.

Error occurred near '*':

Minmus"

My log file also: http://www.filedropper.com/ksp

I hope it helps

Edited June 19, 2015 by RickKermen

[nightingale]

You guys are killing me with these edits... I tend to read posts very fast so if you go back and edit them there's a very good chance I will never see the edit.

Anyway, I missed the fact that RSS actually got released a couple days ago, since it's an official release now (and since I've gotten through the bulk of the other problem reports) I'm much more willing to go try and reproduce this myself. If either of you can reproduce on 32-bit before I'm able to test it (won't be for at least 7 hours), then I'd appreciate it.

One major problem is this:

ArgumentException: 'Kerbin' is not a valid CelestialBody.

Basically, with the move to real names (which is a good change), most contract packs won't work with RSS. Tourism and Field Research should've (but when I looked there's a handful of contracts in each that I accidentally hardcoded to Kerbal names). Still those are about the only two that will even remotely work.

But that won't cause your stock contracts to disappear. I couldn't see what the problem there was - but I looked rather quickly, and it could be because of 64 bit or any other number of things. Anyway, I'll take a look at this soon.

[RickKermen]

Sorry about the edits

Concerning 32 bit version, it won't even launch on my linux installation, is there any other way i can help?

[nightingale]

I believe it has something to do with the names of planets, was checking the logs and contract configurator is giving these errors, (RSS uses now Kopernicus with real planet names):

"[EXC 20:22:35.107] ArgumentException: 'Minmus' is not a valid CelestialBody. ContractConfigurator.ConfigNodeUtil.ParseCelestialBodyValue (System.String celestialName)

ContractConfigurator.ExpressionParser.CelestialBodyParser.ParseIdentifier (ContractConfigurator.ExpressionParser.Token token)

ContractConfigurator.ExpressionParser.ExpressionParser`1[T].ParseVarOrIdentifier (ContractConfigurator.ExpressionParser.Token token)

ContractConfigurator.ExpressionParser.ExpressionParser`1[CelestialBody].ParseSimpleStatement[CelestialBody] ()

Rethrow as Exception: Error parsing statement.

Error occurred near '*':

Minmus"

My log file also: http://www.filedropper.com/ksp

I hope it helps

Huh, that's interesting... 32 bit log, but nothing jumps out aside from the 100 planet name errors (which shouldn't cause any problems with the contract system itself, as those contracts will just be disabled).

I'll dig some more once I have access to KSP.

[RickKermen]

I've tested with the 2 contract packs that don't have Kerbal names hardcoded

1- http://i.imgur.com/dvVC7B6.jpg - Tourism and Field Research installed

2- http://i.imgur.com/Kj0jpWq.jpg - Stock contracts show up with no problems (don't know if the Toursim and Field Research are supposed to show up at a new started game)

3- http://i.imgur.com/8W4BE70.jpg - With ScanSat contracts pack, also shows the stock contracts but not the scansat ones (again don't know if they are supposed to show up at a new started game)

I hope it helps, good luck and ty for the must have mods you provide us!

[nightingale]

Okay, that last report from RickKermen is actually super helpful - I think I know what's going on.

First off, neither Tourism or Field Research have start of game contracts. Tourism you need to achieve orbit, whereas you can start getting Field Research as soon as you've done the first launch (I think, going by memory). SCANsat (both variants) you need to achieve orbit of the planet that you want to scan.

Because of the way Contract Configurator works, there's only really one contract type as far as stock KSP is concerned. But if the player has a ton of contract packs installed, I want the contract system to generate more Contract Configurator contracts that it otherwise would, so I cheat the system and add duplicates in the list. The number I add is based on a little formula that I've tweaked over time, but if you install everything that's available, you'll probably end up with 6 or 7 in the list (you can see this by going to the debug menu and looking at one of the contract tabs, you'll see a bunch of ConfiguredContract entries).

Now, what'll happen is that when KSP tries to generate a contract, it'll pick from that list, and end up picking Contract Configurator. Which will say "no, can't generate a contract right now". The way the stock system works, it won't try every type, but will just give up after a few failed attempts at generating a contract (guessing 3 or 4, but haven't tested to confirm).

So there's two problems:

1. I still count failed contracts when determining the "magic number" to use (I can fix this).
   
2. Stock KSP contract generation is sill (I cannot fix this).
   

So the big take-away is to only use the contract packs that support RSS, which at the moment should be:

1. Tourism Plus (needs a minor bugfix to one contract - will do soon).
   
2. Field Research (needs fixes to about half the contracts - will do soon).
   
3. SCANsat lite (haven't confirmed, but 95% sure it'll be good based on what I know of it).
   

For any other contract pack... go to the author's thread, and bug them that you want RSS support!

[RickKermen]

Alright, i'll start bugging them all!

Regards!

[nightingale]

Actually, I may look at just providing an RSS compatibility file instead... In many cases that would be good enough.

[nightingale]

Silly thing - I realized the reason no contracts were showing up was that Advanced Progression disables them. With nothing to replace them with, it becomes a lonely mission control.

[nightingale]

And ignore my comment on an RSS compatibility file via module manager. That's a bad idea for many reasons. In general it's much better if contract packs choose to make their stuff work in RSS.

Now being able to tell someone that the contract pack they are using does not work in RSS would be a nice feature... I'll have to think to see if I can come up with an easy way to do that.

[RickKermen]

And ignore my comment on an RSS compatibility file via module manager. That's a bad idea for many reasons. In general it's much better if contract packs choose to make their stuff work in RSS.

Now being able to tell someone that the contract pack they are using does not work in RSS would be a nice feature... I'll have to think to see if I can come up with an easy way to do that.

So Advanced Progression is the main issue...? Also contract makers inform that the contract pack they are using does not work in RSS is really a good suggestion!

I'll also keep testing contract packs

[compy286]

I am also getting the stutter on flight and removing the anomoly contracts seems to fix it. I have started looking through the source this looks a little less than performant to me:

private IEnumerable<KeyValuePair<ConfiguredContract, bool>?> ContractGenerator(Contract.ContractPrestige prestige)

{

// Loop through all the contract groups

IEnumerable<ContractGroup> groups = ContractGroup.AllGroups;

foreach (ContractGroup group in groups.Skip(nextContractGroup).Concat(groups.Take(nextContractGroup)))

{

nextContractGroup = (nextContractGroup + 1) % groups.Count();

foreach (KeyValuePair<ConfiguredContract, bool>? pair in ContractGenerator(prestige, group))

{

yield return pair;

}

}

}

it looks like you are doing this:

take N items from the beginning, push them to the end, and then skip those N items. Basically you are shaving and re-appending a list to do something you could solve simply by tracking an index globally.

private static int groupIndex = 0;

private IEnumerable<KeyValuePair<ConfiguredContract, bool>?> ContractGenerator(Contract.ContractPrestige prestige)

{

IEnumerable<ContractGroup> groups = ContractGroup.AllGroups;

if(groupIndex > groups.Count) groupIndex = 0;

ContractGroup group = groups.elementAt(groupIndex );

groupIndex ++;

foreach (KeyValuePair<ConfiguredContract, bool>? pair in ContractGenerator(prestige, group))

{

yield return pair;

}

return NULL; //you should never reach here

}

I did a test in a mono.net project over just a list of integers and over 1million iterations the loop you wrote is taking about 4 seconds slower to complete. Basically you are making the computer do all of the work to shuffle those arrays to simply return them in the order it's stored in.

Your skipping(.Skip) N forward but then you are cutting N off the front of the array with .Take and moving it to the back with .Concat(). It wouldn't be so bad if you stored that result and refreshed it but since the IEnumerable that is created from all that only lives the scope of that foreach loop. Furthermore the .Take creates a new list that then chews some memory up and triggers garbage collection.

There are a few more places where you do something similar but I didn't really look into them, I am just a programmer sad that once you get enough contract mods installed this mod eats all my CPU time :-(. If I was actually set up to develop KSP mods i'd be glad to look into it a bit for you.

[nightingale]

I am also getting the stutter on flight and removing the anomoly contracts seems to fix it. I have started looking through the source this looks a little less than performant to me:

<snip>

Interesting analysis, but there's a couple issues:

1. You mention "the .Take creates a new list". This is completely wrong. The IEnumerable methods (Skip, Take, Concat, etc) do not create new lists. Enumerables in C# use deferred execution - meaning that it's really just storing a "pointer" and returning the appropriate next element when it is asked for. Is it slower than if I had just indexed with an int? Certainly - but I don't believe it's significant.
   
2. You say you've tested it and over 1 million iterations, my loop is slower by 4 seconds (slower than "something"). But you haven't shown any code for me to comment on, and worse, the 4 seconds is not terribly meaningful without context. What percentage faster is it?
   
3. In the whole, I don't believe the section you've locked on to is material to the performance perceived by the player (more on this in a bit).
   

The biggest problem is that the most valuable resource I have when it comes to Contract Configurator development is time (my time, specifically). I have a very limited amount of time to work on Contract Configurator at the moment, and with bug fixes and so there's only so much I can get done at the moment.

Anyway, in terms of the more general performance issues that you speak of, there's two main areas that need to be addressed:

1. Time to execute the contract generation logic (ie. performance).
   
2. Frequency of with the contract generation logic is run
   

The main culprit for #1 is the expression parser. Back when it was a very simple system used in a couple small places, I made a simplifying decision to combine the parse and execution logic into one. Now the expression sub-system is a fairly major thing (and is absolutely huge in terms of how it is used in Field Research, which is why the bulk of reports relate to that). So I really need to go back and split out parsing and execution into different code paths (which means parsing the expressions into a sort of "executable" object). This is a big effort. I could probably work for 2-3 weeks on nothing but that and still not quite be done. So even though it's probably what ideally needs to be done, it's not something I'm looking at taking on.

Frequency is something I have more control over with the changes in 1.3.0, so it'll be something I'll be looking at. There's lots and lots of little things that can get done here that will hopefully add up to reducing how often the laggy bits happen (but not eliminate them entirely):

1. Break up the contract requirement logic to check some thing that can't be affected by expressions before the expressions are run.
   
2. Add logic to be smarter about not trying to constantly re-generate the same contract type if it keeps failing over and over (usually due to the player not meeting the proper requirements yet).
   
3. Don't generate at all in some pre-determined circumstances (like when the "list" is full). This is actually already the case, but definitely could use a second look.
   

The final thing that I want to look into is just shoving the contract generation into its own thread. I've seen lots of rhetoric about "Unity not supporting multi-threading", but very little discourse on what that actually means (ie. where are the limitations). I'm really doing very little "Unity" stuff in the contract generation logic, so hypothetically it may be possible to move that into its own thread... big potential to break stuff though, so would need a huge amount of testing.

Anyway, if you do really still believe that the code you've identified is material, then test it out in Contract Configurator, and send a pull request (for functionally equivalent code ). Or if you really want to help out, you can roll up your sleeves and start poking at one of the items above (there's a couple that are relatively low-cost to implement/test).

There are a few more places where you do something similar but I didn't really look into them, I am just a programmer sad that once you get enough contract mods installed this mod eats all my CPU time :-(. If I was actually set up to develop KSP mods i'd be glad to look into it a bit for you.

What's stopping you? Sounds like you've got the compiler, you have the skills from your post, and you obviously have the game, so...

[compy286]

I did a comparison of a standard single incrementing index that loops back to 0 vs. your code.

The execution with the groups.Skip(N).Concat(groups.Take(n); vs a a standard static index.

Here's a code example, just plop it into a new console application in Monodevelop/xamarin/visual studio:

https://www.dropbox.com/s/8lu1i1k9x269z4p/Program.cs?dl=0

In vs 2012 it was about 3.8 seconds to iterator the .take etc out .35 to do mine over a million iterations. In mono you see 6 seconds vs .38.

I think the concat is what is biting you in the ass really in that tight loop. It's also a bit concearning to see linq execution times in mono take almost twice as long as they do in native .net.

I did read up on how to start with KSP modding and didn't realize it was as easy as simply pointing Visual studio at some DLLs. I got Contract Configurator built, dropped in a static index vs the linq/ienumarable code and saw that most runs aren't hitting your debug warnings. I also enabled verbose logging and did notice that the real problem was in fact the parsing engine as you said.

I did find you could gain an immense deal of performance boost from simply making all the list initialization using temp variables on one line like this:

This is from scientist.cfg from Field Research

OLD:

DATA

{

type = List<ScienceSubject>

scienceSubjectsTemp1 = AllScienceSubjectsByBiome([@biome])

scienceSubjectsTemp2 = @scienceSubjectsTemp1.Where(s => s.Situation() == SrfLanded)

scienceSubjectsTemp3 = @scienceSubjectsTemp2.Where(s => s.NextScienceReportValue() > 1.0)

scienceSubjects = @scienceSubjectsTemp3.Where(s => !s.Biome().IsKSC()).Random(3)

}

vs NEW:

DATA

{

type = List<ScienceSubject>

scienceSubjects = AllScienceSubjectsByBiome([@biome]).Where(s => (s.Situation() == SrfLanded && s.NextScienceReportValue() > 1.0 && !s.Biome().IsKSC())).Random(3)

}

Readability sucks but it does end in a noticeable net increase for the parsing/execution bits. I get a pretty nasty amount of stutter but it's more frequent stutter instances that last shorter so the heartbeat is definitely ticking a bit faster.

Threading the parsing bit is an interesting proposition. While unity doesn't handle threading I bet it wouldn't mind linking to a library that is threaded. I do it in a single threaded scripting engine for running automated tests at work. Testcomplete lets you load in .net libraries which are great for creating glue layers for functionality you wish you had.

The DATA regions are the ones that are dynamically reloaded correct? The parameter regions stay static?